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

śr., 12 wrz 2018 o 10:07 Niall Douglas via Boost napisał(a):

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.ht... ) 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` which has an error. Let's call this "value-on-valueless".

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

``` template 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, std::conditional_t<

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

...

...

...

;

template > using boost_result = basic_result;

template > using result = boost_result; ```

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?

Let me add some perspective and clarification to what Niall said. There is first the important issue to be clear about. Should "value on valueless" be considered a programmer error, or a correct action? My *impression* of the first review was that most of the people were of opinion that this is a programmer bug and the controversy was about whether it should cause a language-level UB or be nonetheless be reported via an exception. But this may be just my impression. Under this impression, it was my understanding that in order to propose a compromise solution for the impasse Niall added the policies that determine if "value on valueless" is an exception or an UB. But since the throwing policies now guarantee to throw, no-one can tell to the programmers "you are using value on valueless incorrectly", because for throwing policies the result is well defined. After some experience with using the Outcome library it became clear to me that throwing on value from valueless is actually a very useful feature, which enables the switch from Outcome-like error handling to normal exception handling. In a more abstract level this means that that function converting a string to an int does not make a call whether obtaining string "A" is an error or not: it pushes this decision on the caller. If the caller wants to consider this an error, she types `convert(s).value()`. and the error is signaled via an exception. The only thing that is disrupting this view is that the default policy in `result` is maybe-throwing maybe-UB depending on the type of EC. I suppose the motivation behind the semantics of the default value-on-valueless policy was that for the semi-experienced users, who know that they can also control the `EC` type we do not want to expose the knowledge the third parameter (the policy) yet, so we choose the default action. But as it is easy to figure out what type of exception to throw for `std::error_code` (it is std::system_error), you cannot guess it for `MyErrorType`, so the default in that case was not to throw but cause the UB. But I think failure to compile is even better: we are clear to the user that no good default can be chosen. I am actually proposing something slightly different to what Niall described. if `EC` is `void` I would also go for fail-to-compile. Regards, &rzej;