[NFC][libc++] Removes concepts tests.

[Mordante]

Summary

Concepts should be supported by all supported compilers. This is
probably a left over from an older version of the expected patch.

[philnik]

No, not all compilers support P0848R3, which is required for the triviality.

[Mordante]

In D140632#4015448, @philnik wrote:

No, not all compilers support P0848R3, which is required for the triviality.

Which compilers we support don't support this change? The CI is green https://buildkite.com/llvm-project/libcxx-ci/builds/16465

[philnik]

In D140632#4015497, @Mordante wrote:

In D140632#4015448, @philnik wrote:

No, not all compilers support P0848R3, which is required for the triviality.

Which compilers we support don't support this change? The CI is green https://buildkite.com/llvm-project/libcxx-ci/builds/16465

Clang trunk doesn't claim support: https://godbolt.org/z/hes3nah8s. I'm actually quite surprised this doesn't break the CI. I remember having problems with at least Clang 15 when trying to use conditionally trivial special member functions.

[h-vetinari]

Clang trunk doesn't claim support: https://godbolt.org/z/hes3nah8s. I'm actually quite surprised this doesn't break the CI. I remember having problems with at least Clang 15 when trying to use conditionally trivial special member functions.

See https://reviews.llvm.org/D128619; it's implemented, but there are still a large number of open concept issues so the feature macro hasn't been set.

[Mordante]

In D140632#4016552, @h-vetinari wrote:

Clang trunk doesn't claim support: https://godbolt.org/z/hes3nah8s. I'm actually quite surprised this doesn't break the CI. I remember having problems with at least Clang 15 when trying to use conditionally trivial special member functions.

See https://reviews.llvm.org/D128619; it's implemented, but there are still a large number of open concept issues so the feature macro hasn't been set.

That might be true, but this part seems to work. So I really would like to land this.
We normally don't use feature-test macros in our test, instead we define a helper macro which we use in the tests.
Since it works it makes little sense to add a helper macro to do the right thing.

[philnik]

In D140632#4018515, @Mordante wrote:

In D140632#4016552, @h-vetinari wrote:

Clang trunk doesn't claim support: https://godbolt.org/z/hes3nah8s. I'm actually quite surprised this doesn't break the CI. I remember having problems with at least Clang 15 when trying to use conditionally trivial special member functions.

See https://reviews.llvm.org/D128619; it's implemented, but there are still a large number of open concept issues so the feature macro hasn't been set.

That might be true, but this part seems to work. So I really would like to land this.
We normally don't use feature-test macros in our test, instead we define a helper macro which we use in the tests.
Since it works it makes little sense to add a helper macro to do the right thing., s

I'm not a huge fan of enabling the tests when we know that the implementation is still incomplete and buggy (which is the reason the FTM hasn't been bumped yet). Though I guess we do that already in other places, so I guess this looks good to me. We should reconsider that position though IMO, since it results in workarounds where nobody knows in the end why they existed in the first place.

[Mordante]

In D140632#4020164, @philnik wrote:

In D140632#4018515, @Mordante wrote:

That might be true, but this part seems to work. So I really would like to land this.
We normally don't use feature-test macros in our test, instead we define a helper macro which we use in the tests.
Since it works it makes little sense to add a helper macro to do the right thing., s

I'm not a huge fan of enabling the tests when we know that the implementation is still incomplete and buggy (which is the reason the FTM hasn't been bumped yet). Though I guess we do that already in other places, so I guess this looks good to me. We should reconsider that position though IMO, since it results in workarounds where nobody knows in the end why they existed in the first place.

I'm not against partly disabling tests when they don't work, but then we should use a libc++ specific test macro so we can enable/disable the feature based on our selection criteria. This is exactly how we have done it in the past, usually they were indeed based on a FTM but not always.

Thanks for the review!

libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.copy.pass.cpp

 static_assert(std::is_copy_constructible_v<std::expected<CopyableNonTrivial, CopyableNonTrivial>>);
 static_assert(!std::is_copy_constructible_v<std::expected<NonCopyable, int>>);
 static_assert(!std::is_copy_constructible_v<std::expected<int, NonCopyable>>);
 static_assert(!std::is_copy_constructible_v<std::expected<NonCopyable, NonCopyable>>);
 
 // Test: This constructor is trivial if
 // - is_trivially_copy_constructible_v<T> is true and
 // - is_trivially_copy_constructible_v<E> is true.
-#if __cpp_concepts >= 202002
 static_assert(std::is_trivially_copy_constructible_v<std::expected<int, int>>);
