[review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
107 messages Options
123456
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
On Sun, May 14, 2017 at 5:39 PM, charleyb123 . <[hidden email]> wrote:
> Hope that helps ...?

Yep, that is exactly as I expected. I would just like to make sure
that we are all on the same page.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
2017-05-15 1:35 GMT+02:00 Peter Dimov via Boost <[hidden email]>:

> Niall Douglas wrote:
>
> The major refinement of outcome<T>, result<T> and option<T> over
>> expected<T, E> is that you **never** get undefined behaviour when using
>> their observers like you do with expected<T, E>. So when you call .error()
>> for example, you always get well defined semantics and behaviours.
>>
>
> I fully agree with this design decision of yours, by the way. In fact I
> consider that a defect in expected<>.
>
> you get a C++ exception thrown of type monad_error(no_state).
>>
>
> As a side note, it would be nice from my point of view if you eradicate
> these last remaining references to 'monad' in the public interface and make
> that outcome_error (resp. outcome_errc, outcome_category.)
>
> outcome<Foo> found;  // default constructs to empty
>> for(auto &i : container)
>> {
>>   auto v = something(i); // returns a result<Foo>
>>   if(v)  // if not errored
>>   {
>>     found = std::move(v);  // auto upconverts
>>     break;
>>   }
>> }
>> if(!found)
>> {
>>   die();
>> }
>>
>
> OK, let's go with that. Why not construct 'found' initially to contain
> some error, instead of being empty? You can even define a special errc
> constant to denote an empty outcome.
>
> Sure, this will not throw the exception in your earlier example which
> accessed v.error() when !v, but is the exception really necessary there?
>
> What I'm driving at is that these result types are conceptually (T|E) and
> the empty state could just be a special case of E.
>
> Or, in more general terms, I feel that there's still much extra weight
> that can be stripped off (not in terms of sizeof, but in terms of the
> interface.)
>

I think there is no good solution here. The expectation of the caller, as
you say, is that the function either returns a `T` or an error (explanation
why if failed to produce a `T`). But on the callee side you often need the
empty state, because the flow would often be like this:

```
vector<outcome<T>> os (20); // no value or error to assign yet

for (auto const& e : some_collection)
if (cond(e))
  os[i].set_value();
else
  os[i].set_exception();
```

And in some cases, you cannot immediately rewrite the code in order to
avoid default construction. You do not want to return an "empty" object,
but you need this value temporarily, end there may be no good value of `E`
to use.

I think Niall's choice is a practical one: if you must compromise the
interface anyway on one or the other side of the function, at least
optimize for performance: implementing the "empty" state comes for free.

Regards,
&rzej;

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
2017-05-15 0:54 GMT+02:00 Gavin Lambert via Boost <[hidden email]>:

> On 12/05/2017 04:19, charleyb123 wrote:
>
>> The formal review of Niall Douglas' Outcome library starts next week
>> (Fri-19-May to Sun-28-May).
>>
> [...]
>
>> - What is your evaluation of the documentation?
>>
>
> Granted std::error_code does let you transport "foreign" error codes, but
> surely translating recognised codes from a method's internal implementation
> to those of its external interface is desirable?  (Though perhaps that may
> depend somewhat on your views of encapsulation and implementation hiding.)


I think, breaking encapsulation in case of reporting failures from
functions (be it an exception or an error code) is generally expected and
desired, even though nobody will say it explicitly.

When I want to communicate with another subsystem, I want the mechanisms of
the communications to be abstracted away from me. But when it gets wrong, I
want all the information: that I was in fact using the TCP/IP for
communication; even exposed to the users, because they may know how to
respond.

Regards,
&rzej;

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
> -----Original Message-----
> From: Boost [mailto:[hidden email]] On Behalf Of Niall Douglas via Boost
> Sent: 15 May 2017 00:36
> To: [hidden email]
> Cc: Niall Douglas
> Subject: Re: [boost] [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)
>
> On 14/05/2017 23:09, Peter Dimov via Boost wrote:
> > That would be Paul Bristow, in
> >
> > https://lists.boost.org/Archives/boost/2015/05/222687.php
>
> That's an amazing find Peter. I had tried and given up. Thank you.
>
> Logged to https://github.com/ned14/boost.outcome/issues/17.
>
> And thanks to Paul as well.

You only needed to ask me and I could have told you ;-)

