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

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/test/std/utilities/memory/specialized.algorithms/specialized.construct/ranges_construct_at.pass.cpp:126
+  return 0;
+}
----------------
ldionne wrote:
> Quuxplusone wrote:
> > 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.)
> See https://reviews.llvm.org/D114649 for rectifying the solution for array types.
Given that there's an in-progress patch addressing LWG 3436, I'm inclined to leave as-is with a TODO to test array types once the patch lands.

> (We'd still have to decide what to do in C++20, because what was published is not implementable.)
I don't have a strong preference here. I presume we can postpone this discussion until D114649 lands.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116078