[libcxx-commits] [PATCH] D106916: [libc++] Handle arrays in std::destroy_at

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/__memory/construct_at.h:61
+#if _LIBCPP_STD_VER > 17
+template <class _Tp, class = void, class = _EnableIf<is_array_v<_Tp>>>
+_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17
----------------
Instead of using this `class = void` hack, could you just use
```
template <class _Tp, _EnableIf<!is_array_v<_Tp>, int> = 0>
```
in the first case and
```
template <class _Tp, _EnableIf<is_array_v<_Tp>, int> = 0>
```
in the second case?


================
Comment at: libcxx/test/std/utilities/memory/specialized.algorithms/specialized.destroy/destroy_n.pass.cpp:82-85
+    if (std::is_constant_evaluated())
+        return true;
+
+    {
----------------
I'd prefer to wrap each offending block in an `if`:
```
if (!std::is_constant_evaluated()) {
    using Array = Counted[3];
    ...
}
if (!std::is_constant_evaluated()) {
    using Array = Counted[3][2];
    ...
}   
```
That way if someone adds to the end of the function, it won't get accidentally nerfed.
Alternatively, probably even better: pull the offending blocks out into a separate function, `bool test_arrays()`, and just don't test it in constexpr mode yet.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106916/new/

https://reviews.llvm.org/D106916