[libc++][RFC] Use type traits builtins directly in <type_traits> and <concepts> #114840

philnik777 · 2024-11-04T17:57:41Z

Using the builtins directly is quite a bit faster in my tests (at least 2x) and avoids some transitive includes. The main reason to not use them throughout the library is that GCC doesn't mangle the builtins, which means that they can't be used unconditionally.

llvmbot · 2024-11-04T17:58:17Z

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

Using the builtins directly is quite a bit faster in my tests (at least 2x) and avoids some transitive includes. The main reason to not use them throughout the library is that GCC doesn't mangle the builtins, which means that they can't be used unconditionally.

Patch is 33.47 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/114840.diff

32 Files Affected:

(modified) libcxx/include/__concepts/class_or_enum.h (+1-5)
(modified) libcxx/include/__concepts/constructible.h (+1-2)
(modified) libcxx/include/__concepts/convertible_to.h (+1-1)
(modified) libcxx/include/__concepts/derived_from.h (+1-3)
(modified) libcxx/include/__concepts/different_from.h (+1-2)
(modified) libcxx/include/__concepts/movable.h (+1-2)
(modified) libcxx/include/__concepts/same_as.h (+1-2)
(modified) libcxx/include/__concepts/swappable.h (+1-2)
(modified) libcxx/include/__type_traits/can_extract_key.h (+4-6)
(modified) libcxx/include/__type_traits/common_reference.h (+7-11)
(modified) libcxx/include/__type_traits/common_type.h (+2-4)
(modified) libcxx/include/__type_traits/datasizeof.h (-2)
(modified) libcxx/include/__type_traits/decay.h (+5-8)
(modified) libcxx/include/__type_traits/is_always_bitcastable.h (+4-8)
(modified) libcxx/include/__type_traits/is_destructible.h (+2-4)
(modified) libcxx/include/__type_traits/is_equality_comparable.h (+2-4)
(modified) libcxx/include/__type_traits/is_execution_policy.h (+2-4)
(modified) libcxx/include/__type_traits/is_floating_point.h (+1-2)
(modified) libcxx/include/__type_traits/is_integral.h (+1-2)
(modified) libcxx/include/__type_traits/is_pointer.h (+1-2)
(modified) libcxx/include/__type_traits/is_reference_wrapper.h (+1-2)
(modified) libcxx/include/__type_traits/is_referenceable.h (+1-3)
(modified) libcxx/include/__type_traits/is_scalar.h (+2-4)
(modified) libcxx/include/__type_traits/is_trivially_assignable.h (-1)
(modified) libcxx/include/__type_traits/is_trivially_lexicographically_comparable.h (+1-3)
(modified) libcxx/include/__type_traits/is_trivially_relocatable.h (+1-3)
(modified) libcxx/include/__type_traits/make_32_64_or_128_bit.h (+1-2)
(modified) libcxx/include/__type_traits/make_signed.h (+2-4)
(modified) libcxx/include/__type_traits/make_unsigned.h (+2-4)
(modified) libcxx/include/__type_traits/remove_cvref.h (-2)
(modified) libcxx/include/__type_traits/underlying_type.h (+2-3)
(modified) libcxx/include/type_traits (+1)

diff --git a/libcxx/include/__concepts/class_or_enum.h b/libcxx/include/__concepts/class_or_enum.h
index 2739e31e14ba65..053aff09bfcbdf 100644
--- a/libcxx/include/__concepts/class_or_enum.h
+++ b/libcxx/include/__concepts/class_or_enum.h
@@ -10,10 +10,6 @@
 #define _LIBCPP___CONCEPTS_CLASS_OR_ENUM_H
 
 #include <__config>
-#include <__type_traits/is_class.h>
-#include <__type_traits/is_enum.h>
-#include <__type_traits/is_union.h>
-#include <__type_traits/remove_cvref.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -26,7 +22,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 // Whether a type is a class type or enumeration type according to the Core wording.
 
 template <class _Tp>
