Re: [boost] [review] Review of Outcome v2 (Fri-19-Jan to Sun-28-Jan, 2018)

Subject: Re: [boost] [review] Review of Outcome v2 (Fri-19-Jan to Sun-28-Jan, 2018)
From: Rob Stewart (rstewart_at_[hidden])
Date: 2018-02-08 09:52:07

On February 6, 2018 4:15:14 AM EST, Niall Douglas via Boost <boost_at_[hidden]> wrote:
> >> When I compile with clang, with -Wall -Wextra -Werror, I get an
> error
> >> saying, "unused variable 'r'". See online examle:
> >> https://wandbox.org/permlink/WE7dK6r5XCIxJ83f
> > 
> > IOW, one must use certain tools and options to get the protection.
> 
> If you're not using clang-tidy in your project, then you ought to
> start.
> I've managed to raise it on an ancient VS2008 codebase. It
> surprisingly works.

While that may be true, the library can't be designed with only that tool in mind to find problems in code.

> > I think the library could, at least, assert in debug builds when a
> result or outcome is not inspected.
> 
> What does "not inspected" mean?

Creating a variable and then doing nothing more with it.

> In C and C++, historically when the programmer wishes to signal that
> they are intentionally throwing away a result, they write:
> 
> ```
> extern result<int> foo();
> ...
> // I don't care if this succeeds or fails
> (void) foo();
> ```
> 
> So okay, perhaps people should do that. But this is something people
> definitely do:
> 
> ```
> result<int> r = foo();
> if(r)
> {
>    x = r.value();
>    ...
> }
> ```
> 
> So now we need to make the explicit boolean operator become non-const
> in
> order to get it to set an internal flag so that the destructor can
> trip an assert on uninspected destruction.

The flag can be mutable.

> Now, I don't mind doing this, indeed I've left a free status bit for
> this sort of thing in the future. But are:
> 
> - Non-const boolean operator
> - Non-const .has_value(), .has_error() etc
> - Non-const .value(), .error() etc observers
> 
> ... a price worth paying for asserting an uninspected result, and this
> includes breaking the cast-to-void idiom?

The library should do what it can to ensure proper error handling. After all, one of the benefits of exceptions is that you cannot ignore them, so this library should strive for the same.

> One could of course const cast off any const-ness, and flip the
> inspected bit on any form of inspection. But I personally think there
> is
> a much better way that what you propose Rob, and that's to use the
> construction hooks to log the construction of result/outcome in debug
> mode.
> 
> This is what I do in my own code. See
> https://github.com/ned14/afio/blob/master/include/afio/v2.0/config.hpp#L321.
> This lets one detect, much more easily and without breaking behaviour
> semantics, unintentionally uninspected outcome/result.

How does that help since there are no hooks for when value(), error(), etc. are called?

--
Rob
(Sent from my portable computation device.)