[libc++] Use _If for conditional_t

[philnik777]

This avoids different instantiations when the if and else types are different, resulting in reduced memory use by the compiler.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

This avoids different instantiations when the if and else types are different, resulting in reduced memory use by the compiler.

---

Full diff: https://github.com/llvm/llvm-project/pull/96193.diff

1 Files Affected:

• (modified) libcxx/include/__type_traits/conditional.h (+4-5)

diff --git a/libcxx/include/__type_traits/conditional.h b/libcxx/include/__type_traits/conditional.h
index 5b5445a837427..7d5849ee824e3 100644
--- a/libcxx/include/__type_traits/conditional.h
+++ b/libcxx/include/__type_traits/conditional.h
@@ -44,15 +44,14 @@ struct _LIBCPP_TEMPLATE_VIS conditional<false, _If, _Then> {
   using type _LIBCPP_NODEBUG = _Then;
 };
 
+template <bool _Bp, class _IfRes, class _ElseRes>
+using __conditional_t _LIBCPP_NODEBUG = _If<_Bp, _IfRes, _ElseRes>;
+
 #if _LIBCPP_STD_VER >= 14
 template <bool _Bp, class _IfRes, class _ElseRes>
-using conditional_t _LIBCPP_NODEBUG = typename conditional<_Bp, _IfRes, _ElseRes>::type;
+using conditional_t _LIBCPP_NODEBUG = __conditional_t<_Bp, _IfRes, _ElseRes>;
 #endif
 
-// Helper so we can use "conditional_t" in all language versions.
-template <bool _Bp, class _If, class _Then>
-using __conditional_t _LIBCPP_NODEBUG = typename conditional<_Bp, _If, _Then>::type;
-
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___TYPE_TRAITS_CONDITIONAL_H

[zmodem]

We've run into compile errors after this. I'm not sure if this is an unintentional effect of this change, or if we're doing something wrong. The code looks like this:

  // This overload returns true_type for the trait below.
  // The conditional_t is to make the enabler type dependent.
  template <class Alloc,
            typename = std::enable_if_t<absl::is_trivially_relocatable<
                std::conditional_t<false, Alloc, value_type>>::value>>
  static std::true_type transfer_impl(Alloc*, slot_type* new_slot,
                                      slot_type* old_slot, Rank1) {

And now fails to compile:

../../third_party/libc++/src/include/__type_traits/enable_if.h:32:40: error: no type named 'type' in 'std::enable_if<false>'; 'enable_if' cannot be used to disable this declaration
   32 | using enable_if_t = typename enable_if<_Bp, _Tp>::type;
      |                                        ^~~
../../third_party/abseil-cpp/absl/container/internal/common_policy_traits.h:117:29: note: in instantiation of template type alias 'enable_if_t' requested here
  117 |             typename = std::enable_if_t<absl::is_trivially_relocatable<
      |                             ^
../../third_party/abseil-cpp/absl/container/internal/hash_policy_traits.h:33:29: note: in instantiation of template class 'absl::container_internal::common_policy_traits<absl::container_internal::FlatHashMapPolicy<sh::SpirvType, sh::SpirvTypeData>>' requested here
   33 | struct hash_policy_traits : common_policy_traits<Policy> {
      |                             ^
../../third_party/abseil-cpp/absl/container/internal/raw_hash_set.h:2346:30: note: in instantiation of template class 'absl::container_internal::hash_policy_traits<absl::container_internal::FlatHashMapPolicy<sh::SpirvType, sh::SpirvTypeData>>' requested here
 2346 |   using init_type = typename PolicyTraits::init_type;
      |                              ^
../../third_party/abseil-cpp/absl/container/internal/raw_hash_map.h:33:29: note: in instantiation of template class 'absl::container_internal::raw_hash_set<absl::container_internal::FlatHashMapPolicy<sh::SpirvType, sh::SpirvTypeData>, sh::SpirvTypeHash, std::equal_to<sh::SpirvType>, std::allocator<std::pair<const sh::SpirvType, sh::SpirvTypeData>>>' requested here
   33 | class raw_hash_map : public raw_hash_set<Policy, Hash, Eq, Alloc> {
      |                             ^
../../third_party/abseil-cpp/absl/container/flat_hash_map.h:126:14: note: in instantiation of template class 'absl::container_internal::raw_hash_map<absl::container_internal::FlatHashMapPolicy<sh::SpirvType, sh::SpirvTypeData>, sh::SpirvTypeHash, std::equal_to<sh::SpirvType>, std::allocator<std::pair<const sh::SpirvType, sh::SpirvTypeData>>>' requested here
  126 |     : public absl::container_internal::raw_hash_map<
      |              ^
../../third_party/angle/src/compiler/translator/spirv/BuildSPIRV.h:500:61: note: in instantiation of template class 'absl::flat_hash_map<sh::SpirvType, sh::SpirvTypeData, sh::SpirvTypeHash, std::equal_to<sh::SpirvType>>' requested here
  500 |     angle::HashMap<SpirvType, SpirvTypeData, SpirvTypeHash> mTypeMap;
      |                                                             ^

We can work around it by using conditional<>::type instead of conditional_t<>, but it seems wrong that they should behave differently.

[ldionne]

I think this patch made us non-conforming. Minimal reproducer:

#include <type_traits>

template <class T, class = std::enable_if_t<
  !std::is_same_v<int, std::conditional_t<false, T, int>>
>>
void f() { }

Godbolt: https://godbolt.org/z/jGxn3nhbz

@philnik777 , can you please revert this patch and add a new test case to avoid regressing this?

The issue is that the Standard specifies:

template< bool B, class T, class F >
using conditional_t = typename conditional<B,T,F>::type;

The new implementation does not satisfy this "equivalent to" property because in @zmodem's use case, typename conditional<false, Alloc, value_type>::type as a whole would be a dependent type since Alloc is a template parameter of the function. However, with our new implementation of conditional_t, we disregard the first type entirely since the boolean is false, and this makes the instantiation of conditional_t<B, T, F> "less lazy" than it used to be, which is technically non-conforming.

[d0k]

Went ahead and reverted this in 3748162, see discussion above.

[EricWF]

We should probably add a test for this.