-concept __class_or_enum = is_class_v<_Tp> || is_union_v<_Tp> || is_enum_v<_Tp>;
+concept __class_or_enum = __is_class(_Tp) || __is_union(_Tp) || __is_enum(_Tp);
 
 #endif // _LIBCPP_STD_VER >= 20
 
diff --git a/libcxx/include/__concepts/constructible.h b/libcxx/include/__concepts/constructible.h
index 835a44429c092f..395d5cb31592e5 100644
--- a/libcxx/include/__concepts/constructible.h
+++ b/libcxx/include/__concepts/constructible.h
@@ -12,7 +12,6 @@
 #include <__concepts/convertible_to.h>
 #include <__concepts/destructible.h>
 #include <__config>
-#include <__type_traits/is_constructible.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -24,7 +23,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 
 // [concept.constructible]
 template <class _Tp, class... _Args>
-concept constructible_from = destructible<_Tp> && is_constructible_v<_Tp, _Args...>;
+concept constructible_from = destructible<_Tp> && __is_constructible(_Tp, _Args...);
 
 // [concept.default.init]
 
diff --git a/libcxx/include/__concepts/convertible_to.h b/libcxx/include/__concepts/convertible_to.h
index 6d5b6c1268d5d2..9646789d8a1eaa 100644
--- a/libcxx/include/__concepts/convertible_to.h
+++ b/libcxx/include/__concepts/convertible_to.h
@@ -24,7 +24,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 // [concept.convertible]
 
 template <class _From, class _To>
-concept convertible_to = is_convertible_v<_From, _To> && requires { static_cast<_To>(std::declval<_From>()); };
+concept convertible_to = __is_convertible(_From, _To) && requires { static_cast<_To>(std::declval<_From>()); };
 
 #endif // _LIBCPP_STD_VER >= 20
 
diff --git a/libcxx/include/__concepts/derived_from.h b/libcxx/include/__concepts/derived_from.h
index 9875faee81b901..d3775c5eb13915 100644
--- a/libcxx/include/__concepts/derived_from.h
+++ b/libcxx/include/__concepts/derived_from.h
@@ -10,8 +10,6 @@
 #define _LIBCPP___CONCEPTS_DERIVED_FROM_H
 
 #include <__config>
-#include <__type_traits/is_base_of.h>
-#include <__type_traits/is_convertible.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -24,7 +22,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 // [concept.derived]
 
 template <class _Dp, class _Bp>
-concept derived_from = is_base_of_v<_Bp, _Dp> && is_convertible_v<const volatile _Dp*, const volatile _Bp*>;
+concept derived_from = __is_base_of(_Bp, _Dp) && __is_convertible(const volatile _Dp*, const volatile _Bp*);
 
 #endif // _LIBCPP_STD_VER >= 20
 
diff --git a/libcxx/include/__concepts/different_from.h b/libcxx/include/__concepts/different_from.h
index fd31f6e25805d3..abf56f46e8e6bf 100644
--- a/libcxx/include/__concepts/different_from.h
+++ b/libcxx/include/__concepts/different_from.h
@@ -11,7 +11,6 @@
 
 #include <__concepts/same_as.h>
 #include <__config>
-#include <__type_traits/remove_cvref.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -22,7 +21,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 #if _LIBCPP_STD_VER >= 20
 
 template <class _Tp, class _Up>
-concept __different_from = !same_as<remove_cvref_t<_Tp>, remove_cvref_t<_Up>>;
+concept __different_from = !same_as<__remove_cvref(_Tp), __remove_cvref(_Up)>;
 
 #endif // _LIBCPP_STD_VER >= 20
 
diff --git a/libcxx/include/__concepts/movable.h b/libcxx/include/__concepts/movable.h
index bc5b9d767c6a51..584e7196e90112 100644
--- a/libcxx/include/__concepts/movable.h
+++ b/libcxx/include/__concepts/movable.h
@@ -13,7 +13,6 @@
 #include <__concepts/constructible.h>
 #include <__concepts/swappable.h>
 #include <__config>
