[libc++] Consistency in _LIBCPP_CLANG_VER tests in <type_traits>

[• Quuxplusone]

Summary

This came out of my review comments on D97283.

This patch re-enables the use of __is_fundamental, __is_signed, etc. on non-Clang compilers. Previously, when we found that a builtin didn't work on old Clangs, we had been reacting by limiting its use to new Clangs (i.e., we'd also stop using it on new GCCs and new MSVCs, just because of the old Clang bug). I claim that this was unintentional.

[zoecarver]

Thanks, Arthur. The tests (and regression tests) should ensure that these issues don't exist on GCC. Not sure how to test this on MSVC (but I think that's "OK" because we don't really test anything on MSVC).

libcxx/include/type_traits

1132–1134

Nit: I think this means we won't take this path on Clang 9.0.1 anymore (which is maybe OK).

[zoecarver]

Also, saying "style" in the commit message seems to indicate that this is a nfc. I would explicitly point out in the body of the commit message that this enables these builtins on other platforms.

[• Quuxplusone]

libcxx/include/type_traits

1132–1134

Yeah, I saw that and decided that it was OK, since any difference would be observed merely as a compile-speed pessimization on Clang 9.x compilers. I value consistency-of-syntax more than... that. :)

[zoecarver]

libcxx/include/type_traits

1132–1134

Fair enough. Especially given these paths won't have to exist much longer.

[• Quuxplusone]

Rebase on main after the landing of D97283.

[• Quuxplusone]

libcxx/include/type_traits

1465

FWIW: Reverted the addition of && _LIBCPP_CLANG_VER >= 1300 here, for consistency with how we do it in the other places whose #ifs I'm changing. The compiler-version-specific epicycles don't show up in the #else or #endif comments there.

[curdeius]

_LIBCPP_CLANG_VER isn't defined for Apple Clang, so branches that were previously disabled, are now enabled, hence the failures for is_pointer and is_unsigned.
An ugly quickfix would be to use (!defined(_LIBCPP_COMPILER_CLANG) || _LIBCPP_CLANG_VER >= N) instead of (!defined(_LIBCPP_CLANG_VER) || _LIBCPP_CLANG_VER >= N).
Cf. https://github.com/llvm/llvm-project/blob/main/libcxx/include/__config#L184-L188.
Another solution would be to define _LIBCPP_CLANG_VER also for Apple Clang, but that might be tricky.
WDYT?

Apart from that, I like the clean-up.

[• Quuxplusone]

_LIBCPP_CLANG_VER isn't defined for Apple Clang, so branches that were previously disabled, are now enabled

Oh that's just evil. So from our POV they have all the same bugs as Clang, but they pretend to be "not Clang"? :/ I guess that's the only simple approach given how their version numbering doesn't match Clang's.

It does seem like maybe we could keep a mapping like this in __config, but I don't know what the magic numbers would need to be. (And it would bit-rot as new Apple Clang versions were released.)

#if defined(__apple_build_version__)
 #if (__apple_build_version__ < 12340000)
 #define _LIBCPP_CLANG_VER 1200
 #elif (__apple_build_version__ < 12340000)
 #define _LIBCPP_CLANG_VER 1100
 #elif (__apple_build_version__ < 12340000)
 #define _LIBCPP_CLANG_VER 1000
 #else
 #define _LIBCPP_CLANG_VER 100
 #endif
#endif

Personally, I'm okay with your suggestion to use _LIBCPP_COMPILER_CLANG and will update the diff accordingly to poke buildkite. But I agree this has become a can of worms.

[• Quuxplusone]

Use _LIBCPP_COMPILER_CLANG.
Also, DeMorgan the condition so that it'll be easier to add && !(some-other-compiler) when we learn where else our tricks don't work.

[curdeius]

LGTM pending CI.
@ldionne, maybe you have an idea about what should be done for Apple Clang?

libcxx/include/type_traits

 _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_array_v
     = is_array<_Tp>::value;
 #endif
 
 #endif // __has_keyword(__is_array)
 
 // is_pointer
 
-// In clang 10.0.0 and earlier __is_pointer didn't work with Objective-C types.
-#if __has_keyword(__is_pointer) && _LIBCPP_CLANG_VER > 1000
+// Before Clang 11, __is_pointer didn't work for Objective-C types.
+#if __has_keyword(__is_pointer) && !(defined(_LIBCPP_COMPILER_CLANG) && _LIBCPP_CLANG_VER < 1100)
 
 template<class _Tp>
 struct _LIBCPP_TEMPLATE_VIS is_pointer : _BoolConstant<__is_pointer(_Tp)> { };
 
 #if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)
 template <class _Tp>
 _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_pointer_v = __is_pointer(_Tp);
 #endif

 #if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)
 template <class _Tp>
 _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_arithmetic_v
     = is_arithmetic<_Tp>::value;
 #endif
 
 // is_fundamental
 
