[libcxx-commits] [PATCH] D102135: [libcxx][ranges] adds _`copyable-box`_

[Louis Dionne via Phabricator via libcxx-commits]

ldionne marked 3 inline comments as done.
ldionne added inline comments.


================
Comment at: libcxx/include/__ranges/copyable_box.h:47
+  class __copyable_box {
+    optional<_Tp> __val_;
+
----------------
tcanens wrote:
> ldionne wrote:
> > Quuxplusone wrote:
> > > ldionne wrote:
> > > > cjdb wrote:
> > > > > Can this be `[[no_unique_address]]`?
> > > > There's really no point in doing that AFAICT, because `std::optional` is never empty.
> > > `[[no_unique_address]]` also has a layout effect on non-empty types, if they're tail-padded: https://godbolt.org/z/qjMWo699x
> > > 
> > > However, AIUI, its effect never leaks out of a type altogether: it makes a difference //only// on a member of `X` that is directly followed by another member of `X`. So if you have a class with only one data member, like this version of `__copyable_box`, then adding `[[no_unique_address]]` to that unique data member will never make a difference. (I think.) https://godbolt.org/z/K3GPPaxbz
> > That matches my understanding too. I don't think it's possible for the effect to leak out of the type since it must be possible to lay out objects of that type one after another in an array without messing up their alignment.
> Not quite: https://godbolt.org/z/Yhs6MqzYn
Ugh, interesting. So it does "leak out" when the type is used as a member which is itself marked as `[[no_unique_address]]`. In that case, I guess it's worth adding it here to allow propagating the no-unique-addressness to the types that use `copyable_box`.

For a second, I was also concerned that it might behave like `pragma pack(1)` and mess up alignment, but it looks like the compiler does the only sane thing it can, and doesn't mess it up. Awesome.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102135