-#include <__type_traits/is_object.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -26,7 +25,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 // [concepts.object]
 
 template <class _Tp>
-concept movable = is_object_v<_Tp> && move_constructible<_Tp> && assignable_from<_Tp&, _Tp> && swappable<_Tp>;
+concept movable = __is_object(_Tp) && move_constructible<_Tp> && assignable_from<_Tp&, _Tp> && swappable<_Tp>;
 
 #endif // _LIBCPP_STD_VER >= 20
 
diff --git a/libcxx/include/__concepts/same_as.h b/libcxx/include/__concepts/same_as.h
index 4241131c70c1f4..e0d055cb1844ae 100644
--- a/libcxx/include/__concepts/same_as.h
+++ b/libcxx/include/__concepts/same_as.h
@@ -10,7 +10,6 @@
 #define _LIBCPP___CONCEPTS_SAME_AS_H
 
 #include <__config>
-#include <__type_traits/is_same.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -23,7 +22,7 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 // [concept.same]
 
 template <class _Tp, class _Up>
-concept __same_as_impl = _IsSame<_Tp, _Up>::value;
+concept __same_as_impl = __is_same(_Tp, _Up);
 
 template <class _Tp, class _Up>
 concept same_as = __same_as_impl<_Tp, _Up> && __same_as_impl<_Up, _Tp>;
diff --git a/libcxx/include/__concepts/swappable.h b/libcxx/include/__concepts/swappable.h
index 985c733021a0d7..a62fcd473a7e1c 100644
--- a/libcxx/include/__concepts/swappable.h
+++ b/libcxx/include/__concepts/swappable.h
@@ -18,7 +18,6 @@
 #include <__type_traits/extent.h>
 #include <__type_traits/is_nothrow_assignable.h>
 #include <__type_traits/is_nothrow_constructible.h>
-#include <__type_traits/remove_cvref.h>
 #include <__utility/exchange.h>
 #include <__utility/forward.h>
 #include <__utility/move.h>
@@ -46,7 +45,7 @@ void swap(_Tp&, _Tp&) = delete;
 // clang-format off
 template <class _Tp, class _Up>
 concept __unqualified_swappable_with =
-    (__class_or_enum<remove_cvref_t<_Tp>> || __class_or_enum<remove_cvref_t<_Up>>) &&
+    (__class_or_enum<__remove_cvref(_Tp)> || __class_or_enum<__remove_cvref(_Up)>) &&
     requires(_Tp&& __t, _Up&& __u) {
         swap(std::forward<_Tp>(__t), std::forward<_Up>(__u));
     };
diff --git a/libcxx/include/__type_traits/can_extract_key.h b/libcxx/include/__type_traits/can_extract_key.h
index b8359d07088104..0685a674311e2b 100644
--- a/libcxx/include/__type_traits/can_extract_key.h
+++ b/libcxx/include/__type_traits/can_extract_key.h
@@ -13,7 +13,6 @@
 #include <__fwd/pair.h>
 #include <__type_traits/conditional.h>
 #include <__type_traits/integral_constant.h>
-#include <__type_traits/is_same.h>
 #include <__type_traits/remove_const.h>
 #include <__type_traits/remove_const_ref.h>
 
@@ -29,19 +28,18 @@ struct __extract_key_self_tag {};
 struct __extract_key_first_tag {};
 
 template <class _ValTy, class _Key, class _RawValTy = __remove_const_ref_t<_ValTy> >
-struct __can_extract_key
-    : __conditional_t<_IsSame<_RawValTy, _Key>::value, __extract_key_self_tag, __extract_key_fail_tag> {};
+struct __can_extract_key : __conditional_t<__is_same(_RawValTy, _Key), __extract_key_self_tag, __extract_key_fail_tag> {
+};
 
 template <class _Pair, class _Key, class _First, class _Second>
 struct __can_extract_key<_Pair, _Key, pair<_First, _Second> >
