[libc++] type_traits: fix short-circuiting in std::conjunction.

Replace the two-level implementation with a simpler one that directly subclasses
the predicates, avoiding the instantiation of the template to get the `type`
member in a situation where we should short-circuit. This prevents incorrect
diagnostics when the instantiated predicate contains a static assertion.

Add a test case that reproduced the previous problem. The existing test case
involving `HasNoValue` didn't catch the problem because `HasNoValue` was in the
final position. The bug comes up when the predicate that shouldn't be
instantiated is after the short-circuit position but there is more to follow,
because then `__conjunction_impl<False, BadPredicate, ...>` instantiates
`__conjunction_impl<BadPredicate, ...>` (in order to obtain its `type` member),
which in turn instantiates `BadPredicate` in order to obtain its `value` member.

In contrast the new implementation doesn't recurse in instantiation any further
than it needs to, because it doesn't require particular members of the recursive
case.

I've also updated the test cases for `std::disjunction` to match,
although it doesn't have the same particular bug (its implementation is
quite different).

Fixes #58490.

Reviewed By: #libc, ldionne, philnik

Spies: philnik, ldionne, libcxx-commits

Differential Revision: https://reviews.llvm.org/D136318

diff --git a/libcxx/include/__type_traits/conjunction.h b/libcxx/include/__type_traits/conjunction.h
index 45fe5cd..9715854 100644 (file)
--- a/libcxx/include/__type_traits/conjunction.h
+++ b/libcxx/include/__type_traits/conjunction.h
@@ -20,26 +20,6 @@
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
-#if _LIBCPP_STD_VER > 14
-
-template <class _Arg, class... _Args>
-struct __conjunction_impl {
-  using type = conditional_t<!bool(_Arg::value), _Arg, typename __conjunction_impl<_Args...>::type>;
-};
-
-template <class _Arg>
-struct __conjunction_impl<_Arg> {
-  using type = _Arg;
-};
-
-template <class... _Args>
-struct conjunction : __conjunction_impl<true_type, _Args...>::type {};
-
-template<class... _Args>
-inline constexpr bool conjunction_v = conjunction<_Args...>::value;
-
-#endif // _LIBCPP_STD_VER > 14
-
 template <class...>
 using __expand_to_true = true_type;
 
@@ -52,6 +32,22 @@ false_type __and_helper(...);
 template <class... _Pred>
 using _And _LIBCPP_NODEBUG = decltype(__and_helper<_Pred...>(0));
 
+#if _LIBCPP_STD_VER > 14
+
+template <class...>
+struct conjunction : true_type {};
+
+template <class _Arg>
+struct conjunction<_Arg> : _Arg {};
+
+template <class _Arg, class... _Args>
+struct conjunction<_Arg, _Args...> : conditional_t<!bool(_Arg::value), _Arg, conjunction<_Args...>> {};
+
+template <class... _Args>
+inline constexpr bool conjunction_v = conjunction<_Args...>::value;
+
+#endif // _LIBCPP_STD_VER > 14
+
 _LIBCPP_END_NAMESPACE_STD
 
 #endif // _LIBCPP___TYPE_TRAITS_CONJUNCTION_H

diff --git a/libcxx/test/std/utilities/meta/meta.logical/conjunction.compile.pass.cpp b/libcxx/test/std/utilities/meta/meta.logical/conjunction.compile.pass.cpp
index d5d3aa2..725eb5b 100644 (file)
--- a/libcxx/test/std/utilities/meta/meta.logical/conjunction.compile.pass.cpp
+++ b/libcxx/test/std/utilities/meta/meta.logical/conjunction.compile.pass.cpp
@@ -80,6 +80,13 @@ static_assert(std::is_base_of_v<std::false_type, std::conjunction<std::false_typ
 static_assert(!std::conjunction<std::false_type, HasNoValue>::value);
 static_assert(!std::conjunction_v<std::false_type, HasNoValue>);
 
+// Also check the case where HasNoValue is not the last in the list (https://llvm.org/PR584900).
+static_assert(!std::conjunction<std::false_type, HasNoValue, std::true_type>::value);
+static_assert(!std::conjunction_v<std::false_type, HasNoValue, std::true_type>);
+
+static_assert(!std::conjunction<std::false_type, HasNoValue, std::false_type>::value);
+static_assert(!std::conjunction_v<std::false_type, HasNoValue, std::false_type>);
+
 static_assert(std::conjunction<MyOtherSpecialTrueType>::value == -1);
 static_assert(std::conjunction_v<MyOtherSpecialTrueType>);
 

diff --git a/libcxx/test/std/utilities/meta/meta.logical/disjunction.compile.pass.cpp b/libcxx/test/std/utilities/meta/meta.logical/disjunction.compile.pass.cpp
index 2a6c920..43952f6 100644 (file)
--- a/libcxx/test/std/utilities/meta/meta.logical/disjunction.compile.pass.cpp
+++ b/libcxx/test/std/utilities/meta/meta.logical/disjunction.compile.pass.cpp
@@ -80,6 +80,13 @@ static_assert(std::is_base_of_v<std::true_type, std::disjunction<std::true_type,
 static_assert(std::disjunction<std::true_type, HasNoValue>::value);
 static_assert(std::disjunction_v<std::true_type, HasNoValue>);
 
+// Also check the case where HasNoValue is not the last in the list (https://llvm.org/PR584900).
+static_assert(std::disjunction<std::true_type, HasNoValue, std::true_type>::value);
+static_assert(std::disjunction_v<std::true_type, HasNoValue, std::true_type>);
+
+static_assert(std::disjunction<std::true_type, HasNoValue, std::false_type>::value);
+static_assert(std::disjunction_v<std::true_type, HasNoValue, std::false_type>);
+
 static_assert(std::disjunction<MySpecialTrueType>::value == -1);
 static_assert(std::disjunction_v<MySpecialTrueType>);