[libc++][NFC] Use __construct_at and __destroy_at instead of using preprocessor conditionals

[philnik777]

No description provided.

[ldionne]

LGTM % CI.

[EricWF]

It's unfortunate that __construct_at appears here, but I think that suggests we're testing too much of the compiler message.

A test should only fail when the behavior under test no longer holds. Is the name part of the behavior under test?

[EricWF]

Why is using destroy_at better than simply invoking the destructor? One costs a lot less to instantiate.
It's not a customization point, right?

[EricWF]

Same question as below.

Are we required to use construct_at here? If not, it seems to be more expensive, and causes arguably poorer diagnostic messages.

[ldionne]

This is for constexpr-friendliness IIRC.

[github-actions]

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:

git-clang-format --diff 6bdeb53ed9ad85fc16f495120b4e2382c4bdaafa 18e2a699569190d4c360090d310dd4be7b02b770 -- libcxx/include/__memory/allocator_traits.h libcxx/include/optional libcxx/test/std/containers/sequences/vector/vector.cons/copy.move_only.verify.cpp libcxx/test/std/containers/sequences/vector/vector.modifiers/resize_not_move_insertable.verify.cpp

View the diff from clang-format here.

diff --git a/libcxx/include/__memory/allocator_traits.h b/libcxx/include/__memory/allocator_traits.h
index eea5ee973d..a2ce48b3fb 100644
--- a/libcxx/include/__memory/allocator_traits.h
+++ b/libcxx/include/__memory/allocator_traits.h
@@ -300,7 +300,7 @@ struct _LIBCPP_TEMPLATE_VIS allocator_traits
         __enable_if_t<!__has_construct<allocator_type, _Tp*, _Args...>::value> >
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX20
     static void construct(allocator_type&, _Tp* __p, _Args&&... __args) {
-        std::__construct_at(__p, std::forward<_Args>(__args)...);
+      std::__construct_at(__p, std::forward<_Args>(__args)...);
     }
 
     template <class _Tp, class =
@@ -315,7 +315,7 @@ struct _LIBCPP_TEMPLATE_VIS allocator_traits
         __enable_if_t<!__has_destroy<allocator_type, _Tp*>::value> >
     _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR_SINCE_CXX20
     static void destroy(allocator_type&, _Tp* __p) {
-        std::__destroy_at(__p);
+      std::__destroy_at(__p);
     }
 
     template <class _Ap = _Alloc, class =
diff --git a/libcxx/test/std/containers/sequences/vector/vector.cons/copy.move_only.verify.cpp b/libcxx/test/std/containers/sequences/vector/vector.cons/copy.move_only.verify.cpp
index af8ca01370..1179fde3f1 100644
--- a/libcxx/test/std/containers/sequences/vector/vector.cons/copy.move_only.verify.cpp
+++ b/libcxx/test/std/containers/sequences/vector/vector.cons/copy.move_only.verify.cpp
@@ -16,5 +16,6 @@
 
 void f() {
     std::vector<MoveOnly> v;
-    std::vector<MoveOnly> copy = v; // expected-error-re@* {{{{(no matching function for call to '__construct_at')|(call to deleted constructor of 'MoveOnly')}}}}
+    std::vector<MoveOnly> copy =
+        v; // expected-error-re@* {{{{(no matching function for call to '__construct_at')|(call to deleted constructor of 'MoveOnly')}}}}
 }

[ldionne]

@EricWF This is essentially a small simplification of the code -- we have to use construct_at and destroy_at for constexpr-friendliness, and we also like to do it for consistency everywhere. So this really just trades #if for an unconditional call to something that does the right thing under the hood, so I'd go ahead with this change.

[EricWF]

What does "constexpr friendlyness" mean?
Is there magic in construct_at?

Because it's just placement new.

If this adds constexpr friendlyness then doesn't in need more tests?