-    : __conditional_t<_IsSame<__remove_const_t<_First>, _Key>::value, __extract_key_first_tag, __extract_key_fail_tag> {
-};
+    : __conditional_t<__is_same(__remove_const_t<_First>, _Key), __extract_key_first_tag, __extract_key_fail_tag> {};
 
 // __can_extract_map_key uses true_type/false_type instead of the tags.
 // It returns true if _Key != _ContainerValueTy (the container is a map not a set)
 // and _ValTy == _Key.
 template <class _ValTy, class _Key, class _ContainerValueTy, class _RawValTy = __remove_const_ref_t<_ValTy> >
-struct __can_extract_map_key : integral_constant<bool, _IsSame<_RawValTy, _Key>::value> {};
+struct __can_extract_map_key : integral_constant<bool, __is_same(_RawValTy, _Key)> {};
 
 // This specialization returns __extract_key_fail_tag for non-map containers
 // because _Key == _ContainerValueTy
diff --git a/libcxx/include/__type_traits/common_reference.h b/libcxx/include/__type_traits/common_reference.h
index c802902eb19fc3..bb09070d363105 100644
--- a/libcxx/include/__type_traits/common_reference.h
+++ b/libcxx/include/__type_traits/common_reference.h
@@ -13,10 +13,6 @@
 #include <__type_traits/common_type.h>
 #include <__type_traits/copy_cv.h>
 #include <__type_traits/copy_cvref.h>
-#include <__type_traits/is_convertible.h>
-#include <__type_traits/is_reference.h>
-#include <__type_traits/remove_cv.h>
-#include <__type_traits/remove_cvref.h>
 #include <__type_traits/remove_reference.h>
 #include <__utility/declval.h>
 
@@ -59,7 +55,7 @@ using __cv_cond_res = __cond_res<__copy_cv_t<_Xp, _Yp>&, __copy_cv_t<_Yp, _Xp>&>
 template <class _Ap, class _Bp, class _Xp, class _Yp>
   requires
     requires { typename __cv_cond_res<_Xp, _Yp>; } &&
-    is_reference_v<__cv_cond_res<_Xp, _Yp>>
+    __is_reference(__cv_cond_res<_Xp, _Yp>)
 struct __common_ref<_Ap&, _Bp&, _Xp, _Yp> {
   using __type = __cv_cond_res<_Xp, _Yp>;
 };
@@ -75,8 +71,8 @@ using __common_ref_C = remove_reference_t<__common_ref_t<_Xp&, _Yp&>>&&;
 template <class _Ap, class _Bp, class _Xp, class _Yp>
   requires
     requires { typename __common_ref_C<_Xp, _Yp>; } &&
-    is_convertible_v<_Ap&&, __common_ref_C<_Xp, _Yp>> &&
-    is_convertible_v<_Bp&&, __common_ref_C<_Xp, _Yp>>
+    __is_convertible(_Ap&&, __common_ref_C<_Xp, _Yp>) &&
+    __is_convertible(_Bp&&, __common_ref_C<_Xp, _Yp>)
 struct __common_ref<_Ap&&, _Bp&&, _Xp, _Yp> {
   using __type = __common_ref_C<_Xp, _Yp>;
 };
@@ -92,7 +88,7 @@ using __common_ref_D = __common_ref_t<const _Tp&, _Up&>;
 template <class _Ap, class _Bp, class _Xp, class _Yp>
   requires
     requires { typename __common_ref_D<_Xp, _Yp>; } &&
-    is_convertible_v<_Ap&&, __common_ref_D<_Xp, _Yp>>
+    __is_convertible(_Ap&&, __common_ref_D<_Xp, _Yp>)
 struct __common_ref<_Ap&&, _Bp&, _Xp, _Yp> {
   using __type = __common_ref_D<_Xp, _Yp>;
 };
@@ -139,7 +135,7 @@ template <class _Tp, class _Up>
 struct common_reference<_Tp, _Up> : __common_reference_sub_bullet1<_Tp, _Up> {};
 
 template <class _Tp, class _Up>
