Re: [boost] Boost.Outcome review - First questions

Subject: Re: [boost] Boost.Outcome review - First questions
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2017-05-21 23:15:11

Now to the reply to your post point by point ...

>>> # Our |value_or()| member functions pass through the reference rather than
>>> returning by value (we don't understand why the LEWG proposal does
>>> except that |std::optional<T>| does, which itself is a bad design choice
>>> and it should be changed).
>>>
>>> Well, what is the advantage to returning by reference?
>> You don't cause .value_or() to surprisingly fail to compile when T lacks
>> a move or copy constructor. The reference returning edition does not
>> impose the requirement for T to have a move or copy constructor. This
>> choice for .value_or() interferes less on end users.
> Do you have cases where expected<T,E> has a T that is not move/copy
> constructible?
> How can you return it?

You can't directly. But some might wrap it in a unique_ptr. Up to them,
it is they who supply type T and they may not even control its
implementation. If a user is forced to use such a T they cannot modify,
I don't think you should punish them for that.

In general I don't think you can rightly impose that T needs to have a
move or copy constructor. You don't *have* to impose it, so don't impose
it. Don't mess up the end user's life and make their programming harder
for no good reason.

>> The fact optional implemented .value_or() with a return by value is a
>> mistake. Expected need not repeat that mistake.
> Could you point me to the C++ standard std::optional defect? Have you
> created one?

