[libcxx-commits] [PATCH] D115100: [libc++] Enable the optimized _IsSame on GCC as well as Clang.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone added inline comments.


================
Comment at: libcxx/include/tuple:1120
+template <class ..._Ts>
+struct __all_are_swappable : __all<__is_swappable<_Ts>::value...> {};
+
----------------
ldionne wrote:
> Instead, I really feel like we could redefine `__all` to be:
> 
> ```
> template <bool ..._Pred>
> struct __all : _IsSame<__all_dummy<_Pred...>, __all_dummy<((void)_Pred, true)...>> { };
> ```
> 
> After all, we will generally instantiate `__all` with several types, however we probably won't instantiate `__all` itself several times. What I'm trying to say is that since `__all` is variadic, we will probably instantiate it some factor less than the number of types that we're passing into it, so it might be reasonable to define it as a `struct` despite the additional costs. That would fix our mangling problem without needing a definition for `__all_are_swappable`, which IMO is kind of weird.
That would fix the mangling problem for `__all` specifically, but that's an O(n) patch for an O(n^2) problem (doesn't fix things that aren't `__all`), whereas what I've done here is the O(1) patch for the O(n^2) problem.
Personally I'd rather keep the O(1) paper-over patch for now, and then if we see this Clang/GCC incompatibility issue coming up repeatedly, we think about how to generalize it //then//. I suspect the issue won't come up much at all, because C++>=20 stuff will all use `requires` instead of return-type SFINAE.


================
Comment at: libcxx/include/type_traits:553
 
-#if __has_keyword(__is_same)
+#if __has_builtin(__is_same)
 
----------------
ldionne wrote:
> Do we support any compilers where this is false? I don't think so -- we could probably just drop the `#if` altogether.
It appeared to me that MSVC doesn't support `__has_builtin` at all, so it ends up nerfed to false by default.
However, I'm OK with dropping the `#if` and seeing what happens. Someone can always put it back later.


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

https://reviews.llvm.org/D115100