-  requires is_reference_v<_Tp> && is_reference_v<_Up> && requires { typename __common_ref_t<_Tp, _Up>; }
+  requires(__is_reference(_Tp) && __is_reference(_Up) && requires { typename __common_ref_t<_Tp, _Up>; })
 struct __common_reference_sub_bullet1<_Tp, _Up> {
   using type = __common_ref_t<_Tp, _Up>;
 };
@@ -151,8 +147,8 @@ struct basic_common_reference {};
 
 template <class _Tp, class _Up>
 using __basic_common_reference_t =
-    typename basic_common_reference<remove_cvref_t<_Tp>,
-                                    remove_cvref_t<_Up>,
+    typename basic_common_reference<__remove_cvref(_Tp),
+                                    __remove_cvref(_Up),
                                     __xref<_Tp>::template __apply,
                                     __xref<_Up>::template __apply>::type;
 
diff --git a/libcxx/include/__type_traits/common_type.h b/libcxx/include/__type_traits/common_type.h
index ef542f8bccfe02..1962331573fe18 100644
--- a/libcxx/include/__type_traits/common_type.h
+++ b/libcxx/include/__type_traits/common_type.h
@@ -12,8 +12,6 @@
 #include <__config>
 #include <__type_traits/conditional.h>
 #include <__type_traits/decay.h>
-#include <__type_traits/is_same.h>
-#include <__type_traits/remove_cvref.h>
 #include <__type_traits/type_identity.h>
 #include <__type_traits/void_t.h>
 #include <__utility/declval.h>
@@ -48,7 +46,7 @@ struct __common_type3 {};
 // sub-bullet 4 - "if COND_RES(CREF(D1), CREF(D2)) denotes a type..."
 template <class _Tp, class _Up>
 struct __common_type3<_Tp, _Up, void_t<__cond_type<const _Tp&, const _Up&>>> {
-  using type = remove_cvref_t<__cond_type<const _Tp&, const _Up&>>;
+  using type = __remove_cvref(__cond_type<const _Tp&, const _Up&>);
 };
 
 template <class _Tp, class _Up, class = void>
@@ -96,7 +94,7 @@ struct _LIBCPP_TEMPLATE_VIS common_type<_Tp> : public common_type<_Tp, _Tp> {};
 // sub-bullet 1 - "If is_same_v<T1, D1> is false or ..."
 template <class _Tp, class _Up>
 struct _LIBCPP_TEMPLATE_VIS common_type<_Tp, _Up>
-    : __conditional_t<_IsSame<_Tp, __decay_t<_Tp> >::value && _IsSame<_Up, __decay_t<_Up> >::value,
+    : __conditional_t<__is_same(_Tp, __decay_t<_Tp>) && __is_same(_Up, __decay_t<_Up>),
                       __common_type2_imp<_Tp, _Up>,
                       common_type<__decay_t<_Tp>, __decay_t<_Up> > > {};
 
diff --git a/libcxx/include/__type_traits/datasizeof.h b/libcxx/include/__type_traits/datasizeof.h
index 0c1ed94f840294..54735cd52fdb59 100644
--- a/libcxx/include/__type_traits/datasizeof.h
+++ b/libcxx/include/__type_traits/datasizeof.h
@@ -11,8 +11,6 @@
 
 #include <__config>
 #include <__cstddef/size_t.h>
-#include <__type_traits/is_class.h>
-#include <__type_traits/is_final.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
diff --git a/libcxx/include/__type_traits/decay.h b/libcxx/include/__type_traits/decay.h
index 7412044f931796..dff6ecfc1d9ddf 100644
--- a/libcxx/include/__type_traits/decay.h
+++ b/libcxx/include/__type_traits/decay.h
@@ -12,12 +12,8 @@
 #include <__config>
 #include <__type_traits/add_pointer.h>
 #include <__type_traits/conditional.h>
-#include <__type_traits/is_array.h>
-#include <__type_traits/is_function.h>
 #include <__type_traits/is_referenceable.h>