Paul



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
Andrzej Krzemienski вроте:
> vector<outcome<T>> os (20); // no value or error to assign yet
>
> for (auto const& e : some_collection)
> if (cond(e))
>   os[i].set_value();
> else
>   os[i].set_exception();

.set_error presumably. (Incidentally, result<> is documented to have
set_exception, which makes no sense to me.)

> And in some cases, you cannot immediately rewrite the code in order to
> avoid default construction. You do not want to return an "empty" object,
> but you need this value temporarily, end there may be no good value of `E`
> to use.

The argument that it's better to default-construct to some kind of a
valueless state instead of T{} is legitimate, but even if I grant all the
premises of that example (which is a stretch), I still don't see why
default-constructing to make_error_code( outcome_errc::uninitiaized ) isn't
better than special-casing the empty state.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
2017-05-15 12:37 GMT+02:00 Peter Dimov via Boost <[hidden email]>:

> Andrzej Krzemienski вроте:
>
>> vector<outcome<T>> os (20); // no value or error to assign yet
>>
>> for (auto const& e : some_collection)
>> if (cond(e))
>>   os[i].set_value();
>> else
>>   os[i].set_exception();
>>
>
> .set_error presumably. (Incidentally, result<> is documented to have
> set_exception, which makes no sense to me.)
>
> And in some cases, you cannot immediately rewrite the code in order to
>> avoid default construction. You do not want to return an "empty" object,
>> but you need this value temporarily, end there may be no good value of `E`
>> to use.
>>
>
> The argument that it's better to default-construct to some kind of a
> valueless state instead of T{} is legitimate, but even if I grant all the
> premises of that example (which is a stretch), I still don't see why
> default-constructing to make_error_code( outcome_errc::uninitiaized ) isn't
> better than special-casing the empty state.
>

