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

[Zoe Carver via Phabricator via libcxx-commits]

zoecarver added inline comments.


================
Comment at: libcxx/include/__ranges/copyable_box.h:111
+  class __copyable_box<_Tp> {
+    [[no_unique_address]] _Tp __val_;
+
----------------
What do you think about this problem: https://reviews.llvm.org/D103056#inline-999709

Basically, if we have two copyable-box members that are both `no_unique_address` where T is copyable and empty, we will never invoke the copy constructor on assignment. I think this is probably a bug with copyable box, and not its users. 


================
Comment at: libcxx/include/__ranges/copyable_box.h:89
+
+    constexpr _Tp const& operator*() const { return *__val_; }
+    constexpr _Tp& operator*() { return *__val_; }
----------------
ldionne wrote:
> Quuxplusone wrote:
> > zoecarver wrote:
> > > This and the other's need to be noexcept. (After my patch, std::optional's star will also be noexcept.)
> > Since this is an internal helper, it doesn't //need// to be noexcept; there's definitely nothing in libc++ (and by definition, nothing outside libc++) that would ever check its noexceptness-status.
> > We know from other reviews that there is a danger in accidentally saying `noexcept` when the function might throw (namely, rogue calls to `std::terminate`), whereas there is no danger in accidentally saying nothing when the function doesn't throw.
> > However, I wouldn't strongly //object// to marking it noexcept, //because// it's so trivial that its noexceptness is "obvious" (famous last words).
> @zoecarver  I'm not sure I understand why it needs to be `noexcept`. Can you please explain?
We need this to be noexcept so that it can be used in transform_view. For example `transform_view::operator*` needs to be able to dereference the copyable-box to invoke the function object. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102135