[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/include/__memory/ranges_construct_at.h:48
+  constexpr _Tp* operator()(_Tp* __location, _Args&& ...__args) const {
+    return _VSTD::construct_at(__location, _VSTD::forward<_Args>(__args)...);
+  }
----------------
ldionne wrote:
> Quuxplusone wrote:
> > As always, I'd feel better if this were
> > ```
> > return ::new (_VSTD::__voidify(*__location)) _Tp(_VSTD::forward<_Args>(__args)...);
> > ```
> > (for obvious-correctness, simplicity of implementation, and efficiency-at-`-O0`) but I suppose the extra-indirection way has the minor benefit that if we ever need to do any special magic to make `construct_at` constexpr-friendly, we'll only need to do it in one place? (For better or worse, I guess. If it later turned out that that magic was //required// to happen only in `construct_at` and not in `ranges::construct_at`, then we'd have to go back to the simple `::new`-expression here anyway.)
> It's actually the other way around, if you use `::new (location)` then it won't be constexpr-friendly, right? `std::construct_at` was introduced exactly to make this sort of pattern constexpr friendly, unless I'm having a giant brain fart.
> 
> This seems to confirm what I'm saying: https://godbolt.org/z/9jb7jx4q3
> 
> TLDR: AFAICT, this is the only correct way to implement the function without having to change Clang (and possibly the other compilers we support).
Looks like Clang probably whitelists "everything in `std`": https://godbolt.org/z/sGx1d1T7o
GCC must do something more narrowly tailored: I see their own `std::ranges::construct_at` is constexpr-friendly, but I haven't been able to craft one myself.

Anyway, I had not realized that `std::ranges::construct_at` was explicitly mentioned in [expr.const/6](https://eel.is/c++draft/expr.const#6) alongside `std::construct_at`; therefore it's //already// supposed to have the same constexpr magic as `std::construct_at`. So okay, I //still// think I'd rather see `::new` in both places, but if this way is projected to help us jump through GCC's hoops better, then I certainly won't block over it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116078