-#include <__type_traits/remove_cv.h>
 #include <__type_traits/remove_extent.h>
-#include <__type_traits/remove_reference.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -35,25 +31,26 @@ struct decay {
 };
 
 #else
+// This implementation is only used with GCC, so we can use __remove_reference and __is_array directly
 template <class _Up, bool>
 struct __decay {
-  typedef _LIBCPP_NODEBUG __remove_cv_t<_Up> type;
+  typedef _LIBCPP_NODEBUG __remove_cv(_Up) type;
 };
 
 template <class _Up>
 struct __decay<_Up, true> {
 public:
   typedef _LIBCPP_NODEBUG
-      __conditional_t<is_array<_Up>::value,
+      __conditional_t<__is_array(_Up),
                       __add_pointer_t<__remove_extent_t<_Up> >,
-                      __conditional_t<is_function<_Up>::value, typename add_pointer<_Up>::type, __remove_cv_t<_Up> > >
+                      __conditional_t<__is_function(_Up), typename add_pointer<_Up>::type, __remove_cv(_Up)> >
           type;
 };
 
 template <class _Tp>
 struct _LIBCPP_TEMPLATE_VIS decay {
 private:
-  typedef _LIBCPP_NODEBUG __libcpp_remove_reference_t<_Tp> _Up;
+  typedef _LIBCPP_NODEBUG __remove_reference(_Tp) _Up;
 
 public:
   typedef _LIBCPP_NODEBUG typename __decay<_Up, __libcpp_is_referenceable<_Up>::value>::type type;
diff --git a/libcxx/include/__type_traits/is_always_bitcastable.h b/libcxx/include/__type_traits/is_always_bitcastable.h
index 5bc650b41358a8..76c4e8b99af350 100644
--- a/libcxx/include/__type_traits/is_always_bitcastable.h
+++ b/libcxx/include/__type_traits/is_always_bitcastable.h
@@ -12,10 +12,6 @@
 #include <__config>
 #include <__type_traits/integral_constant.h>
 #include <__type_traits/is_integral.h>
-#include <__type_traits/is_object.h>
-#include <__type_traits/is_same.h>
-#include <__type_traits/is_trivially_copyable.h>
-#include <__type_traits/remove_cv.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -31,13 +27,13 @@ _LIBCPP_BEGIN_NAMESPACE_STD
 // considered bit-castable.
 template <class _From, class _To>
 struct __is_always_bitcastable {
-  using _UnqualFrom = __remove_cv_t<_From>;
-  using _UnqualTo   = __remove_cv_t<_To>;
+  using _UnqualFrom = __remove_cv(_From);
+  using _UnqualTo   = __remove_cv(_To);
 
   // clang-format off
   static const bool value =
       // First, the simple case -- `From` and `To` are the same object type.
-      (is_same<_UnqualFrom, _UnqualTo>::value && is_trivially_copyable<_UnqualFrom>::value) ||
+      (__is_same(_UnqualFrom, _UnqualTo) && __is_trivially_copyable(_UnqualFrom)) ||
 
       // Beyond the simple case, we say that one type is "always bit-castable" to another if:
       // - (1) `From` and `To` have the same value representation, and in addition every possible value of `From` has
@@ -75,7 +71,7 @@ struct __is_always_bitcastable {
         sizeof(_From) == sizeof(_To) &&
         is_integral<_From>::value &&
         is_integral<_To>::value &&
-        !is_same<_UnqualTo, bool>::value
+        !__is_same(_UnqualTo, bool)
       );
   // clang-format on
 };
diff --git a/libcxx/include/__type_traits/is_destructible.h b/libcxx/include/__type_traits/is_destructible.h
index 3248b07d36ee67..650132854b438f 100644
--- a/libcxx/include/__type_traits/is_destructible.h
+++ b/libcxx/include/__type_traits/is_destructible.h
@@ -11,8 +11,6 @@
 
 #include <__config>
 #include <__type_traits/integral_constant.h>
-#include <__type_traits/is_function.h>
-#include <__type_traits/is_reference.h>
 #include <__type_traits/remove_all_extents.h>
 #include <__utility/declval.h>
 
@@ -71,13 +69,13 @@ template <class _Tp, bool>
 struct __destructible_false;
 
 template <class _Tp>
-struct __destructible_false<_Tp, false> : public __destructible_imp<_Tp, is_reference<_Tp>::value> {};
+struct __destructible_false<_Tp, false> : public __destructible_imp<_Tp, __is_reference(_Tp)> {};
 
 template <class _Tp>
 struct __destructible_false<_Tp, true> : public false_type {};
 
 template <class _Tp>
-struct is_destructible : public __destructible_false<_Tp, is_function<_Tp>::value> {};
+struct is_destructible : public __destructible_false<_Tp, __is_function(_Tp)> {};
 
 template <class _Tp>
 struct is_destructible<_Tp[]> : public false_type {};
diff --git a/libcxx/include/__type_traits/is_equality_comparable.h b/libcxx/include/__type_traits/is_equality_comparable.h
index 4397f743e5ee95..892e848da49760 100644
--- a/libcxx/include/__type_traits/is_equality_comparable.h
+++ b/libcxx/include/__type_traits/is_equality_comparable.h
@@ -13,7 +13,6 @@
 #include <__type_traits/enable_if.h>
 #include <__type_traits/integral_constant.h>
 #include <__type_traits/is_integral.h>
-#include <__type_traits/is_same.h>
 #include <__type_traits/is_signed.h>
 #include <__type_traits/is_void.h>
 #include <__type_traits/remove_cv.h>
@@ -64,7 +63,7 @@ template <class _Tp, class _Up>
 struct __libcpp_is_trivially_equality_comparable_impl<
     _Tp,
     _Up,
-    __enable_if_t<is_integral<_Tp>::value && is_integral<_Up>::value && !is_same<_Tp, _Up>::value &&
+    __enable_if_t<is_integral<_Tp>::value && is_integral<_Up>::value && !__is_same(_Tp, _Up) &&
                   is_signed<_Tp>::value == is_signed<_Up>::value && sizeof(_Tp) == sizeof(_Up)> > : true_type {};
 
 template <class _Tp>
@@ -76,8 +75,7 @@ struct __libcpp_is_trivially_equality_comparable_impl<_Tp*, _Up*>
     : integral_constant<
           bool,
           __is_equality_comparable<_Tp*, _Up*>::value &&
-              (is_same<__remove_cv_t<_Tp>, __remove_cv_t<_Up> >::value || is_void<_Tp>::value || is_void<_Up>::value)> {
-};
+              (__is_same(__remove_cv(_Tp), __remove_cv(_Up)) || is_void<_Tp>::value || is_void<_Up>::value)> {};
 
 template <class _Tp, class _Up>
 using __libcpp_is_trivially_equality_comparable =
diff --git a/libcxx/include/__type_traits/is_execution_policy.h b/libcxx/include/__type_traits/is_execution_policy.h
index 6884f17ba16c88..15103165122616 100644
--- a/libcxx/include/__type_traits/is_execution_policy.h
+++ b/libcxx/include/__type_traits/is_execution_policy.h
@@ -10,7 +10,6 @@
 #define _LIBCPP___TYPE_TRAITS_IS_EXECUTION_POLICY_H
 
 #include <__config>
-#include <__type_traits/remove_cvref.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #  pragma GCC system_header
@@ -27,14 +26,13 @@ template <class>
 inline constexpr bool __is_unsequenced_execution_policy_impl = false;
 
 template <class _Tp>
-inline constexpr bool __is_unsequenced_execution_policy_v =
-    __is_unsequenced_execution_policy_impl<__remove_cvref_t<_Tp>>;
+inline constexpr bool __is_unsequenced_execution_policy_v = __is_unsequenced_execution_policy_impl<__remove_cvref(_Tp)>;
 
 template <class>
 inline constexpr bool __is_parallel_execution_policy_impl = false;
...
[truncated]

ldionne

Did you plan on drawing a line where the builtins should be used and where the normal _v type traits should be used? For example, using the builtins directly from within __concepts/ and __type_traits/, but the libc++ API from everywhere else?

I don't want to unnecessarily make this into a policy discussion because I don't think it's important enough to mandate that, but at the same time this will lead to some inconsistencies unless we have something written down we can refer to.

If the plan is to use builtins from anywhere in the codebase, I have some reservations about that because it might tie us a bit too closely to the specific compilers we support today. There's also other downsides like the inability to work around a compiler bug in a trait, which we've sometimes done in the libc++ version of the trait and has proven useful while we wait for the compiler to be fixed.

philnik777 · 2024-11-16T13:32:29Z

Did you plan on drawing a line where the builtins should be used and where the normal _v type traits should be used? For example, using the builtins directly from within __concepts/ and __type_traits/, but the libc++ API from everywhere else?

I think that is the line I'd want to draw, yes. I don't know whether we might want to extend it in the future, but so far that's not my plan.

I don't want to unnecessarily make this into a policy discussion because I don't think it's important enough to mandate that, but at the same time this will lead to some inconsistencies unless we have something written down we can refer to.

I'll try to write something up.

If the plan is to use builtins from anywhere in the codebase, I have some reservations about that because it might tie us a bit too closely to the specific compilers we support today. There's also other downsides like the inability to work around a compiler bug in a trait, which we've sometimes done in the libc++ version of the trait and has proven useful while we wait for the compiler to be fixed.

…<concepts>

ldionne

I like this change a bit more now that you've added a section to the coding guidelines, but I still don't like it much. I think it increases the maintenance burden of the library and will lead to more inconsistency in the codebase (esp. for newcommers), and it's difficult to quantify how much impact this truly has on real world code.

FWIW, if (for example) we had a single header implementing all of type_traits, and we wanted to use builtins directly inside that header (and that header only), I'd have much less of a problem. We could have a fat comment at the top of that header explaining why we're using builtins throughout that file. But that property would be very local to that header only, so there would be less risk of introducing inconsistencies as the codebase evolves.

Given #121278, we also probably don't want to merge this right now anyway. I would suggest we re-evaluate this change again in the future based on what type_traits looks like then.

philnik777 requested a review from a team as a code owner November 4, 2024 17:57

llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Nov 4, 2024

ldionne reviewed Nov 4, 2024

View reviewed changes

philnik777 force-pushed the use_type_traits_builtins_directly branch from bc134c0 to d06fc33 Compare December 17, 2024 12:24

[libc++][RFC] Use type traits builtins directly in <type_traits> and …

fd90a9f

…<concepts>

philnik777 force-pushed the use_type_traits_builtins_directly branch from d06fc33 to fd90a9f Compare December 20, 2024 10:45

philnik777 mentioned this pull request Dec 29, 2024

Incorrect handling of incomplete types in std::derived_from #121278

Closed

ldionne requested changes Jan 8, 2025

View reviewed changes

philnik777 marked this pull request as draft January 8, 2025 21:54

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[libc++][RFC] Use type traits builtins directly in <type_traits> and <concepts> #114840

[libc++][RFC] Use type traits builtins directly in <type_traits> and <concepts> #114840

Uh oh!

philnik777 commented Nov 4, 2024

Uh oh!

llvmbot commented Nov 4, 2024

Uh oh!

ldionne left a comment

Uh oh!

philnik777 commented Nov 16, 2024

Uh oh!

ldionne left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[libc++][RFC] Use type traits builtins directly in <type_traits> and <concepts> #114840

Are you sure you want to change the base?

[libc++][RFC] Use type traits builtins directly in <type_traits> and <concepts> #114840

Uh oh!

Conversation

philnik777 commented Nov 4, 2024

Uh oh!

llvmbot commented Nov 4, 2024

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

philnik777 commented Nov 16, 2024

Uh oh!

ldionne left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants