[libc++] Enable the optimized _IsSame on GCC as well as Clang.

[• Quuxplusone]

Summary

However, there's a problem on both GCC and Clang: they can't mangle
`__is_same(T,U)` if it appears anywhere that affects mangling. That's
a hard error. And it turns out that GCC puts dependent return types
into the mangling more aggressively than Clang, so for GCC's benefit
we need to avoid using raw `_IsSame` in the return type of
`swap(tuple&, tuple&)`. Therefore, make `__all` into a named type
instead of an alias.

[• Quuxplusone]

However, there's a problem on both GCC and Clang: they can't mangle
`__is_same(T,U)` if it appears anywhere that affects mangling. That's
a hard error. And it turns out that GCC puts dependent return types
into the mangling more aggressively than Clang, so we need to avoid
using raw `_IsSame` (or type aliases such as `__all` that expand into
`_IsSame`) in one return type, for GCC's benefit.

[ldionne]

libcxx/include/tuple

1120 ↗

(On Diff #391883)

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.

libcxx/include/type_traits

552–553

Do we support any compilers where this is false? I don't think so -- we could probably just drop the #if altogether.

[• Quuxplusone]

libcxx/include/tuple

1120 ↗

(On Diff #391883)

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.

libcxx/include/type_traits

552–553

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.

[• Quuxplusone]

Remove the #if entirely.

[ldionne]

libcxx/include/tuple

1120 ↗

(On Diff #391883)

The problem I have with fixing it locally is that we'll eventually hit the same problem if we use __all in another place. It seems cleaner to me to fix __all and add a comment explaining the GCC bug. Then, file a bug with GCC and reference it in the comment. That way, we pessimize __all but we reduce the likelihood of hitting the bug again to the maximum, and eventually we'll remove the pessimization when the GCC bug is fixed.

libcxx/include/type_traits

552–553

AFAICT we have CI for all our supported platforms. If CI is happy without #if, I think that means we're good. If not, then let's keep the #if.

[jloser]

libcxx/include/type_traits

571

Any appetite for adding a libcxx test for this behavior? It is a "private" type trait, but useful property in helping compile times, so I think a simple test doesn't hurt.

941

Why move this below the // is_class comment? It's a standalone helper type unrelated to is_class AFAICT, right? I recommend moving it above the // is_class comment. Maybe even add the // helper class: comment above the struct __two {char __lx[2];};.

Alternatively, revert the diff entirely as it's not needed for this patch. :)

[• Quuxplusone]

libcxx/include/type_traits

941

is_class is the first one who needs it, although yeah this was path-dependent and I should just move it below the old // helper class: comment. (Not sure how it wound up above it in the first place, and haven't looked.)

[• Quuxplusone]

Updated with @ldionne's suggestion. Let's see if buildkite is happy with this; if so, then I'm ready to land it and forget it. :)

[ldionne]

LGTM with comments.

libcxx/include/type_traits

564–565

I would remove that part of the comment -- I don't see what it adds.

Also, I would document that this _IsSame can't be used anywhere that affects mangling in order to save some cycles if someone encounters the same issue in the future.

583–585

I'm going to be really nitpicky and ask that you make this change in a separate NFC commit, no review needed. Just to keep reviews clean.

[• Quuxplusone]

libcxx/include/type_traits

564–565

I added the latter part of the comment because it wasn't obvious to me at first glance. The alternative (and the first thing I happened to think of) is that on those platforms we define _IsSame<T,U> as an alias for is_same<T,U> and just have to beware of using _IsSame anywhere that the difference would be observable.
However, I'll move that into the commit message; and move the "affects mangling" bit (partly) out of the commit message into the comment.

583–585

Done!

libcxx/include/__tuple

 struct __make_tuple_types<__tuple_types<_Types...>, _Ep, 0, true> {
   typedef _LIBCPP_NODEBUG __tuple_types<_Types...> type;
 };
 
 template <bool ..._Preds>
 struct __all_dummy;
 
 template <bool ..._Pred>
-using __all = _IsSame<__all_dummy<_Pred...>, __all_dummy<((void)_Pred, true)...>>;
+struct __all : _IsSame<__all_dummy<_Pred...>, __all_dummy<((void)_Pred, true)...>> {};
 
 struct __tuple_sfinae_base {
   template <template <class, class...> class _Trait,
             class ..._LArgs, class ..._RArgs>
   static auto __do_test(__tuple_types<_LArgs...>, __tuple_types<_RArgs...>)
     -> __all<typename enable_if<_Trait<_LArgs, _RArgs>::value, bool>::type{true}...>;
   template <template <class...> class>
   static auto __do_test(...) -> false_type;

libcxx/include/type_traits

 template <class _If, class _Then>
     struct _LIBCPP_TEMPLATE_VIS conditional<false, _If, _Then> {typedef _Then type;};
 
 #if _LIBCPP_STD_VER > 11
 template <bool _Bp, class _If, class _Then> using conditional_t = typename conditional<_Bp, _If, _Then>::type;
 #endif
 
 // is_same
 
-#if __has_keyword(__is_same)
-
 template <class _Tp, class _Up>
 struct _LIBCPP_TEMPLATE_VIS is_same : _BoolConstant<__is_same(_Tp, _Up)> { };
 
 #if _LIBCPP_STD_VER > 14
 template <class _Tp, class _Up>
 inline constexpr bool is_same_v = __is_same(_Tp, _Up);
 #endif
 
-#else
-
-template <class _Tp, class _Up> struct _LIBCPP_TEMPLATE_VIS is_same           : public false_type {};
-template <class _Tp>            struct _LIBCPP_TEMPLATE_VIS is_same<_Tp, _Tp> : public true_type {};
+// _IsSame<T,U> has the same effect as is_same<T,U> but instantiates fewer types:
+// is_same<A,B> and is_same<C,D> are guaranteed to be different types, but
+// _IsSame<A,B> and _IsSame<C,D> are the same type (namely, false_type).
+// Neither GCC nor Clang can mangle the __is_same builtin, so _IsSame
+// mustn't be directly used anywhere that contributes to name-mangling
+// (such as in a dependent return type).
 
-#if _LIBCPP_STD_VER > 14
 template <class _Tp, class _Up>
-inline constexpr bool is_same_v = is_same<_Tp, _Up>::value;
-#endif
-
-#endif // __is_same
+using _IsSame = _BoolConstant<__is_same(_Tp, _Up)>;
 
 template <class _Tp, class _Up>
-using _IsSame = _BoolConstant<
-#ifdef __clang__
-    __is_same(_Tp, _Up)
-#else
-    is_same<_Tp, _Up>::value
-#endif
->;
-
-template <class _Tp, class _Up>
-using _IsNotSame = _BoolConstant<
-#ifdef __clang__
-    !__is_same(_Tp, _Up)
-#else
-    !is_same<_Tp, _Up>::value
-#endif
->;
-
+using _IsNotSame = _BoolConstant<!__is_same(_Tp, _Up)>;
 
 template <class _Tp>
 using __test_for_primary_template = __enable_if_t<
     _IsSame<_Tp, typename _Tp::__primary_template>::value
   >;
 template <class _Tp>
 using __is_primary_template = _IsValidExpansion<
     __test_for_primary_template, _Tp
   >;
 
 // helper class
 
 struct __two {char __lx[2];};
 
 // is_const
 
 #if __has_keyword(__is_const)
 
 template <class _Tp>
 struct _LIBCPP_TEMPLATE_VIS is_const : _BoolConstant<__is_const(_Tp)> { };
 

 
 #if _LIBCPP_STD_VER > 14
 template <class _Tp>
 inline constexpr bool is_union_v = is_union<_Tp>::value;
 #endif
 
 // is_class
 
 #if __has_feature(is_class) || defined(_LIBCPP_COMPILER_GCC)
 
 template <class _Tp> struct _LIBCPP_TEMPLATE_VIS is_class
     : public integral_constant<bool, __is_class(_Tp)> {};
 
 #else
 
 namespace __is_class_imp
 {