optional is in the C++ 17 standard. Code will be written against its
current design of .value_or(). It can no longer be changed :(

>>> * it is not clear from the reference documentation if expected<void,E>
>>> is valid?
>> Do you mean there is no reference in the API docs to an expected<void,
>> E> specialisation? I didn't add one as I figured it was self evident and
>> unsurprising. Also, if we document expected<void, E>, we'd have to also
>> do result<void>, outcome<void>, expected<void, void> etc and to be
>> honest, they all behave exactly as you'd expect a void typed edition
>> would: wherever there is a T, you get T=void. Nothing special.
> Maybe or maybe not. What is the signature of value_or?

Exactly the same: value_type &value_or(value_type &)

value_type is defined to an unusable but helpfully named type for
compiler warnings and error when raw_value_type=void. This is very
tersely explained in the Expected synopsis page. I will be improving the
reference API docs to describe this better as part of issue #26.

>>> |* |is expected<T,E> the sum type of T, E and empty? and exception_ptr?
>> expected<T, E> may only have the state of T or E. The valueless by
>> exception state has been eliminated recently in develop branch.
> Is this the case also for outcome/result/option?

Yes.

>>> What are the exception warranties for |error_code_extended?
>> It never throws, not ever.
>>
>> This is pretty clear in the reference docs. Everything is marked noexcept.
> I don't see it in
> https://ned14.github.io/boost.outcome/classboost_1_1outcome_1_1v1__xxx_1_1error__code__extended.html
> Could you point me where noexcept is?

Heh. You are completely right, noexcept is missing. That's a bug: logged
to https://github.com/ned14/boost.outcome/issues/34.

>>> What are the exception warranties for |outcome<T>||?
>> The tutorial covers those. The reference docs will too when issue #26 is
>> implemented. The noexcept modifiers on most of the member functions are
>> pretty clear however.
> This is the kind of information we want on the reference documentation.
> Could you tell us what the issue #26 The default actions for each of
> .value(), .error(), .exception() etc need to be spelled out in the
> reference docs <https://github.com/ned14/boost.outcome/issues/26> will
> add to the docs and how this issue is related?
> What about the other functions, constructors, assignments ...

The information in the tutorial will be copied into each reference API's
docs using a style similar to standardese. I will cover constructors,
assignments, destructors etc as well.

>> You are correct, but I left it unfixed to remind me to ask here what
>> would be a suitable unit test for testing an "initializer_list<U> il,
>> Args&&... args" constructor where the Args&&... gets used correctly. I
>> was unsure of the semantics.
> Take a look at std::optional test if you are curious.

I found only this in libstdc++'s unit tests for its optional:

std::optional<std::vector<int>> o { std::in_place, { 18, 4 },
std::allocator<int> {} };

It'll do.

>>> Why do you prefer an implicit conversion. This deviation must be in the
>>> rationale of the differences.
>> The deviations list already explains that types T and E cannot be
>> convertible into one another because we use simple overloading to avoid
>> using SFINAE to keep compile time load low.
>>
>> Code written for LEWG Expected will, unless types T and E are
>> convertible into one another, work as expected.
> You don't answer to my question. Why implicit?
> What is wrong with the constructor
> 
>     expected(unexpect_t, error)
> 
> ?

As I've mentioned to you by private email, a lot of end users want
implicit conversion from T and E to save on boilerplate. Outcome doesn't
allow T and E to be convertible into one another, so permitting implicit
conversion from both T and E is safe.

Consider it a populist extension over LEWG Expected. I know you don't
like it, and I to be honest think it best used sparingly, but it'll be
very popular with many.

>>> * raw types
>>>
>>> It is not clear what is the difference between xxx and raw_xxx
>> Logged to https://github.com/ned14/boost.outcome/issues/29
> Thanks for creating the issue, but I'm interested in knowing it now
> during the review. Otherwise I couldn't accept even conditionally the
> library.

When configured with void, value_type, error_type and exception_type are
all set to an unusable but usefully named types suitable for compiler
warnings and errors. This makes writing metaprogramming much easier as
you don't need to deal specially with void and its weird semantics, plus
any attempt to use the type causes a very descriptive compiler error.

raw_value_type, raw_error_type and raw_exception_type are the true,
original type configured. You are correct that this is a deviation from
LEWG Expected which would cause code written for LEWG Expected to fail
to compile with Outcome's Expected. I think that safe.

>>> * what is the sizeof expected <T,E>?
>> It says already at the top of the page. It's max(24, sizeof(R)+8) on 64
>> bit CPUs.
> An now that you will ensure the never empty warranties?

It's the same sizeof.

If all your types have nothrow move, or your types don't have move nor
copy constructors, we directly destruct and construct during assignment
without saving the old state as the fast path. Else we move the current
state onto a stack based copy before copy or move constructing in the
new state. If that throws, we move back the previous state. If that
restoring move throws as well, we are left valueless by exception/empty.

I didn't implement Anthony William's double buffer solution for perfect
never empty warranties even when the types have no move nor copy
constructors. It is quite complex if you also wish to get alignment
right, constexpr right, and keep the optimiser maximally folding code.

I will document the semantics described above in the tutorial and
reference API docs. They are a good balance of never empty warranties
with space consumption and runtime overhead. I believe these guarantees
are similar to those in your Expected proposal? As in, you don't
implement the double buffer solution either.

>> The return by E rather than expected<void, E> is by design. Remember we
>> permit implicit construction from E, so this is safe and works well.
>> Again, code written for LEWG Expected should never notice this
>> implementation detail.
> Why? if the signatures doesn't match I will have sure a problem somewhere.

Such problems are on me as the maintainer.

>> (the real cause for this deviation is because the policy classes cannot
>> return types still being fabricated, so they cannot return an
>> expected<void, E>. If this deviation ever should become a problem, I can
>> relocate get_unexpected to the main class and enable_if it, or else use
>> a trampoline type. But I believe the existing form replicates the same
>> semantics, it should be fine)
> This is what I don't like of your library. In order to have a common
> implementation you sacrifice the concrete interfaces.

The common implementation is the heart and soul of Outcome, and is
precisely its "value add" over most other Either monad or Expected
implementations. A common implementation allows seamless interoperation
between implementations. It lets you write code using expected<T, E>
with outcome<T>, result<T> and option<T> all using the exact same
framework. End users can extend the family with any arbitrary custom
error transport of their choosing for some bespoke use case.

> I prefer to have concrete classes that provide the interface is the most
> adapted to these classes and then have an adaptation to some type of
> classes requirement when I need to do generic code.

In most of my libraries so do I. But in this situation, I went a
different direction. I think it's the right one for this use case.

> This something that I remarked since the beginning of your library. The
> basic_monad class is over-engineered IMHO.

Please see the other email for a reply to this sentiment.

>> Vicente thanks very much for such detailed feedback. It was very valuable.
>>
> Niall, please, don't wait until the review is finished to tell us how
> the issues will be fixed. You could do it on each issue and come back in
> this ML.

I try my best to do so. But a lot of time, how the issues will be fixed
is very obvious: I copy and paste documentation from one location to
another for example, or I repair an incorrect constructor so it is
correct and so on. It's not rocket science to deduce how most issues
will be fixed.

Niall

-- 
ned Productions Limited Consulting
http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/