[libcxx-commits] [PATCH] D116078: [libc++][ranges] Implement `construct_at` and `destroy{, _at}`.

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

Quuxplusone added inline comments.


================
Comment at: libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/ranges_construct_at.pass.cpp:126
+  return 0;
+}
----------------
var-const wrote:
> Quuxplusone wrote:
> > I'd like to see some tests with array types, if that's supposed to work; or otherwise,
> > ```
> > static_assert(!can_construct_at((int(*)[])nullptr));
> > static_assert(!can_construct_at((int(*)[2])nullptr));
> > static_assert(!can_construct_at((int(*)[2])nullptr, 1, 2));
> > 
> > // and function types while we're at it
> > static_assert(!can_construct_at((int(*)())nullptr));
> > static_assert(!can_construct_at((int(*)())nullptr, nullptr));
> > ```
> Added the test for function types.
> 
> The situation with arrays is interesting. They don't work due to how the return type is currently specified (see https://godbolt.org/z/sT8xM7PvP), but there's no requirement that arrays should be SFINAE'd away. Also, there's a recently-updated proposal to support arrays: [LWG 3436](https://cplusplus.github.io/LWG/lwg-active.html#3436).
> 
> So we can a) make an extension to SFINAE away array types, then make it conditional in C++23 (or just remove it altogether) or b) do nothing and allow such calls to fail with a hard error. What do you think?
Personally, //I'd// just implement the proposed resolution for LWG3436 (in C++20-and-later). It's a good fix for a bug in the Standard, and it's always useful to get implementation experience with this kind of thing ASAP. Also, we don't want people to start relying on the wrong behavior and then change it out from under them in C++23. Also, once we've implemented the correct behavior, there's no reason to limit it to C++23-and-later when it could apply just as well to C++20. (We'd still have to decide what to do in C++20, because what was published is not implementable.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116078