[boost] [outcome] Change semantics on UB from peer review agreed semantics?

Subject: [boost] [outcome] Change semantics on UB from peer review agreed semantics?
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2018-09-12 08:07:08

Dear Boost,

At https://github.com/ned14/outcome/issues/150, Andrzej has raised his
desire to change Outcome's semantics on UB from what the two peer
reviews here agreed to previously. As this would be a major change to
interface different to what the consensus here agreed to, I am asking
boost-dev to weigh in with its opinion.

Status:
-------
Outcome has been finished, apart from docs, since April
(http://boost.2283326.n4.nabble.com/outcome-v2-1-progress-report-td4702900.html)
for the Boost and standalone variants, and since July for the
experimental P1028 edition. The documentation has not been touched since
the February review, but will be tackled when my current work contract
ends this November, and I regain free time.

Preamble:
---------
Outcome has the notion of programmable "default actions" which occur
when the user does something like call `.value()` on a `result<T, E>`
which has an error. Let's call this "value-on-valueless".

Outcome, as was approved here, has the following selector on `result<T,
E>` in the Boost edition:

```
template <class T, class EC, class E>
using default_policy = std::conditional_t<
  std::is_void<EC>::value && std::is_void<E>::value,
  terminate,

  std::conditional_t<

  trait::has_error_code_v<EC>, error_code_throw_as_system_error<T, EC,
E>,
  std::conditional_t<

  trait::has_exception_ptr_v<EC> || trait::has_exception_ptr_v<E>,
exception_ptr_rethrow<T, EC, E>,
  all_narrow

>>>;

template <class R, class S = boost::system::error_code, class
NoValuePolicy = policy::default_policy<R, S, void>>
using boost_result = basic_result<R, S, NoValuePolicy>;

template <class R, class S = boost::system::error_code, class
NoValuePolicy = policy::default_policy<R, S, void>>
using result = boost_result<R, S, NoValuePolicy>;
```

So, if result's `E` is:

- `void`, value-on-valueless calls `std::terminate()`.
- contains an error code, value-on-valueless throws `system_error()`.
- otherwise is hard undefined behaviour i.e. we tell the compiler that
value-on-valueless will never happen, and the compiler doesn't even
compile code for it to happen.

The documentation, as was approved here, made it very clear that hard UB
is the default for custom defined `E` types. See
https://ned14.github.io/outcome/tutorial/default-actions/happens1/. A
nice segfault usually emerges.

For the standalone edition, judging from user feedback and bug reports,
this hard UB behaviour is the most common intentional configuration for
Outcome users. I have not received any complaints about this choice, if
anything a majority has been choosing Outcome precisely *because* of
this choice.

Andrzej's request
-----------------
Andrzej points out in https://github.com/ned14/outcome/issues/150 that
it's too easy to get surprised by a small source change which causes the
error code trait to no longer match the `E` type, thus flipping the code
base from throwing `system_error` into hard UB unexpectedly and without
warning. He wishes to change the default to failure to compile i.e. if
result's `E` is:

- `void`, value-on-valueless calls `std::terminate()`.
- contains an error code, value-on-valueless throws `system_error()`.
- otherwise a failure to compile with useful diagnostic.

Users can, of course, manually specify a `NoValuePolicy` of `all_narrow`
if they wish hard UB on value-on-valueless. It would be a non-trivial
change from what peer review (twice) agreed, as the default would have
substantially changed.

What is boost-dev's opinion on this proposed change in semantics?

Thanks,
Niall