Currently, if you are using library L, the `error_code` describes
"disappointments" typical to L's business domain. And an empty `outcome`
represents problems related to incorrectly using the Outcome library. IOW,
getting and error_code` from library L indicates a robust design that takes
into account any run-time situations related to managing resources or
irregular user input, whereas an empty state indicates a bug in the code
(you may use an empty state temporarily, but you shouldn't return it from
the function). I think I am now saying something different than Niall, but
that would be my conceptual model for this empty state.

Also, to construct some object, it takes time. error_code is just two
words, but why waste time on setting them if you will be overriding them in
the course of the function call. And, in the current state you can perform
a loss-less conversion (or "upgrade") form `option<T>` to `outcome<T>`.

Regards,
&rzej;

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
I wished you hadn’t use  **NEXT WEEK** shouty tile, it constantly makes me wrongly qualify new posts in this thread as spam.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
>> The major refinement of outcome<T>, result<T> and option<T> over
>> expected<T, E> is that you **never** get undefined behaviour when
>> using their observers like you do with expected<T, E>. So when you
>> call .error() for example, you always get well defined semantics and
>> behaviours.
>
> I fully agree with this design decision of yours, by the way. In fact I
> consider that a defect in expected<>.

It's the fault of std::optional<T>. In my opinion it never should have
had silent reinterpret_cast built into its API like that. The cost of
the state check is unmeasurable on any recent CPU, it's not worth not
having.

I've been a strong advocate that the same mistake not be repeated with
expected<T, E> with Vicente, but I have failed to shift his opinion. He
wants optional<T> equivalence, and I can see where he is coming from
(though I think optional<T> should get fixed instead, even if it breaks
existing code).

Another defect in Expected in my opinion is having value return
semantics for .value_or(). You'll note Outcome's Expected has reference
return semantics for .value_or() which is a deviation. Again, they only
chose value semantics because of optional<T>, and I have tried to ensure
they don't repeat the same mistake. I may have a chance of winning that
one however, they will discuss it in Toronto.

Finally, std::expected provides comparison operators which makes no
sense to me at all. If you are putting Expected's into a map, you need
your head examined in my opinion. Outcome's Expected does not provide
comparisons except for equality.

>> you get a C++ exception thrown of type monad_error(no_state).
>
> As a side note, it would be nice from my point of view if you eradicate
> these last remaining references to 'monad' in the public interface and
> make that outcome_error (resp. outcome_errc, outcome_category.)

A lot of people grumble at having "monad" in the naming. I don't
personally see the problem, it's just a name, and "basic_monad" is
exactly what is it: a building block for a monad.

I'll tell you what though, if two more people strongly feel that
"basic_monad" and "monad_error" need to be renamed, I will do it. Please
people do suggest a better alternative name though.

>> outcome<Foo> found;  // default constructs to empty
>> for(auto &i : container)
>> {
>>   auto v = something(i); // returns a result<Foo>
>>   if(v)  // if not errored
>>   {
>>     found = std::move(v);  // auto upconverts
>>     break;
>>   }
>> }
>> if(!found)
>> {
>>   die();
>> }
>
> OK, let's go with that. Why not construct 'found' initially to contain
> some error, instead of being empty? You can even define a special errc
> constant to denote an empty outcome.

You can ask it to be some error too:

outcome<Foo> found(make_errored_outcome<Foo>(std::errc::whatever));

If you didn't like the lack of explicit initialisation, you can also do:

outcome<Foo> found(empty);  // constexpr empty_t empty; let's you tag

But the point being made (badly) in the example above was that you can
use the empty state to indicate lack of find, and any errored state for
errors during find etc. So, let me rewrite the above now it's morning
and I am not babbling incoherently:

outcome<Foo> found(empty);
for(auto &i : container)
{
  auto v = something(i); // returns a result<Foo>
  if(!v.empty())  // if not empty
  {
    found = std::move(v);  // auto upconverts preserving error or Foo
    break;
  }
}
if(found.has_error())
{
  die(found.error());
}
if(found)
{
  Do something with found.value() ...
}

The above is much better. Please disregard what I wrote last night.

> What I'm driving at is that these result types are conceptually (T|E)
> and the empty state could just be a special case of E.

I did consider that design, but I rejected it. outcome<T>, result<T> and
option<T> all require E to be some form of error_code_extended, so the
user can override E, but not significantly change what contract it
promises. You might think then there is an ideal scope for !E to mean
empty, but that it turns out is a bad idea. A null error code has the
convention of meaning "no error occurred". It does not mean "empty".

You could define a special error code category to mean "empty", but now
you break all other code using error_code because there is no
error_condition which represents "empty".

Finally, if you did have some special E value to mean empty, you would
have to write special checks for it in Outcome in order to give it the
stronger abort type semantics it has - if you don't have it being given
alternative semantics, then there is no point in having an empty state.
If you are writing special checks, then you might as well just have a
formal empty state in the first place.

This argument can be generalised into the argument in favour of ternary
logics over binary logics. Sure, 90% of the time binary logics are
sufficient, but binary is a subset of ternary. And *you don't have to
use* the "other" state in a ternary logic just because it's there. Just
don't use it, so long as its implementation signals its unintentional
usage with a very loud bang, it's a safe design.

(There is an argument that Outcome's "very loud bang" isn't loud enough.
I'd be pleased to hear feedback on that)

> Or, in more general terms, I feel that there's still much extra weight
> that can be stripped off (not in terms of sizeof, but in terms of the
> interface.)

As you have surely noticed, I have (intentionally) provided many
undisambiguated ways of using Outcome, pushing the problem of which
specific combination to use onto the end user.

That looks sloppy I am sure. None of my other libraries do that either,
they provide one or two "clean" ways of doing any given thing, Outcome
is the only library I've done where I give the end user enormous scope
to choose their particular style and idioms and conventions and I give
no guidance as to which to use (I use different conventions in AFIO v2
and KernelTest myself personally).

But it was not done without reason. During my ACCU talk, about half the
audience really want implicit conversion for T and E in Expected so you
can skip typing make_xxx() all the time. The other half want always
explicit instantiation so that it is impossible to create an Expected
*without* using make_xxx() (BTW, Toronto will be discussing enforcing
this for std::expected i.e. to remove all implicit conversion)

The same goes for C++ monadic programming frameworks. There are many
diametrically opposed opinions, and no clear evidence which is right and
which is wrong. Outcome's monadic API (disabled in this presentation)
deliberately sits on the fence and provides just enough hooks to be used
by any external monadic programming framework. It doesn't implement a
full monadic programming DSEL. I intentionally don't choose sides nor
favourites. That upsets almost everyone, but my personal opinion here is
that there is no one right answer, sorry.

Similarly I intentionally sit on the fence with all the rest of Outcome:
it is up to the end user to choose the idiomatic style they prefer.
That's why the public API looks "heavy", but really it's just many
syntax ways of doing exactly the same thing. The core is extremely
simple to keep compiler optimisers happy and generating least possible
runtime overhead or code bloat which it very successfully achieves.

Ultimately what I would ask is instead of "is this API too fussy?" is
rather "is this API unsafe?" or "is this API self contradictory?". I am
very interested in feedback on the latter two.

Niall

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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
Andrzej Krzemienski wrote:

> Currently, if you are using library L, the `error_code` describes
> "disappointments" typical to L's business domain. And an empty `outcome`
> represents problems related to incorrectly using the Outcome library.

Exactly. Returning an uninitialized outcome<> to the caller is incorrect use
of library Outcome. Not supposed to happen in your example.

> Also, to construct some object, it takes time. error_code is just two
> words, but why waste time on setting them if you will be overriding them
> in the course of the function call.

Overly focusing on outcome<>'s runtime performance is odd given that it's
usually returned by functions that take millions of cycles to do their job.
But either way, it's possible to optimize the default construction to still
set a bit and only create the error_code on demand when it's retrieved.

> And, in the current state you can perform a loss-less conversion (or
> "upgrade") form `option<T>` to `outcome<T>`.

option<T> is only present because of the empty state; no empty state, no
option<T>. Duplicating opional<T> is questionable, upconverting it to
outcome<T> is, too. If you call a function that optionally gives you an
object T, and you have to return outcome<T>, you need to provide a reason
for the missing T, and the silent upconversion allows you to punt. Error
context is lost because only you know why your implementation detail failed
to provide the T you asked of it.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
> I think Niall's choice is a practical one: if you must compromise the
> interface anyway on one or the other side of the function, at least
> optimize for performance: implementing the "empty" state comes for free.

It's definitely the case that internally you always need to have an
empty state unless you are using Anthony's Williams' double buffer
design to enable variants to never ever be empty.

But Outcome's API choice was based from the beginning on choosing a
ternary logic for the public API. I personally think it's a great fit
which is why I chose it, but I appreciate the majority thinks these
sorts of class ought to have binary state. Hence Expected's design.

As I mentioned to Peter, if you really hate the ternary logic, just
don't use the other state. Then you get boolean logic.

Niall

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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
> .set_error presumably. (Incidentally, result<> is documented to have
> set_exception, which makes no sense to me.)

Defect logged to https://github.com/ned14/boost.outcome/issues/18.

Niall

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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
>> Also, to construct some object, it takes time. error_code is just two
>> words, but why waste time on setting them if you will be overriding
>> them in the course of the function call.
>
> Overly focusing on outcome<>'s runtime performance is odd given that
> it's usually returned by functions that take millions of cycles to do
> their job.

That may not be the case in some use cases. I've tried very hard to make
Outcome's runtime overhead statistically indistinguishable from C
integer error code returns (see FAQ for graph).

> But either way, it's possible to optimize the default
> construction to still set a bit and only create the error_code on demand
> when it's retrieved.

Outcome does do this, but in a different situation. If you try to
retrieve a .error() from an excepted Outcome, you get back an error_code
of monad_errc::exception_present with category monad_category.

>> And, in the current state you can perform a loss-less conversion (or
>> "upgrade") form `option<T>` to `outcome<T>`.
>
> option<T> is only present because of the empty state; no empty state, no
> option<T>. Duplicating opional<T> is questionable, upconverting it to
> outcome<T> is, too. If you call a function that optionally gives you an
> object T, and you have to return outcome<T>, you need to provide a
> reason for the missing T, and the silent upconversion allows you to
> punt. Error context is lost because only you know why your
> implementation detail failed to provide the T you asked of it.

I think you might be underestimating the bug detecting value of an
outcome<T> having an unhandled empty state. It traps failure to handle
option<T> correctly very nicely.

Also, don't forget option<T> is the only one from result<T> and
outcome<T> which is usable in constexpr programming. One therefore ends
up using option<T> or expected<T, E> in constexpr and feeding the output
to result<T> or outcome<T>.

Niall

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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 14/05/2017 12:33, Niall Douglas wrote:

>>> But I can see a feature like __builtin_expect that going the way of the
>>> dodo as the compiler vendors really would prefer if you used profile
>>> guided optimisation instead. Passing that sort of micro-info from the
>>> parser to the backend is surely complex to get right.
>>
>> I looked for bug reports about __builtin_expect in gcc's bugzilla, and
>> the only relevant one I found was
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59521 about using it in a
>> switch. Could you back your statement with some links? Otherwise it
>> sounds like FUD (at least the part about ignoring multiple persistent
>> reports).
>
> It's possible my memory is faulty, but I don't think it was. It was a
> very good talk, basically a long list with extensive microdetail of how
> compiler version quirks get in the way of micro optimisation. Builtin
> expect was but one of many in a long list.
>
> The presenter, Jason McGuiness, is one of the more colourful regular
> attendees at ACCU and from all my interactions with him to date, I would
> be considering him to not be FUDing. He's presented similar material at
> ACCU London talks and several others.
>
> His slides for the ACCU talk aren't online yet, so I'm sent him a
> LinkedIn request and I'll ask for a copy. I'll post a link here if I get
> them.

Jason has provided a copy of the ACCU talk slides at
http://research.ma.cx/ACCU_Conference_2017_v1_1.pdf. Lots of graphs and
assembler showing builtin_expect not working right on GCC.

Niall

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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
Niall Douglas wrote:
> I'll tell you what though, if two more people strongly feel that
> "basic_monad" and "monad_error" need to be renamed, I will do it. Please
> people do suggest a better alternative name though.

basic_monad is an implementation detail, but monad_error is part of the
interface.

> Similarly I intentionally sit on the fence with all the rest of Outcome:
> it is up to the end user to choose the idiomatic style they prefer. That's
> why the public API looks "heavy", but really it's just many syntax ways of
> doing exactly the same thing.

The problem with this approach, apart from not sitting well with me
personally, is that once the library enters wide use, you can no longer take
any of these alternative APIs away without breaking code. It's better, in my
opinion, to provide a minimal interface first, then add bells and whistles
on an as-needed basis. I look at result<>'s reference documentation and all
I can think of is that 2/3 of it isn't needed and 2/3 of that even consists
of different spellings of the same thing.

I'd even remove value_or, there's nothing wrong with r? r.value(): def.

> During my ACCU talk, about half the audience really want implicit
> conversion for T and E in Expected so you can skip typing make_xxx() all
> the time. The other half want always explicit instantiation so that it is
> impossible to create an Expected *without* using make_xxx() (BTW, Toronto
> will be discussing enforcing this for std::expected i.e. to remove all
> implicit conversion)

A legitimate fork in the road. What I'd do is enable conversion when
unambiguous, otherwise require make_expected<T> and make_unpexpected<E>. But
that's not what we're talking about here, because make_expected and
make_unexpected will be present regardless of the decision to enable
implicit conversion or not.

> Another defect in Expected in my opinion is having value return semantics
> for .value_or(). You'll note Outcome's Expected has reference return
> semantics for .value_or() which is a deviation.

I don't care for the pervasive &/&&/const&/const&& duplication
(fourplication) very much myself, would return by value everywhere, but
that's a matter of taste. Follows from the philosophy to provide the simple
thing first, then complicate if need arises.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
2017-05-15 13:08 GMT+02:00 Niall Douglas via Boost <[hidden email]>:

>
> >> you get a C++ exception thrown of type monad_error(no_state).
> >
> > As a side note, it would be nice from my point of view if you eradicate
> > these last remaining references to 'monad' in the public interface and
> > make that outcome_error (resp. outcome_errc, outcome_category.)
>
> A lot of people grumble at having "monad" in the naming. I don't
> personally see the problem, it's just a name, and "basic_monad" is
> exactly what is it: a building block for a monad.
>
> I'll tell you what though, if two more people strongly feel that
> "basic_monad" and "monad_error" need to be renamed, I will do it. Please
> people do suggest a better alternative name though.
>

I strongly feel that name "monad" need to be renamed :)
Proposed alternatives:

   - `basic_monad` -> `outcome_base` or `basic_outcome`
   - `monad_error` -> `bad_outcome` or `outcome_error`

Regards,
&rzej;

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Mon, 15 May 2017, Niall Douglas via Boost wrote:

> On 14/05/2017 12:33, Niall Douglas wrote:
>>>> But I can see a feature like __builtin_expect that going the way of the
>>>> dodo as the compiler vendors really would prefer if you used profile
>>>> guided optimisation instead. Passing that sort of micro-info from the
>>>> parser to the backend is surely complex to get right.
>>>
>>> I looked for bug reports about __builtin_expect in gcc's bugzilla, and
>>> the only relevant one I found was
>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=59521 about using it in a
>>> switch. Could you back your statement with some links? Otherwise it
>>> sounds like FUD (at least the part about ignoring multiple persistent
>>> reports).
>>
>> It's possible my memory is faulty, but I don't think it was. It was a
>> very good talk, basically a long list with extensive microdetail of how
>> compiler version quirks get in the way of micro optimisation. Builtin
>> expect was but one of many in a long list.
>>
>> The presenter, Jason McGuiness, is one of the more colourful regular
>> attendees at ACCU and from all my interactions with him to date, I would
>> be considering him to not be FUDing. He's presented similar material at
>> ACCU London talks and several others.
>>
>> His slides for the ACCU talk aren't online yet, so I'm sent him a
>> LinkedIn request and I'll ask for a copy. I'll post a link here if I get
>> them.
>
> Jason has provided a copy of the ACCU talk slides at
> http://research.ma.cx/ACCU_Conference_2017_v1_1.pdf.

Thanks.

> Lots of graphs and assembler showing builtin_expect not working right on
> GCC.

Not really. There are only 2 mentions of __builtin_expect in those slides:
* switch expansion in gcc ignores __builtin_expect (that's the PR I
referenced above)
* an unclear benchmark about adding __builtin_expect in 1 place in some
unknown code, where __builtin_expect actually seems to help with the more
recent version of gcc.

No "inverting the code path you specifically told the compiler was the hot
path", no "multiple persistent reports", no "the feature is now useless on
GCC >= 5" (he actually writes "Newer versions of g++ make better use of
optimization" though that's not specifically about __builtin_expect), no
"The speaker recommended clang, especially very recent clang" ("No one
compiler appears to be best - choice is crucial. Newest versions of clang
have not been investigated.")

The vocabulary in the slides is odd (any memory is called stack, anything
64 bits is called SSE) and there are some questionable statements. Taking
a gcc dev point of view, the main information I got form those slides is
interest in PR 59521 and some small margin of progress on copying constant
strings.

If anything, the section on static branch prediction advertises that one
should use __builtin_expect more ;-)

--
Marc Glisse

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
>> Similarly I intentionally sit on the fence with all the rest of
>> Outcome: it is up to the end user to choose the idiomatic style they
>> prefer. That's why the public API looks "heavy", but really it's just
>> many syntax ways of doing exactly the same thing.
>
> The problem with this approach, apart from not sitting well with me
> personally,

I do want to emphasise that I agree with you. Outcome is the only C++
library I've ever written which was multi-paradigm. Normally I decide on
one "right" way to do things, and enforce that on end users. You may
remember that from the AFIO v1 review where everyone didn't like my
choice of single vision, hence universal rejection bar one.

But in the end, I am overwhelmed by evidence that a single "right" way
is the wrong design here in this one very unusual situation. In my own
libraries which use Outcome I am using different conventions and use
patterns. I tried to summarise those with catchy names for the design
patterns in the Outcome tutorial, but to quote from
https://ned14.github.io/boost.outcome/md_doc_md_07-faq.html#examples_of_use:

"The main reason I designed and wrote Outcome was to implement a
universal error handling framework which could express in the minimum
possible overhead, and without losing information, the many C++ error
handling design patterns possible, and even more importantly that
individual libraries could use the design pattern which best suited them
whilst seamlessly interoperating with other libraries using different
error handling designs. To go into a bit more detail:

* Proposed Boost.AFIO v2 is a very low level very thin file i/o and
filesystem library which sits just above the raw kernel syscalls.
Throwing exceptions in such a library is overkill, so AFIO v2 uses the
"sea of noexcept" design pattern both in its public API and in its
internal implementation (i.e. it doesn't use C++ exceptions at all).

* Proposed Boost.KernelTest is a kernel based testing infrastructure
which uses Outcomes as the storage for each kernel permutation run in
its permutation tables of preconditions, postconditions, parameters and
outcomes. KernelTest itself is written using the "exceptions are
exceptional" design pattern where expected errors are returned via
outcomes but unexpected errors which abort the test use thrown
exceptions which are collected into outcome<T>'s. AFIO v2's test suite
is written using KernelTest.

* Planned Boost.BLOBStore will be a versioned, ACID transactional key to
BLOB store written using AFIO v2 which will use both the "sea of
noexcept" and the "exceptions are exceptional" design patterns together.
This allows user supplied callbacks to throw exceptions which aborts the
current transaction and for those exceptions to be propagated, if
desired, out of BLOBStore whilst the internal implementation of
BLOBStore and indeed its public API is all noexcept and never throws
exceptions (writing correct filesystem code is hard enough without
dealing with unexpected control flow reversal). BLOBStore will also use
KernelTest for its test suite."


I know you've read the history page Peter, so you know Outcome has
already had its API trimmed by half. These past 18 months I've been
removing stuff, paring down to the minimum. I don't claim I'm done
removing stuff yet, I agree there is a little more to go, but it's
getting harder to decide on what isn't really necessary any more.

> is that once the library enters wide use, you can no longer
> take any of these alternative APIs away without breaking code.

I absolutely agree with this assessment. Which is why Outcome's ABI is
versioned, and can be iterated with breaking changes if needs be.

Right now the ABI is permuted per git commit with the commit SHA as
Outcome is unstable. Once it's been firmed up through people using it
more, I'll declare a stable ABI which will be written in stone and
tested with abi-compliance-checker per commit. But thanks to the ABI
versioning, I can also make breaking changes without breaking code if it
turns out I made a terrible mistake in semantics.

> It's
> better, in my opinion, to provide a minimal interface first, then add
> bells and whistles on an as-needed basis. I look at result<>'s reference
> documentation and all I can think of is that 2/3 of it isn't needed and
> 2/3 of that even consists of different spellings of the same thing.

Are you referring to .get() and .value()?

I use a convention of .value() in my own code to indicate when I am
retrieving the value, and .get() to indicate I am throwing away the
fetched value but I do want any default actions to occur e.g. if not
valued, throw an exception. So .get() means "fetch and throw any error
state if present".

If people like this convention, I can make it formal by tagging
.value()'s return with [[nodiscard]] and have .get() return void. If
people dislike this convention, .get() can be removed.

I really wasn't sure what people might prefer. [[nodiscard]] is so new
it's hard to decide on if we are using it overkill or not. I look
forward to any feedback.

> I'd even remove value_or, there's nothing wrong with r? r.value(): def.

Both optional and expected have .value_or().

I've also found the ternary operator a poor substitute for .value_or()
in practice because both sides need to be the same type else the
compiler complains. .value_or() coerces the type properly. Less surprise
and less typing.

>> During my ACCU talk, about half the audience really want implicit
>> conversion for T and E in Expected so you can skip typing make_xxx()
>> all the time. The other half want always explicit instantiation so
>> that it is impossible to create an Expected *without* using make_xxx()
>> (BTW, Toronto will be discussing enforcing this for std::expected i.e.
>> to remove all implicit conversion)
>
> A legitimate fork in the road. What I'd do is enable conversion when
> unambiguous, otherwise require make_expected<T> and make_unpexpected<E>.

I completely agree for the Expected proposal. But Vicente doesn't like
that idea.

Regarding the community disagreement being a legitimate fork in the
road, I used to think that. But as I hopefully demonstrated above, as I
design more libraries using Outcome I find myself using multiple
paradigms, and I think I am right to have done that for each library in
question. I claim therefore that this is not a fork in the road, but
rather parallel roads all starting from the same place and ending in the
same place.

One therefore needs an error handling system which easily lets you
"cross lanes" between those parallel roads simply and without losing
original information. You thus get Outcome.

>> Another defect in Expected in my opinion is having value return
>> semantics for .value_or(). You'll note Outcome's Expected has
>> reference return semantics for .value_or() which is a deviation.
>
> I don't care for the pervasive &/&&/const&/const&& duplication
> (fourplication) very much myself, would return by value everywhere, but
> that's a matter of taste. Follows from the philosophy to provide the
> simple thing first, then complicate if need arises.

Alas Expected and Outcome permit usage with types with no default
constructor, no copy nor move. If this were not the case, I'd agree with
returning by value everywhere. But if you permit types limited like
that, returning by value anywhere in your API ought to be ruled out as
causing needless surprise and consternation to end users when an API
suddenly stops working just because the type used has changed.

BTW, is everyone aware that expected<void, void> is legal? Outcome
actually uses it internally. It is actually useful, believe it or not.

Niall

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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
>> I'll tell you what though, if two more people strongly feel that
>> "basic_monad" and "monad_error" need to be renamed, I will do it. Please
>> people do suggest a better alternative name though.
>>
>
> I strongly feel that name "monad" need to be renamed :)
> Proposed alternatives:
>
>    - `basic_monad` -> `outcome_base` or `basic_outcome`
>    - `monad_error` -> `bad_outcome` or `outcome_error`

Ok, second vote registered.

Niall

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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] **NEXT WEEK** Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
> The vocabulary in the slides is odd (any memory is called stack,
> anything 64 bits is called SSE) and there are some questionable
> statements. Taking a gcc dev point of view, the main information I got
> form those slides is interest in PR 59521 and some small margin of
> progress on copying constant strings.
>
> If anything, the section on static branch prediction advertises that one
> should use __builtin_expect more ;-)

I am not the author of the talk. I can say he checked the contents of
this thread and had a few colourful things to say about it, but did not
disagree with my recollection of what he said.

As I mentioned before, he's a long time attendee and presenter at ACCU.
I am inclined to believe whatever he claims, he knows his stuff.

Niall

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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [review] Review of Outcome (starts Fri-19-May)

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
Niall Douglas wrote:

> I know you've read the history page Peter, so you know Outcome has already
> had its API trimmed by half. These past 18 months I've been removing
> stuff, paring down to the minimum. I don't claim I'm done removing stuff
> yet, I agree there is a little more to go, but it's getting harder to
> decide on what isn't really necessary any more.

I know that, which is why I'd prefer the removal to proceed to its logical
conclusion. :-)

> Are you referring to .get() and .value()?

Not just that; is_ready, get_or, set_*, op->, op*, get_error, get_error_or.

> > I'd even remove value_or, there's nothing wrong with r? r.value(): def.
>
> Both optional and expected have .value_or().

Yeah, they are also wrong in having it. You/we don't need to copy
std::optional. It's not a good example to follow.

> I've also found the ternary operator a poor substitute for .value_or() in
> practice because both sides need to be the same type else the compiler
> complains.

That's more of a feature. If you're passing something else this may well
indicate a mistake.

> Alas Expected and Outcome permit usage with types with no default
> constructor, no copy nor move.

This would rely on guaranteed return value elision and return { in_place,
args... }, I suppose?

If these classes at some point enter the standard, the committee will
inevitably adorn them with all this beauty, but until then, no need to
prematurely do that ourselves.

> BTW, is everyone aware that expected<void, void> is legal?

I wasn't aware that E could be void. Which reminds me to ask you, do you
really drop the value on the floor when converting from <T> to <void>? This
seems like something to avoid. The value is probably important.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
123456
Loading...