-#endif
 static_assert(!std::is_trivially_copy_constructible_v<std::expected<CopyableNonTrivial, int>>);
 static_assert(!std::is_trivially_copy_constructible_v<std::expected<int, CopyableNonTrivial>>);
 static_assert(!std::is_trivially_copy_constructible_v<std::expected<CopyableNonTrivial, CopyableNonTrivial>>);
 
 constexpr bool test() {
   // copy the value non-trivial
   {
     const std::expected<CopyableNonTrivial, int> e1(5);

libcxx/test/std/utilities/expected/expected.expected/ctor/ctor.move.pass.cpp

 static_assert(std::is_move_constructible_v<std::expected<MovableNonTrivial, MovableNonTrivial>>);
 static_assert(!std::is_move_constructible_v<std::expected<NonMovable, int>>);
 static_assert(!std::is_move_constructible_v<std::expected<int, NonMovable>>);
 static_assert(!std::is_move_constructible_v<std::expected<NonMovable, NonMovable>>);
 
 // Test: This constructor is trivial if
 // - is_trivially_move_constructible_v<T> is true and
 // - is_trivially_move_constructible_v<E> is true.
-#if __cpp_concepts >= 202002
 static_assert(std::is_trivially_move_constructible_v<std::expected<int, int>>);
-#endif
 static_assert(!std::is_trivially_move_constructible_v<std::expected<MovableNonTrivial, int>>);
 static_assert(!std::is_trivially_move_constructible_v<std::expected<int, MovableNonTrivial>>);
 static_assert(!std::is_trivially_move_constructible_v<std::expected<MovableNonTrivial, MovableNonTrivial>>);
 
 // Test: The exception specification is equivalent to
 // is_nothrow_move_constructible_v<T> && is_nothrow_move_constructible_v<E>.
 static_assert(std::is_nothrow_move_constructible_v<std::expected<int, int>>);
 static_assert(!std::is_nothrow_move_constructible_v<std::expected<MoveMayThrow, int>>);

libcxx/test/std/utilities/expected/expected.expected/dtor.pass.cpp

 //
 // Remarks: If is_trivially_destructible_v<T> is true, and is_trivially_destructible_v<E> is true,
 // then this destructor is a trivial destructor.
 
 #include <cassert>
 #include <expected>
 #include <type_traits>
 #include <utility>
+#include <memory>
 
 #include "test_macros.h"
 
 // Test Remarks: If is_trivially_destructible_v<T> is true, and is_trivially_destructible_v<E> is true,
 // then this destructor is a trivial destructor.
 struct NonTrivial {
   ~NonTrivial() {}
 };
 
-#if __cpp_concepts >= 202002
 static_assert(std::is_trivially_destructible_v<std::expected<int, int>>);
-#endif
 static_assert(!std::is_trivially_destructible_v<std::expected<NonTrivial, int>>);
 static_assert(!std::is_trivially_destructible_v<std::expected<int, NonTrivial>>);
 static_assert(!std::is_trivially_destructible_v<std::expected<NonTrivial, NonTrivial>>);
 
 struct TrackedDestroy {
   bool& destroyed;
   constexpr TrackedDestroy(bool& b) : destroyed(b) {}
   constexpr ~TrackedDestroy() { destroyed = true; }
// has error
     { [[maybe_unused]] std::expected<TrackedDestroy, TrackedDestroy> e(std::unexpect, errorDestroyed); }
     assert(errorDestroyed);
   }
 
   return true;
 }
 
 int main(int, char**) {
+  std::expected<std::unique_ptr<int>, int> a = std::make_unique<int>(42);
+
   test();
   static_assert(test());
   return 0;
 }

libcxx/test/std/utilities/expected/expected.void/ctor/ctor.copy.pass.cpp

 };
 
 // Test: This constructor is defined as deleted unless is_copy_constructible_v<E> is true.
 static_assert(std::is_copy_constructible_v<std::expected<void, int>>);
 static_assert(std::is_copy_constructible_v<std::expected<void, CopyableNonTrivial>>);
 static_assert(!std::is_copy_constructible_v<std::expected<void, NonCopyable>>);
 
 // Test: This constructor is trivial if is_trivially_copy_constructible_v<E> is true.
-#if __cpp_concepts >= 202002
 static_assert(std::is_trivially_copy_constructible_v<std::expected<void, int>>);
-#endif
 static_assert(!std::is_trivially_copy_constructible_v<std::expected<void, CopyableNonTrivial>>);
 
 constexpr bool test() {
   // copy the error non-trivial
   {
     const std::expected<void, CopyableNonTrivial> e1(std::unexpect, 5);
     auto e2 = e1;
     assert(!e2.has_value());

libcxx/test/std/utilities/expected/expected.void/ctor/ctor.move.pass.cpp

 
 // Test Constraints:
 // - is_move_constructible_v<E> is true.
 static_assert(std::is_move_constructible_v<std::expected<void, int>>);
 static_assert(std::is_move_constructible_v<std::expected<void, MovableNonTrivial>>);
 static_assert(!std::is_move_constructible_v<std::expected<void, NonMovable>>);
 
 // Test: This constructor is trivial if is_trivially_move_constructible_v<E> is true.
-#if __cpp_concepts >= 202002
 static_assert(std::is_trivially_move_constructible_v<std::expected<void, int>>);
-#endif
 static_assert(!std::is_trivially_move_constructible_v<std::expected<void, MovableNonTrivial>>);
 
 // Test: noexcept(is_nothrow_move_constructible_v<E>)
 static_assert(std::is_nothrow_move_constructible_v<std::expected<int, int>>);
 static_assert(!std::is_nothrow_move_constructible_v<std::expected<MoveMayThrow, int>>);
 static_assert(!std::is_nothrow_move_constructible_v<std::expected<int, MoveMayThrow>>);
 static_assert(!std::is_nothrow_move_constructible_v<std::expected<MoveMayThrow, MoveMayThrow>>);
 

libcxx/test/std/utilities/expected/expected.void/dtor.pass.cpp

 
 #include "test_macros.h"
 
 // Test Remarks: If is_trivially_destructible_v<E> is true, then this destructor is a trivial destructor.
 struct NonTrivial {
   ~NonTrivial() {}
 };
 
-#if __cpp_concepts >= 202002
 static_assert(std::is_trivially_destructible_v<std::expected<void, int>>);
-#endif
 static_assert(!std::is_trivially_destructible_v<std::expected<void, NonTrivial>>);
 
 struct TrackedDestroy {
   bool& destroyed;
   constexpr TrackedDestroy(bool& b) : destroyed(b) {}
   constexpr ~TrackedDestroy() { destroyed = true; }
 };