[libcxx-commits] [PATCH] D145186: [libc++] Temporarily not use compiler intrinsics for some type traits in Objective-C++ mode.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/__config:1265
+#ifndef __OBJC__
+#define _LIBCPP_USING_TYPE_TRAITS_COMPILER_INTRINSICS
+#endif
----------------
You'll need to clang-format this bit


================
Comment at: libcxx/include/__config:1265
+#ifndef __OBJC__
+#define _LIBCPP_USING_TYPE_TRAITS_COMPILER_INTRINSICS
+#endif
----------------
ldionne wrote:
> You'll need to clang-format this bit
Let's rename this to e.g. `_LIBCPP_WORKAROUND_OBJCXX_COMPILER_INTRINSICS` -- this is going to be really short lived until Clang is fixed.


================
Comment at: libcxx/include/__type_traits/remove_volatile.h:20
 
-#if __has_builtin(__remove_volatile)
+#if defined(_LIBCPP_USING_TYPE_TRAITS_COMPILER_INTRINSICS) && __has_builtin(__remove_volatile)
 template <class _Tp>
----------------
All type traits that work fine with the intrinsics should keep using the intrinsics IMO. I think we only need to carve out `__remove_pointer` and` __add_pointer` for now.


================
Comment at: libcxx/test/std/utilities/meta/meta.trans/objc_support.pass.mm:65
+// remove_pointer
+// The result of removing and readding pointer to `id` should be still `id`.
+static_assert(std::is_same<std::remove_pointer<id>::type*, id>::value, "");
----------------
ldionne wrote:
> We should also test that `!is_same<remove_ptr<id>::type, id>`. Otherwise, people could get ambiguous overloads (for instance).



================
Comment at: libcxx/test/std/utilities/meta/meta.trans/objc_support.pass.mm:65-66
+// remove_pointer
+// The result of removing and readding pointer to `id` should be still `id`.
+static_assert(std::is_same<std::remove_pointer<id>::type*, id>::value, "");
+
----------------
We should also test that `!is_same<remove_ptr<id>::type, id>`. Otherwise, people could get ambiguous overloads (for instance).


================
Comment at: libcxx/test/std/utilities/meta/meta.trans/objc_support.pass.mm:67
+static_assert(std::is_same<std::remove_pointer<id>::type*, id>::value, "");
+
+// remove_reference
----------------
Let's also add a test like this:

```
static_assert(std::is_same<std::add_pointer<std::remove_pointer<id>::type>::type, id>::value, "");
static_assert(std::is_same<std::remove_pointer<std::add_pointer<id>::type>::type, id>::value, ""); // this one is kinda paranoid, but just to be sure
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145186/new/

https://reviews.llvm.org/D145186