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

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

Quuxplusone added inline comments.


================
Comment at: libcxx/include/__ranges/copyable_box.h:89
+
+    constexpr _Tp const& operator*() const { return *__val_; }
+    constexpr _Tp& operator*() { return *__val_; }
----------------
zoecarver wrote:
> 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. 
So it needs to be dereferenceable, but why does it need to be `noexcept`-dereferenceable?
I had a vague notion that maybe we needed it to be noexcept because `transform_view::iterator::operator*` was specified as "expression-equivalent to" some expression that would be noexcept if not for the noexcept(false)-ness of this function. However, eel.is provides evidence against that hypothesis: https://eel.is/c++draft/range.transform.iterator
```
    constexpr decltype(auto) operator*() const {
      return invoke(*parent_->fun_, *current_);
    }
```
Here the Standard gives a reference implementation (not an "expression-equivalent to"), and the reference implementation is explicitly non-noexcept.

If the Standard had instead depicted
```
    constexpr decltype(auto) operator*() const noexcept(noexcept(invoke(*parent_->fun_, *current_))) {
      return invoke(*parent_->fun_, *current_);
    }
```
then I would agree, marking `copyable_box::operator*` as `noexcept` would be the path of least resistance. But it doesn't say that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102135