-// In clang 9 and lower, this builtin did not work for nullptr_t. Additionally, in C++03 mode,
-// nullptr isn't defined by the compiler so, this builtin won't work.
-#if __has_keyword(__is_fundamental) && _LIBCPP_CLANG_VER > 900 && !defined(_LIBCPP_CXX03_LANG)
+// Before Clang 10, __is_fundamental didn't work for nullptr_t.
+// In C++03 nullptr_t is library-provided but must still count as "fundamental."
+#if __has_keyword(__is_fundamental) && !(defined(_LIBCPP_COMPILER_CLANG) && _LIBCPP_CLANG_VER < 1000) && !defined(_LIBCPP_CXX03_LANG)
 
 template<class _Tp>
 struct _LIBCPP_TEMPLATE_VIS is_fundamental : _BoolConstant<__is_fundamental(_Tp)> { };
 
 #if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)
 template <class _Tp>
 _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_fundamental_v = __is_fundamental(_Tp);
 #endif

 _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_fundamental_v
     = is_fundamental<_Tp>::value;
 #endif
 
 #endif // __has_keyword(__is_fundamental)
 
 // is_scalar
 
-// >= 11 because in C++03 nullptr isn't actually nullptr
+// In C++03 nullptr_t is library-provided but must still count as "scalar."
 #if __has_keyword(__is_scalar) && !defined(_LIBCPP_CXX03_LANG)
 
 template<class _Tp>
 struct _LIBCPP_TEMPLATE_VIS is_scalar : _BoolConstant<__is_scalar(_Tp)> { };
 
 #if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)
 template <class _Tp>
 _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_scalar_v = __is_scalar(_Tp);

 // type_identity
 #if _LIBCPP_STD_VER > 17
 template<class _Tp> struct type_identity { typedef _Tp type; };
 template<class _Tp> using type_identity_t = typename type_identity<_Tp>::type;
 #endif
 
 // is_signed
 
-// In clang 9 and earlier, this builtin did not work for floating points or enums
-#if __has_keyword(__is_signed) && _LIBCPP_CLANG_VER > 900
+// Before Clang 10, __is_signed didn't work for floating-point types or enums.
+#if __has_keyword(__is_signed) && !(defined(_LIBCPP_COMPILER_CLANG) && _LIBCPP_CLANG_VER < 1000)
 
 template<class _Tp>
 struct _LIBCPP_TEMPLATE_VIS is_signed : _BoolConstant<__is_signed(_Tp)> { };
 
 #if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)
 template <class _Tp>
 _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_signed_v = __is_signed(_Tp);
 #endif

 _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_signed_v
     = is_signed<_Tp>::value;
 #endif
 
 #endif // __has_keyword(__is_signed)
 
 // is_unsigned
 
-// Before clang 13, __is_unsigned returned true for enums with signed underlying type
-#if __has_keyword(__is_unsigned) && _LIBCPP_CLANG_VER >= 1300
+// Before Clang 13, __is_unsigned returned true for enums with signed underlying type.
+#if __has_keyword(__is_unsigned) && !(defined(_LIBCPP_COMPILER_CLANG) && _LIBCPP_CLANG_VER < 1300)
 
 template<class _Tp>
 struct _LIBCPP_TEMPLATE_VIS is_unsigned : _BoolConstant<__is_unsigned(_Tp)> { };
 
 #if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)
 template <class _Tp>
 _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_unsigned_v = __is_unsigned(_Tp);
 #endif
 
-#else // __has_keyword(__is_unsigned) && _LIBCPP_CLANG_VER >= 1300
+#else // __has_keyword(__is_unsigned)
 
 template <class _Tp, bool = is_integral<_Tp>::value>
 struct __libcpp_is_unsigned_impl : public _LIBCPP_BOOL_CONSTANT(_Tp(0) < _Tp(-1)) {};
 
 template <class _Tp>
 struct __libcpp_is_unsigned_impl<_Tp, false> : public false_type {};  // floating point
 
 template <class _Tp, bool = is_arithmetic<_Tp>::value>
 struct __libcpp_is_unsigned : public __libcpp_is_unsigned_impl<_Tp> {};
 
 template <class _Tp> struct __libcpp_is_unsigned<_Tp, false> : public false_type {};
 
 template <class _Tp> struct _LIBCPP_TEMPLATE_VIS is_unsigned : public __libcpp_is_unsigned<_Tp> {};
 
 #if _LIBCPP_STD_VER > 14 && !defined(_LIBCPP_HAS_NO_VARIABLE_TEMPLATES)
 template <class _Tp>
 _LIBCPP_INLINE_VAR _LIBCPP_CONSTEXPR bool is_unsigned_v
     = is_unsigned<_Tp>::value;
 #endif
 
-#endif // __has_keyword(__is_unsigned) && _LIBCPP_CLANG_VER >= 1300
+#endif // __has_keyword(__is_unsigned)
 
 // rank
 
 template <class _Tp> struct _LIBCPP_TEMPLATE_VIS rank
     : public integral_constant<size_t, 0> {};
 template <class _Tp> struct _LIBCPP_TEMPLATE_VIS rank<_Tp[]>
     : public integral_constant<size_t, rank<_Tp>::value + 1> {};
 template <class _Tp, size_t _Np> struct _LIBCPP_TEMPLATE_VIS rank<_Tp[_Np]>