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

[Nikolas Klauser via libcxx-commits]

Author: Aaron Jacobs
Date: 2022-10-21T13:09:16+02:00
New Revision: a0549ee2a39013c6ff75b86cda9d4ccfadb0ab88

URL: https://github.com/llvm/llvm-project/commit/a0549ee2a39013c6ff75b86cda9d4ccfadb0ab88
DIFF: https://github.com/llvm/llvm-project/commit/a0549ee2a39013c6ff75b86cda9d4ccfadb0ab88.diff

LOG: [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

Added: 
    

Modified: 
    libcxx/include/__type_traits/conjunction.h
    libcxx/test/std/utilities/meta/meta.logical/conjunction.compile.pass.cpp
    libcxx/test/std/utilities/meta/meta.logical/disjunction.compile.pass.cpp

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__type_traits/conjunction.h b/libcxx/include/__type_traits/conjunction.h
index 45fe5cdca3514..9715854fa0af1 100644
--- 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 d5d3aa268cd76..725eb5b0dba58 100644
--- 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 2a6c920710f7f..43952f6c4a232 100644
--- 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>);