steve-downey / optional / 16595319667

100%

main: 99%

push

github

optional<T&>::or_else: copy *this instead of creating a new optional

If or_else is called on an engaged optional, we're supposed to return a
copy (or a move) of the `*this` object; otherwise we invoke the argument
of or_else, and return whatever optional that returns.

For optional<T> we were doing exactly that (`return *this` or
`move(*this)`). For optional<T&> we were instead doing `return *value_`,
where `value_` is the pointer used in the implementation. That ends up
creating an optional<T&> through its "value constructor".

The problem is that the two forms are not equivalent in corner cases;
consider this code:

```
T *obj = new T;
T &ref = *obj;
delete obj;     // obj now dangles

T &ref2 = ref;  // OK
```

The last line is OK even if `ref` does not refer to an object any more.

This code is instead not OK:

```
T *obj = new T;
T &ref = *obj;
delete obj;

T &ref2 = *obj; // UB, https://eel.is/c++draft/expr.unary.op#1
```

If we use optional<T&>::or_else, the implementation is isomorphic to the
second form, not to the first one:

```
T *obj = new T;
optional<T &> ref = *obj;
delete obj;

assert(ref); // OK
optional<T &> ref2 = ref.or_else(/* ... */); // UB; does *obj internally
```

We can avoid this UB by avoiding the dereference into
optional<T&>::or_else, and returning a copy of *this instead. The
semantics are otherwise the same, but we avoid tripping into UB.

I'm adding a test which however is inconclusive because compilers do
not detect the above UB during constant evaluation, although they're
required to do so. That's likely a bug.

1 of 1 new or added line in 1 file covered. (100.0%)

309 of 309 relevant lines covered (100.0%)

118790.4 hits per line