Discussion:
[boost] [outcome] Second high level summary of review feedback accepted so far
Niall Douglas via Boost
2017-05-30 14:58:22 UTC
Permalink
Firstly thanks to everyone who has contributed no less than **618**
emails at the time of writing to the Outcome review. Despite some on
Reddit calling this review a "train wreck" and indeed some lovely things
about me personally, I have found this review deeply helpful, and I
believe so have most of its contributors in helping us all firm up our
differing opinions about what such a vocabulary type ought to be.


With regard to the previous high level summary at
http://boost.2283326.n4.nabble.com/outcome-High-level-summary-of-review-feedback-accepted-so-far-tp4694767.html,
I have made only these changes to the changes in that summary:

1. I have been persuaded to use longer more appropriate naming for
result<T> and outcome<T>, so now the typedefed varieties with implicit
conversions to their empty-capable form indicated by "=>" are:

- static_checked_outcome<T> => static_checked_optional_outcome<T>
static_checked_result<T> => static_checked_optional_result<T>

These have narrow observers which reinterpret_cast and are therefore
undefined behaviour if the state does not match, but use
__builtin_unreachable() to help static analysis tooling like clang-tidy
spot UB where it can be statically deduced. Conceptually, they are a
very thin convenience wrap of std::variant<...>.


- runtime_checked_outcome<T> => runtime_checked_optional_outcome<T>
runtime_checked_result<T> => runtime_checked_optional_result<T>

The runtime_checked_optional_* varieties are identical and unchanged to
the outcome<T> and result<T> in the submitted library. They have wide
observers with a formal empty state and it is never undefined behaviour
to call any observer. This allows the programmer to save typing
considerable boilerplate knowing that the runtime checks save the
programming needing to spell things out. I have not been persuaded to
change a single thing in their design yet apart from removing
unnecessary member functions, but thanks to everybody who has tried, and
if you think I am making a critical mistake, you still have a few days
left to persuade me of it!

For anyone not familiar with the discussion and feels that the names are
excessively long, be aware they are just convenience typedefs of a more
complex to configure "template<...> class outcome_impl". It is expected
that end users of the library will choose the varieties they want for
their code and typedef them into their local namespace as "result<T>" or
whatever. Nobody is proposing that end users actually type out the full
name each and every time they use them, and the documentation will make
that clear.


2. I have been persuaded to replace the implicit conversion from
static_checked_* varieties to runtime_checked_* varieties with explicit
conversion instead as the implicit conversion could produce runtime
failure where the programmer did not expect that to occur.



Regarding other changes that I have accepted since the previous high
level summary:

1. Outcome's STL exception throw catch to result<T> super-macro was
found to be overkill given that current compilers won't replace simple
throw...catch sequences with a branch, and still invoke the full
throw-catch runtime machinery whatever the case. Therefore an exception
to error code conversion function makes more sense than a macro.
https://github.com/ned14/boost.outcome/issues/50


2. Outcome's usage of git submodules is felt by some reviewers to be a
showstopper for acceptance as they could bring in non-Boost code
unexpectedly in the future. I have agreed to make a cronjob script
generated Boost.Outcome from standalone Outcome in the same fashion as
Boost.ASIO is script generated from standalone ASIO.
https://github.com/ned14/boost.outcome/issues/51

- Licensing to be BSL only
- No cmake, just bjam
- No git submodules at all
- std-flavoured, not boost-flavoured
- No code to be brought in except that written by me i.e. a hardcoded
whitelist applies.
- Contents of Boost repo to only contain material directly relevant to
Boost. Nothing else.


3. Under the assumption that error_code_extended's use of a static
global ring buffer would be controversial in this review, I hacked a
quick and dirty solution expecting to have to remove it. Now we know
that few disapprove, a more serious implementation will be needed.
https://github.com/ned14/boost.outcome/issues/52


4. Given where Vicente appears to be heading next with expected<T, E1,
E2, ...> it may be wise to make the static_checked_* varieties use
std::variant<...> for storage when being compiled on C++ 17.
https://github.com/ned14/boost.outcome/issues/53


Thanks,
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
Vinnie Falco via Boost
2017-05-30 15:06:30 UTC
Permalink
On Tue, May 30, 2017 at 7:58 AM, Niall Douglas via Boost
Post by Niall Douglas via Boost
- No cmake, just bjam
Wait, what? Why would we want to remove functionality that already
exists? I like the CMake script since it generates a Visual Studio
project file (.vcxproj). Please don't take this away, its helpful.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-05-30 15:28:39 UTC
Permalink
Post by Vinnie Falco via Boost
On Tue, May 30, 2017 at 7:58 AM, Niall Douglas via Boost
Post by Niall Douglas via Boost
- No cmake, just bjam
Wait, what? Why would we want to remove functionality that already
exists? I like the CMake script since it generates a Visual Studio
project file (.vcxproj). Please don't take this away, its helpful.
Standalone Outcome would remain cmake based. I have no love nor use case
for bjam personally.

(the bigger answer is that the cmake build support is entirely
implemented by the boost-lite git submodule because it's a universal
build system for a distributed collection of standards aspiring
libraries. The CMakeLists in the Outcome root is just a set of
parameters for that universal build system. As some people consider git
submodules to be a showstopper, that makes impossible cmake build
support in the Boost.Outcome script generated repo because the submodule
with the cmake build support must be removed)

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
Robert Ramey via Boost
2017-05-30 15:40:50 UTC
Permalink
Post by Vinnie Falco via Boost
On Tue, May 30, 2017 at 7:58 AM, Niall Douglas via Boost
Post by Niall Douglas via Boost
- No cmake, just bjam
Wait, what? Why would we want to remove functionality that already
exists? I like the CMake script since it generates a Visual Studio
project file (.vcxproj). Please don't take this away, its helpful.
+1

I'm not a huge fan of CMake, but then I'm not crazy about bjam either. I
think library authors should have the option of including CMake files,
just as they do files for appveyor, travis, etc. Of course bjam files
would remain a requirement.

The question of including stuff "outside" of boost is of course subject
to dispute and discussion which I guess has to be handled on a case by
case basis. In this case I think it's below the minimal threshold of
obtrusiveness. CMake files to add some "clutter", but those who don't
use/need can ignore them. So, if the author want's to, I believe he
should be able to include CMake files.

Robert Ramey

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vinnie Falco via Boost
2017-05-30 15:42:27 UTC
Permalink
On Tue, May 30, 2017 at 8:40 AM, Robert Ramey via Boost
So, if the author want's to, I believe he should be able to include CMake files
I naively assumed that the Outcome CMakeLists.txt was a
straightforward, self-contained affair. After all, how complicated
does it need to be for a library that provides 3 similar classes and a
handful of tests? I was wrong...

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-05-30 17:00:18 UTC
Permalink
Post by Vinnie Falco via Boost
I naively assumed that the Outcome CMakeLists.txt was a
straightforward, self-contained affair. After all, how complicated
does it need to be for a library that provides 3 similar classes and a
handful of tests? I was wrong...
Such a cmake would be useless for production code. You need support for
all the sanitisers and static analysis tooling; building and testing
libraries in header-only, static and dynamic lib forms, C++ Module and
non-module forms, and with C++ exceptions enabled or disabled; CTest
scripting for Travis and Appveyor CI; CPack packaging of prebuilt
binaries; documentation generation with testing of example snippet code;
CDash upload of CI test outputs and build artifacts; tracking and
maintenance of git submodule dependencies; permuting ABI for unstable
libraries - and all that is still a subset of what the common cmake
infrastructure provides for all libraries which use it.

Any reasonably mature production ready cmake project ends up
implementing its own edition of those sorts of features, thus
duplicating work and maintenance. The common cmake infrastructure
provides it for any client library based on filled in parameters. You
just fill in the parameters and voila, no more work, bye bye thinking
about the build or CI or test stuff when you make a new C++ library.

(well, once all the many bugs are ironed out, it's still pretty immature
and buggy)

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
Robert Ramey via Boost
2017-05-30 17:47:33 UTC
Permalink
Post by Niall Douglas via Boost
Post by Vinnie Falco via Boost
I naively assumed that the Outcome CMakeLists.txt was a
straightforward, self-contained affair. After all, how complicated
does it need to be for a library that provides 3 similar classes and a
handful of tests? I was wrong...
Such a cmake would be useless for production code. You need support for
all the sanitisers and static analysis tooling; building and testing
libraries in header-only, static and dynamic lib forms, C++ Module and
non-module forms, and with C++ exceptions enabled or disabled; CTest
scripting for Travis and Appveyor CI; CPack packaging of prebuilt
binaries; documentation generation with testing of example snippet code;
CDash upload of CI test outputs and build artifacts; tracking and
maintenance of git submodule dependencies; permuting ABI for unstable
libraries - and all that is still a subset of what the common cmake
infrastructure provides for all libraries which use it.
I'm not convinced it needs all that to not be "useless". I use it for
generating the IDE. It does a (barely) adequate job on this, but it's
better than trying to maintain the IDE project by hand.
Post by Niall Douglas via Boost
Any reasonably mature production ready cmake project ends up
implementing its own edition of those sorts of features, thus
duplicating work and maintenance. The common cmake infrastructure
provides it for any client library based on filled in parameters. You
just fill in the parameters and voila, no more work, bye bye thinking
about the build or CI or test stuff when you make a new C++ library.
This sounds like it would be a worthy project on it's own - perhaps as
part of boost tools. I don't think including in the scope of a
particular library is a great idea. Doing this makes reviewing the
library a much larger effort for everyone involved.

Robert Ramey
Post by Niall Douglas via Boost
(well, once all the many bugs are ironed out, it's still pretty immature
and buggy)
Niall
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vicente J. Botet Escriba via Boost
2017-05-30 15:33:30 UTC
Permalink
Post by Niall Douglas via Boost
4. Given where Vicente appears to be heading next with expected<T, E1,
E2, ...> it may be wise to make the static_checked_* varieties use
std::variant<...> for storage when being compiled on C++ 17.
https://github.com/ned14/boost.outcome/issues/53
I'm not heading nothing. All my comments about expected<T, E1, E2,
...>where if we had in the hope you defined outcome variadic ;-)

We can have C++14 (or even C++11) version of std::variant. Just someone
needs to port it.
It would be even better if we had also a never-empty variant.

Best,
Vicente

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cg
Peter Dimov via Boost
2017-05-30 16:01:36 UTC
Permalink
Post by Vicente J. Botet Escriba via Boost
We can have C++14 (or even C++11) version of std::variant. Just someone
needs to port it.
It would be even better if we had also a never-empty variant.
I'm on it.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-05-30 17:07:16 UTC
Permalink
Post by Peter Dimov via Boost
Post by Vicente J. Botet Escriba via Boost
We can have C++14 (or even C++11) version of std::variant. Just
someone needs to port it.
It would be even better if we had also a never-empty variant.
I'm on it.
The libc++ variant implementation started life as a standalone library I
believe, and can be found at https://github.com/mpark/variant. Works
right back to C++ 11 on all major compilers.

Now that is an excellent example of external code which should be
accepted into Boost without issue. Nobody can question its quality nor
provenance. Boost licensed too.

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
Peter Dimov via Boost
2017-05-30 17:18:45 UTC
Permalink
Post by Niall Douglas via Boost
Post by Peter Dimov via Boost
Post by Vicente J. Botet Escriba via Boost
We can have C++14 (or even C++11) version of std::variant. Just someone
needs to port it.
It would be even better if we had also a never-empty variant.
I'm on it.
The libc++ variant implementation started life as a standalone library I
believe, and can be found at https://github.com/mpark/variant. Works right
back to C++ 11 on all major compilers.
I know, I use it when I need an std::variant; but I wanted to implement
never-valueless.

Actually I wanted to implement expected<T, E...> but got a bit distracted
along the way. Will get to it eventually though.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Peter Dimov via Boost
2017-05-30 16:11:45 UTC
Permalink
Post by Niall Douglas via Boost
1. I have been persuaded to use longer more appropriate naming for
result<T> and outcome<T>, so now the typedefed varieties with implicit
- static_checked_outcome<T> => static_checked_optional_outcome<T>
static_checked_result<T> => static_checked_optional_result<T>
...
Post by Niall Douglas via Boost
- runtime_checked_outcome<T> => runtime_checked_optional_outcome<T>
runtime_checked_result<T> => runtime_checked_optional_result<T>
...
Post by Niall Douglas via Boost
Nobody is proposing that end users actually type out the full name each
and every time they use them, and the documentation will make that clear.
As I already said, for me this takes away the whole point of the library,
which is to provide STANDARD result types which people use in their public
APIs.

If everyone is typedefing their own varieties, this doesn't provide a
standard infrastructure. Sure, it would be useful for people who control all
of their code, but no more than that.

There should be one and only one result<T>, and one and only one outcome<T>,
with those names, which the world should use.

Let's first see if people who favor a narrow interface are satisfied with
value_if, which for me is a very good compromise. Once you have a raw
pointer, things are as narrow as it gets.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Emil Dotchevski via Boost
2017-05-30 16:34:37 UTC
Permalink
On Tue, May 30, 2017 at 9:11 AM, Peter Dimov via Boost <
Post by Peter Dimov via Boost
Post by Niall Douglas via Boost
1. I have been persuaded to use longer more appropriate naming for
result<T> and outcome<T>, so now the typedefed varieties with implicit
- static_checked_outcome<T> => static_checked_optional_outcome<T>
static_checked_result<T> => static_checked_optional_result<T>
...
Post by Niall Douglas via Boost
- runtime_checked_outcome<T> => runtime_checked_optional_outcome<T>
runtime_checked_result<T> => runtime_checked_optional_result<T>
...
Post by Niall Douglas via Boost
Nobody is proposing that end users actually type out the full name each
and every time they use them, and the documentation will make that clear.
As I already said, for me this takes away the whole point of the library,
which is to provide STANDARD result types which people use in their public
APIs.
+1

This is not unlike function<> and shared_ptr<>, what makes those types
useful at all in interfaces is the same thing that makes pointers useful in
interfaces: a T * is a T * and there aren't different kinds.

I don't even like having both outcome<> and result<>, if I understand
correctly the reason they exist is to remove perceived inefficiencies in
exception_ptr implementations. I understand that this may be important in a
library motivated by perceived inefficiencies in exception handling, but it
seems wrong to write a library to work around flawed language
implementations, inefficiencies which clearly do not exist in the language
definition.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-05-30 17:24:50 UTC
Permalink
Post by Peter Dimov via Boost
Post by Niall Douglas via Boost
1. I have been persuaded to use longer more appropriate naming for
result<T> and outcome<T>, so now the typedefed varieties with implicit
- static_checked_outcome<T> => static_checked_optional_outcome<T>
static_checked_result<T> => static_checked_optional_result<T>
...
Post by Niall Douglas via Boost
- runtime_checked_outcome<T> => runtime_checked_optional_outcome<T>
runtime_checked_result<T> => runtime_checked_optional_result<T>
...
Post by Niall Douglas via Boost
Nobody is proposing that end users actually type out the full name
each and every time they use them, and the documentation will make
that clear.
As I already said, for me this takes away the whole point of the
library, which is to provide STANDARD result types which people use in
their public APIs.
This is not and never has been the whole point of the proposed library.

Do you accept that the static checked and runtime checked varieties are
orthogonal user bases? There is a camp of users who strongly prefer no
runtime overhead and static checking. There is also a camp of users who
strongly prefer no UB possible in an uncertainty returning object.

I can see a point to both camps. Neither is obviously wrong. So seeing
as Outcome's **actual** primary goal is to provide a lightweight
universal error handling system which eases integration of arbitrary
third party libraries each with their own preferred choice of error
handling strategy, the choice to provide both options with zero cost
conversion and interoperation is the obvious one to me.

That's the whole point and vision of Outcome. The design of the one true
result type for C++ is exclusively on Vicente and WG21 LEWG to come up
with. Outcome will support whatever they decide.

I appreciate that isn't what you want for Outcome Peter. You want a
standard result type here and now. But that's Vicente's turf, best argue
for that there. Outcome is here to grease the wheels between differences
in design choice, not to declare any one right way. WG21 are better
suited to do that.

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
Peter Dimov via Boost
2017-05-30 17:43:21 UTC
Permalink
Post by Niall Douglas via Boost
Do you accept that the static checked and runtime checked varieties are
orthogonal user bases? There is a camp of users who strongly prefer no
runtime overhead and static checking.
Why are their needs not served by value_if?

auto r = function();

if( auto* p = r.value_if() )
{
// use *p
// no runtime overhead on using *p
// static checkers know use of *p implies p != nullptr
}

Between this pattern and using `assert( r.has_value() )` directly to advise
the static checker, are we not covered?


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-05-30 17:53:25 UTC
Permalink
Post by Peter Dimov via Boost
Post by Niall Douglas via Boost
Do you accept that the static checked and runtime checked varieties
are orthogonal user bases? There is a camp of users who strongly
prefer no runtime overhead and static checking.
Why are their needs not served by value_if?
auto r = function();
if( auto* p = r.value_if() )
{
// use *p
// no runtime overhead on using *p
// static checkers know use of *p implies p != nullptr
}
Between this pattern and using `assert( r.has_value() )` directly to
advise the static checker, are we not covered?
I'm still pondering your idea on this for the runtime checked editions.
So far I am liking it, but I need to sleep on it some more.

But there was still a large minority of folk who want all-narrow
observers. They haven't voiced anything to say they have changed their
minds.

I currently, roughly speaking, find approx 50% in favour of a runtime
checked edition, approx 40% in favour of a statically checked edition.

Emil and Vicente make up the 10% of folk who want something completely
different.

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
Emil Dotchevski via Boost
2017-05-30 18:05:19 UTC
Permalink
On Tue, May 30, 2017 at 10:53 AM, Niall Douglas via Boost <
Post by Niall Douglas via Boost
Post by Peter Dimov via Boost
Post by Niall Douglas via Boost
Do you accept that the static checked and runtime checked varieties
are orthogonal user bases? There is a camp of users who strongly
prefer no runtime overhead and static checking.
Why are their needs not served by value_if?
auto r = function();
if( auto* p = r.value_if() )
{
// use *p
// no runtime overhead on using *p
// static checkers know use of *p implies p != nullptr
}
Between this pattern and using `assert( r.has_value() )` directly to
advise the static checker, are we not covered?
I'm still pondering your idea on this for the runtime checked editions.
So far I am liking it, but I need to sleep on it some more.
But there was still a large minority of folk who want all-narrow
observers. They haven't voiced anything to say they have changed their
minds.
I currently, roughly speaking, find approx 50% in favour of a runtime
checked edition, approx 40% in favour of a statically checked edition.
Emil and Vicente make up the 10% of folk who want something completely
different.
It is probably not a good idea to measure correctness by popular opinion,
especially if the popular opinion originates in the group that won't use
exceptions "because they're slow". In my experience such people will always
find "Overhead!!!" in any implementation.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-05-30 19:36:42 UTC
Permalink
Post by Emil Dotchevski via Boost
Post by Niall Douglas via Boost
I currently, roughly speaking, find approx 50% in favour of a runtime
checked edition, approx 40% in favour of a statically checked edition.
Emil and Vicente make up the 10% of folk who want something completely
different.
It is probably not a good idea to measure correctness by popular opinion,
I think C++ Either result types will always fall into two camps, narrow
and wide. As you mentioned yourself Emil, the cause is the language's
current inability to specify statically correctness preconditions for
function calls. That would get a lot better if something like Herb's
metaclasses came along, but those are many, many years out, if ever.

Until then we are trapped: either put UB on the observers so it can be
trapped at compile time, or eliminate all UB so logic errors appear at
runtime. You can choose either of those per function, but you cannot
have both at once per function.

I intend to put UB "raw" observers on the runtime checked editions,
maybe using the form Peter suggested. But I am deeply opposed to having
short-to-type-out observers like operator*() do UB unless the type's
name loudly declares "I am an unsafe type".

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
Vicente J. Botet Escriba via Boost
2017-05-30 21:28:20 UTC
Permalink
Post by Niall Douglas via Boost
Post by Emil Dotchevski via Boost
Post by Niall Douglas via Boost
I currently, roughly speaking, find approx 50% in favour of a runtime
checked edition, approx 40% in favour of a statically checked edition.
Emil and Vicente make up the 10% of folk who want something completely
different.
It is probably not a good idea to measure correctness by popular opinion,
I think C++ Either result types will always fall into two camps, narrow
and wide. As you mentioned yourself Emil, the cause is the language's
current inability to specify statically correctness preconditions for
function calls. That would get a lot better if something like Herb's
metaclasses came along, but those are many, many years out, if ever.
Until then we are trapped: either put UB on the observers so it can be
trapped at compile time, or eliminate all UB so logic errors appear at
runtime. You can choose either of those per function, but you cannot
have both at once per function.
You can have two functions :)
Post by Niall Douglas via Boost
I intend to put UB "raw" observers on the runtime checked editions,
maybe using the form Peter suggested. But I am deeply opposed to having
short-to-type-out observers like operator*() do UB unless the type's
name loudly declares "I am an unsafe type".
:(

I don't understand. Aren't we on a C++ forum? on the review of a C++
library?

Do we want to banish narrow contract in this library as if this kind of
access was the leprose?

Do you want that boost.shared_ptr checks if not null before access on
operator->?

I could want that on a DEBUG version, but not on my release version.

Best,

Vicente


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cg
Peter Dimov via Boost
2017-05-30 21:49:16 UTC
Permalink
Post by Vicente J. Botet Escriba via Boost
Do you want that boost.shared_ptr checks if not null before access on
operator->?
Checking in shared_ptr::op-> is narrow, by the way, and I regret putting
that in. Should have been wide. Now we have endless debates whether to drop
the noexcept because LWG guideline. (A daft guideline if you ask me, but
that's what happens when people don't understand what the words "undefined
behavior" mean.)

To expand on that a bit:

T* operator->() const noexcept;
Requires: get() != nullptr.
Returns: get().

Narrow, a mistake.

T* operator->() const noexcept;
Returns: get().

Wide, as it should have been.

Now of course I wouldn't widen it like

T* operator->() const;
Returns: get().
Remarks: If get() == nullptr, throws bad_pointer_access().

because obviously I don't want shared_ptr to introduce overhead here over a
raw pointer.

But outcome<T> is not a pointer and is not used as such. Throwing on value
access when there's no value is actually an intended use, not something that
signals a logic error. It doesn't throw logic_error, it throws the actual
exception stored in the outcome by the producer.

All that said, I'm actually not opposed to outcome<T>::op-> being the second
above (the equivalent of value_if) and op* being effectively *operator->().
I was opposed to their existence, but if they are the price we need to pay
to placate the concerns of the group wanting a narrow interface, so be it.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Andrzej Krzemienski via Boost
2017-05-30 22:55:48 UTC
Permalink
Post by Vicente J. Botet Escriba via Boost
Do you want that boost.shared_ptr checks if not null before access on
Post by Vicente J. Botet Escriba via Boost
operator->?
Checking in shared_ptr::op-> is narrow, by the way, and I regret putting
that in. Should have been wide. Now we have endless debates whether to drop
the noexcept because LWG guideline. (A daft guideline if you ask me, but
that's what happens when people don't understand what the words "undefined
behavior" mean.)
T* operator->() const noexcept;
Requires: get() != nullptr.
Returns: get().
Narrow, a mistake.
You say, you do not like the above, right? You call this mistake *only*
because of this LWG guideline?
Post by Vicente J. Botet Escriba via Boost
T* operator->() const noexcept;
Returns: get().
Wide, as it should have been.
You say, you like the above, right? But what does it mean? That it returns
a `(T*)nullptr` ? This is the same as your value_if?

Regards,
&rzej;

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Peter Dimov via Boost
2017-05-30 23:19:08 UTC
Permalink
Post by Andrzej Krzemienski via Boost
Post by Peter Dimov via Boost
T* operator->() const noexcept;
Requires: get() != nullptr.
Returns: get().
Narrow, a mistake.
You say, you do not like the above, right? You call this mistake *only*
because of this LWG guideline?
For shared_ptr, it is a mistake, period. The function does not require get()
!= nullptr in order to do its job, so it should not have a precondition. {
return get(); } is fine.
Post by Andrzej Krzemienski via Boost
Post by Peter Dimov via Boost
T* operator->() const noexcept;
Returns: get().
Wide, as it should have been.
You say, you like the above, right? But what does it mean? That it returns
a `(T*)nullptr` ?
For shared_ptr, it just returns the pointer. For result<>, yes, it would
return a nullptr when no T is present, not an invalid pointer value.
Post by Andrzej Krzemienski via Boost
This is the same as your value_if?
Yes, as I wrote,
Post by Andrzej Krzemienski via Boost
Post by Peter Dimov via Boost
All that said, I'm actually not opposed to outcome<T>::op-> being the
second above (the equivalent of value_if) and op* being effectively
*operator->().
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-05-31 12:04:26 UTC
Permalink
Post by Vicente J. Botet Escriba via Boost
Post by Niall Douglas via Boost
I intend to put UB "raw" observers on the runtime checked editions,
maybe using the form Peter suggested. But I am deeply opposed to having
short-to-type-out observers like operator*() do UB unless the type's
name loudly declares "I am an unsafe type".
:(
I don't understand. Aren't we on a C++ forum? on the review of a C++
library?
Do we want to banish narrow contract in this library as if this kind of
access was the leprose?
People have the statically checked varieties available to them if they
want narrow contracts.

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


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo
Vicente J. Botet Escriba via Boost
2017-05-31 20:08:06 UTC
Permalink
Post by Niall Douglas via Boost
Post by Vicente J. Botet Escriba via Boost
Post by Niall Douglas via Boost
I intend to put UB "raw" observers on the runtime checked editions,
maybe using the form Peter suggested. But I am deeply opposed to having
short-to-type-out observers like operator*() do UB unless the type's
name loudly declares "I am an unsafe type".
:(
I don't understand. Aren't we on a C++ forum? on the review of a C++
library?
Do we want to banish narrow contract in this library as if this kind of
access was the leprose?
People have the statically checked varieties available to them if they
want narrow contracts.
What if I want both?

Vicente

_______________________________________________
Unsubscribe & other c
Niall Douglas via Boost
2017-05-31 20:45:41 UTC
Permalink
Post by Vicente J. Botet Escriba via Boost
Post by Niall Douglas via Boost
Post by Vicente J. Botet Escriba via Boost
Do we want to banish narrow contract in this library as if this kind of
access was the leprose?
People have the statically checked varieties available to them if they
want narrow contracts.
What if I want both?
You can't have a statically checked function which is always legal to
call. The static analyser couldn't claim calling it to be suspicious.

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


_______________________________________________
Unsubscribe & other changes: htt
Vicente J. Botet Escriba via Boost
2017-05-31 20:57:40 UTC
Permalink
Post by Niall Douglas via Boost
Post by Vicente J. Botet Escriba via Boost
Post by Niall Douglas via Boost
Post by Vicente J. Botet Escriba via Boost
Do we want to banish narrow contract in this library as if this kind of
access was the leprose?
People have the statically checked varieties available to them if they
want narrow contracts.
What if I want both?
You can't have a statically checked function which is always legal to
call. The static analyser couldn't claim calling it to be suspicious.
Sorry, I'm tired to try to explain you the advantages of having both
functions one with wide and and the other with narrow contract.
The narrow function is not always legal to cal as it has a narrow contract.


Hopping someone else could have more chance than me.

Best,
Vicente

_______________________________________________
Unsubscribe & other chang
Andrzej Krzemienski via Boost
2017-06-01 07:14:03 UTC
Permalink
2017-05-31 22:57 GMT+02:00 Vicente J. Botet Escriba via Boost <
Post by Vicente J. Botet Escriba via Boost
Post by Niall Douglas via Boost
Post by Niall Douglas via Boost
Post by Vicente J. Botet Escriba via Boost
Do we want to banish narrow contract in this library as if this kind of
access was the leprose?
People have the statically checked varieties available to them if they
want narrow contracts.
What if I want both?
You can't have a statically checked function which is always legal to
call. The static analyser couldn't claim calling it to be suspicious.
Sorry, I'm tired to try to explain you the advantages of having both
functions one with wide and and the other with narrow contract.
The narrow function is not always legal to cal as it has a narrow contract.
Hopping someone else could have more chance than me.
Both narrow- and wide-contract accessors are provided in std::optional. The
rationale for this is that decision whether I want "just value" or whether
I want the lack of value to be treated as a run-time error is best made at
the point where the value is used: not at the point where the type of the
object is being decided, or at the point where object is constructed.

Back in the days where `boost::optional` was being designed, some people
have suggested that the fall-back value (in case optional is empty) is
provided upon construction:

```
// not boost::optional:
optional<int> o {fallback_value(-1)};
o = 2;
assert (*o == 2);
o = none;
assert (*o == -1);
```

But again that it was decided that fall-back value is better determined at
the call site, not at object construction site. Hence we have `value_or`.

Regards,
&rzej;

_______________________________________________
Unsubscribe & other changes: http
Vicente J. Botet Escriba via Boost
2017-05-30 21:19:19 UTC
Permalink
Post by Niall Douglas via Boost
Post by Peter Dimov via Boost
Post by Niall Douglas via Boost
Do you accept that the static checked and runtime checked varieties
are orthogonal user bases? There is a camp of users who strongly
prefer no runtime overhead and static checking.
Why are their needs not served by value_if?
auto r = function();
if( auto* p = r.value_if() )
{
// use *p
// no runtime overhead on using *p
// static checkers know use of *p implies p != nullptr
}
Between this pattern and using `assert( r.has_value() )` directly to
advise the static checker, are we not covered?
I'm still pondering your idea on this for the runtime checked editions.
So far I am liking it, but I need to sleep on it some more.
But there was still a large minority of folk who want all-narrow
observers. They haven't voiced anything to say they have changed their
minds.
I currently, roughly speaking, find approx 50% in favour of a runtime
checked edition, approx 40% in favour of a statically checked edition.
Emil and Vicente make up the 10% of folk who want something completely
different.
I say that we don't have to choose. We can provide as we have already
done in a lot of libraries provide wide and narrow functions. No need to
split the world on two parts. No silos.

value_if could be a complementary/helper interface. We don't need to
provide them on all the types, but as algorithms. The implementation is
the same for optional, expected, result, ....

The same for
T nv = value_or(pv, v);
E ne = error_or(pe, e);
bool b = has_error(pe, e);

IMHO, most of the wide function constracts can be provided this way.
There are some exceptions e.g. value that throw a different exception
depending on the type, but this is because we don't have found yet the
minimal interface.
E.g. If we had a pe.rethrow() function, we could define value by in
function of has_value, operator* and rethrow.


Vicente


_______________________________________________
Unsubscribe & other changes: http://lists.boos
Vicente J. Botet Escriba via Boost
2017-05-30 21:02:40 UTC
Permalink
Post by Niall Douglas via Boost
Post by Peter Dimov via Boost
Post by Niall Douglas via Boost
1. I have been persuaded to use longer more appropriate naming for
result<T> and outcome<T>, so now the typedefed varieties with implicit
- static_checked_outcome<T> => static_checked_optional_outcome<T>
static_checked_result<T> => static_checked_optional_result<T>
...
Post by Niall Douglas via Boost
- runtime_checked_outcome<T> => runtime_checked_optional_outcome<T>
runtime_checked_result<T> => runtime_checked_optional_result<T>
...
Post by Niall Douglas via Boost
Nobody is proposing that end users actually type out the full name
each and every time they use them, and the documentation will make
that clear.
As I already said, for me this takes away the whole point of the
library, which is to provide STANDARD result types which people use in
their public APIs.
This is not and never has been the whole point of the proposed library.
Do you accept that the static checked and runtime checked varieties are
orthogonal user bases? There is a camp of users who strongly prefer no
runtime overhead and static checking. There is also a camp of users who
strongly prefer no UB possible in an uncertainty returning object.
I can see a point to both camps. Neither is obviously wrong. So seeing
as Outcome's **actual** primary goal is to provide a lightweight
universal error handling system which eases integration of arbitrary
third party libraries each with their own preferred choice of error
handling strategy, the choice to provide both options with zero cost
conversion and interoperation is the obvious one to me.
But a class can have both observer functions: narrow and wide . Why do
we need two classes?
Post by Niall Douglas via Boost
That's the whole point and vision of Outcome. The design of the one true
result type for C++ is exclusively on Vicente and WG21 LEWG to come up
with. Outcome will support whatever they decide.
I don't understand why you say that.
Post by Niall Douglas via Boost
I appreciate that isn't what you want for Outcome Peter. You want a
standard result type here and now. But that's Vicente's turf, best argue
for that there. Outcome is here to grease the wheels between differences
in design choice, not to declare any one right way. WG21 are better
suited to do that.
I believe Peter want a Result type independently of what the standard
will provide one day, and he wants it now and not in 2020. Maybe this
Result type could one day a candidate for the standard. Could you
confirm, Peter?

Vicente

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailm
Peter Dimov via Boost
2017-05-30 21:18:14 UTC
Permalink
I believe Peter want a Result type independently of what the standard will
provide one day, and he wants it now and not in 2020. Maybe this Result
type could one day a candidate for the standard. Could you confirm, Peter?
As I see things, we have a very good std::error_code infrastructure sitting
(underused) in <system_error>, we have a good outline of how it's used in
<filesystem>, and there we have the whole API doubled as in

void api_function( ... ); // throws system_error
void api_function( ..., error_code& ec ); // doesn't throw

for example

bool exists( const std::filesystem::path& p );
bool exists( const std::filesystem::path& p, std::error_code& ec )
noexcept;

I also see that an std::result (or std::outcome, or std::result with outcome
semantics as per Emil) would allow us to express that as

std::result<bool> exists( const std::filesystem::path& p ) noexcept;

with no apparent loss of functionality. (*)

Getting people a library that has a multitude of result classes would
obviously be better than nothing, but how would it move us toward the goal
of having std::result?

How Boost originally worked was, when there is disagreement as to how some
component should work, we hammer out our differences here, produce a
consensus design, produce, as a result of our process, a rationale of why we
arrived at this consensus and how, get the library into the hands of our
users, listen to their feedback, refine as necessary, then hand the finished
product to the LWG.

When some people want X, others Y, a third group Z, it's always the path of
least resistance to just put X+Y+Z into the library, or a policy-based
factory that can create 6^11 classes, among them X, Y, and Z. But that's
really a cop-out.

(*) Not quite because the straightforward implementation would throw
system_error instead of filesystem_error on value() and we'll lose the path,
which is an interesting angle that we need to explore.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-05-31 11:26:06 UTC
Permalink
Post by Peter Dimov via Boost
Getting people a library that has a multitude of result classes would
obviously be better than nothing, but how would it move us toward the
goal of having std::result?
How Boost originally worked was, when there is disagreement as to how
some component should work, we hammer out our differences here, produce
a consensus design, produce, as a result of our process, a rationale of
why we arrived at this consensus and how, get the library into the hands
of our users, listen to their feedback, refine as necessary, then hand
the finished product to the LWG.
When some people want X, others Y, a third group Z, it's always the path
of least resistance to just put X+Y+Z into the library, or a
policy-based factory that can create 6^11 classes, among them X, Y, and
Z. But that's really a cop-out.
You may remember back at the very beginning of this review I said that
these objects were different to normal C++ design principles, there was
a multitude of ways of using them and hence so many varieties of usage
style in their API. I said that for most of my libraries, I decide on
one single clean design and use model, but for this library I really did
feel that multi-modal usage was more appropriate.

Thanks to this review we have reduced down that multi-modal API into
simplified categories, and hived those out into separate types so the
type tells the user the semantic, all with a substantial reduction in
member function count. All a definite improvement in my opinion.

I don't agree that this is a cop-out. I believe that empty vs non-empty
is a very obvious design improvement, it significantly improves the
clarity of contract of public APIs using these objects by indicating
whether an empty state can ever be returned.

The wide vs narrow is much less obvious. During these last few days to
put code to the problem, I put together a toy Outcome implementation
using nothing but templates and std::variant<> (my very first ever C++
17 program yay!) using:

// Statically checked T|error_code
template <class T>
using static_checked_result =
outcome<T, std::error_code, void, default_to::disabled,
emptiness::never, observers::narrow>;

// Runtime checked T|error_code
template <class T>
using runtime_checked_result =
outcome<T, std::error_code, void, default_to::empty,
emptiness::formal, observers::wide>;

... and tried using them in various use cases. I can find absolutely no
difference in runtime overhead. So that makes the sole difference to be
whether you prefer the compiler to tell you about the obvious null
pointer dereference in *std::get_if(&variant) if it's clever enough, or
whether you prefer some runtime default action to occur instead of
dereferencing a null pointer which is an instant segfault.

The former traps logic errors missed by the compiler at runtime
effectively. The latter lets the programmer skip typing out boilerplate.

But flipping the perspective, the former means your program may die
unexpectedly in production due to some untested branch combination. The
latter hides logic errors and therefore programmer bugs.

I know you think this a cop out, but I think those two are unresolvable.
Outcome may simply choose to implement one set and not the other out of
ideological purity, but I think that a "cut off one's nose to spite
one's face" type of decision.

It's a different form of cop out: we can't resolve this, so we choose
one and screw the other.
Post by Peter Dimov via Boost
(*) Not quite because the straightforward implementation would throw
system_error instead of filesystem_error on value() and we'll lose the
path, which is an interesting angle that we need to explore.
What we really actually need is an error_code which can carry payload.
Lawrence's status_code is heading the right way, but I am not keen on
his formulation. I have a gut feeling that we can do better, though
avoiding malloc and keeping the object useful is hard.

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
Vicente J. Botet Escriba via Boost
2017-06-04 22:12:52 UTC
Permalink
Post by Peter Dimov via Boost
Post by Vicente J. Botet Escriba via Boost
I believe Peter want a Result type independently of what the standard
will provide one day, and he wants it now and not in 2020. Maybe this
Result type could one day a candidate for the standard. Could you
confirm, Peter?
As I see things, we have a very good std::error_code infrastructure
sitting (underused) in <system_error>, we have a good outline of how
it's used in <filesystem>, and there we have the whole API doubled as in
void api_function( ... ); // throws system_error
void api_function( ..., error_code& ec ); // doesn't throw
for example
bool exists( const std::filesystem::path& p );
bool exists( const std::filesystem::path& p, std::error_code& ec )
noexcept;
I also see that an std::result (or std::outcome, or std::result with
outcome semantics as per Emil) would allow us to express that as
std::result<bool> exists( const std::filesystem::path& p ) noexcept;
with no apparent loss of functionality. (*)
We have yet some trouble with constructors. We need factories instead :(
Post by Peter Dimov via Boost
Getting people a library that has a multitude of result classes would
obviously be better than nothing, but how would it move us toward the
goal of having std::result?
How Boost originally worked was, when there is disagreement as to how
some component should work, we hammer out our differences here,
produce a consensus design, produce, as a result of our process, a
rationale of why we arrived at this consensus and how, get the library
into the hands of our users, listen to their feedback, refine as
necessary, then hand the finished product to the LWG.
When some people want X, others Y, a third group Z, it's always the
path of least resistance to just put X+Y+Z into the library, or a
policy-based factory that can create 6^11 classes, among them X, Y,
and Z. But that's really a cop-out.
(*) Not quite because the straightforward implementation would throw
system_error instead of filesystem_error on value() and we'll lose the
path, which is an interesting angle that we need to explore.
If we want to be able to throw a specific exception, we need to ensure a
single mapping from E to Exception. We can do it in two ways

Wrap E so that the conversion to Exception can be done, e.g. instead of
std::result<bool> we could have expected<bool, filesystem::error_code>
filesystem::error_code is a wrapper for error_code that defines the
rethrow function throwing filesystem_error.

Again the problem is that now we need to unwrap filesystem::error_code.

Vicente

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mail
Peter Dimov via Boost
2017-06-04 23:08:59 UTC
Permalink
Vicente J. Botet Escriba wrote:
...
Post by Vicente J. Botet Escriba via Boost
Wrap E so that the conversion to Exception can be done, e.g. instead of
std::result<bool> we could have expected<bool, filesystem::error_code>
filesystem::error_code is a wrapper for error_code that defines the
rethrow function throwing filesystem_error.
The problem here is not just that we have to throw filesystem_error; the
problem is that the point of throwing filesystem_error instead of
system_error is that it contains the path (two paths even, although we only
have one in this case), and there's no path in error_code.

If all we had was error_code, the right exception to throw is system_error,
because that's what it's for.

The correct approach here, I think, is not to define fat error codes, but to
return an outcome whose error() is the error_code and whose exception() is
the appropriate filesystem_error exception, complete with the path(s).

Setting however both the error_code and the exception_ptr is not supported
by the current outcome design (and neither would it be supported by
expected<R, error_code, exception_ptr>.)

Or, an alternative that scales better would be to take

http://pdimov.com/cpp2/P0640R0.pdf

and store error_code and exception_info, so that later one can use

throw_with_info( std::system_error( error() ), info );

with the additional information (such as paths) being carried by the
exception_info.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-06-05 11:02:54 UTC
Permalink
Post by Peter Dimov via Boost
The correct approach here, I think, is not to define fat error codes,
but to return an outcome whose error() is the error_code and whose
exception() is the appropriate filesystem_error exception, complete with
the path(s).
Setting however both the error_code and the exception_ptr is not
supported by the current outcome design (and neither would it be
supported by expected<R, error_code, exception_ptr>.)
Or, an alternative that scales better would be to take
http://pdimov.com/cpp2/P0640R0.pdf
and store error_code and exception_info, so that later one can use
throw_with_info( std::system_error( error() ), info );
with the additional information (such as paths) being carried by the
exception_info.
In AFIO v2 I'd personally consider any potential memory allocation in
any of the core API unacceptable (because we never allocate memory in
AFIO v2). Currently we memcpy() the tail of the path into the
error_code_extended on failure, but even that I dislike because each
afio::io_handle already knows its own path, and if it weren't for
lifetime issues I'd just return the pointer to that path and save a lot
of work for everybody.

Down the line AFIO v2 will gain a "fat path" type which is the
combination of an open handle with a path fragment. Those necessarily
will need to decide what to do about lifetime issues. I may yet go light
and make the fat path type just a naked pointer plus filesystem::path,
or I may implement a process-wide registry of "path handles" i.e.
handles opened solely to indicate some unknown location on the
filesystem which is a base for path fragments to work from. I haven't
decided yet, there are a lot of competing tradeoffs, I specifically want
the end user to do all their malloc-work in a pre-init step and away
from any core i/o loops which need to never, ever allocate memory.

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
Vicente J. Botet Escriba via Boost
2017-06-05 14:38:50 UTC
Permalink
Post by Peter Dimov via Boost
...
Post by Vicente J. Botet Escriba via Boost
Wrap E so that the conversion to Exception can be done, e.g. instead
of std::result<bool> we could have expected<bool,
filesystem::error_code>
filesystem::error_code is a wrapper for error_code that defines the
rethrow function throwing filesystem_error.
The problem here is not just that we have to throw filesystem_error;
the problem is that the point of throwing filesystem_error instead of
system_error is that it contains the path (two paths even, although we
only have one in this case), and there's no path in error_code.
If all we had was error_code, the right exception to throw is
system_error, because that's what it's for.
The correct approach here, I think, is not to define fat error codes,
but to return an outcome whose error() is the error_code and whose
exception() is the appropriate filesystem_error exception, complete
with the path(s).
Setting however both the error_code and the exception_ptr is not
supported by the current outcome design (and neither would it be
supported by expected<R, error_code, exception_ptr>.)
Or, an alternative that scales better would be to take
http://pdimov.com/cpp2/P0640R0.pdf
I'll take a look later.
Post by Peter Dimov via Boost
and store error_code and exception_info, so that later one can use
throw_with_info( std::system_error( error() ), info );
with the additional information (such as paths) being carried by the
exception_info.
Oh, in this case it is clear that result<T> is not enough. We need
expected<T, filesystem::error>

where filesystem:error contains the error_code and the path.

Vicente

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/
Peter Dimov via Boost
2017-06-05 14:46:25 UTC
Permalink
Post by Vicente J. Botet Escriba via Boost
Oh, in this case it is clear that result<T> is not enough. We need
expected<T, filesystem::error>
where filesystem:error contains the error_code and the path.
I don't find this particularly appealing, because the whole point of
std::error_code was to avoid the need for each library to define separate
error classes. expected<T, filesystem::error> will work, I'd just not be
willing to recommend it as a general solution.

Something like Niall's new outcome<T> that stores both an error_code and an
exception seems better, because you can use a single common return type in
all libraries.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vicente J. Botet Escriba via Boost
2017-06-05 18:31:01 UTC
Permalink
Post by Peter Dimov via Boost
Post by Vicente J. Botet Escriba via Boost
Oh, in this case it is clear that result<T> is not enough. We need
expected<T, filesystem::error>
where filesystem:error contains the error_code and the path.
I don't find this particularly appealing, because the whole point of
std::error_code was to avoid the need for each library to define
separate error classes. expected<T, filesystem::error> will work, I'd
just not be willing to recommend it as a general solution.
Something like Niall's new outcome<T> that stores both an error_code
and an exception seems better, because you can use a single common
return type in all libraries.
Let define filesystem::error as we consider is the more pertinent. You
don't need a specific expected/outcome/xxxxx class to cover this case.
Do you?

Vicente

_______________________________________________
Unsubscribe & other changes: http://lists
Gavin Lambert via Boost
2017-06-06 02:35:13 UTC
Permalink
Post by Peter Dimov via Boost
The correct approach here, I think, is not to define fat error
codes, but to return an outcome whose error() is the error_code and
whose exception() is the appropriate filesystem_error exception,
complete with the path(s).
This sounds like the design I was suggesting in the "to variant or not
to variant" thread.
Post by Peter Dimov via Boost
Post by Vicente J. Botet Escriba via Boost
Oh, in this case it is clear that result<T> is not enough. We need
expected<T, filesystem::error>
where filesystem:error contains the error_code and the path.
I don't find this particularly appealing, because the whole point of
std::error_code was to avoid the need for each library to define
separate error classes. expected<T, filesystem::error> will work, I'd
just not be willing to recommend it as a general solution.
Is it more attractive if perhaps we had a result<T, E> where E was
constrained to a type derived from std::error_code? (I'm imagining a
sort of orthogonal hierarchy with types derived from std::exception,
although not for error-identification purposes, just to carry additional
data payloads.) Implicit slicing would allow this to be used
generically in contexts where an error_code is expected.

The error_code design is a little unfortunate since users are encouraged
to pass them around by value, which would slice off additional context
information. OTOH even if passed around by reference so the information
is preserved, it wouldn't be readily accessible without dynamic_casts.
(The same is true for exceptions as well, but some language/compiler
conspiracy hides this.)

The idea of error_code having an arbitrary error-info pointer seems
attractive, but the above semantics would probably require it being a
shared_ptr<void>, which would probably annoy everybody for various
reasons (including type safety, atomics, and memory allocation).

Niall's error_code_extended is a sort of half-way point where it
provides some additional fixed data payload which is probably useful in
many cases as a compromise type; additionally defined as POD to avoid
dynamic allocation and so that the ring buffer stomping over the data
doesn't upset too many things. It's a good idea, and I'm not sure if we
can come up with something better (other than arguing about what members
it should have), but perhaps there are some other possibilities to consider.

Perhaps rather than allowing arbitrary Es we could have a single E that
still provides some additional flexibility:

template<typename EI>
struct error_code_info : public error_code
{
// ...
const optional<EI>& error_info() const;
// ...
optional<EI> m_error_info;
};

template<typename T, typename EI>
using result = expected_impl<T, error_code_info<EI>>;

(Again, just a sketch to present an idea; don't get too hung up on the
specifics.)

Could something like this work? An error can be specified with an
arbitrary data payload, and yet consumers could just slice that off and
treat it as a plain error_code if they want. Meanwhile the
result/expected implementation can rely on noexcept/move guarantees
provided by error_code_info.

The main complication I see with this is that you probably don't want to
over-constrain the EI type, to simplify usage -- that's why I suggested
it store optional<EI> rather than EI directly, so that error_code_info
could be noexcept in all cases; if EI doesn't have noexcept move/copy
then error_code_info just discards the info if EI's constructor throws.

(So this does allow an empty state and the empty state should not be
considered weird; consumer code just needs to tolerate that like they
would any other optional<>.)

This does mean that consumers can introduce memory allocation into their
error codes (probably via std::string), which some people won't like --
but this leaves the choice of doing that or avoiding that up to them
(and the libraries they choose to consume).

The main downside I see of this is that it's less straightforward to
pass around error_code_info<EI>s than error_codes, but since this should
be mostly confined to a well defined call chain (with specific concrete
EI values rather than generics) I hope this wouldn't be a problem in
actual practice. (Using a shared_ptr<void> would avoid that issue but I
think that's probably worse overall.)

It might also result in a proliferation of EI types (as each method of
each library could conceivably define a unique one) but again I doubt
that should be an issue in practice.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Andrzej Krzemienski via Boost
2017-06-06 08:45:34 UTC
Permalink
Post by Peter Dimov via Boost
The correct approach here, I think, is not to define fat error
codes, but to return an outcome whose error() is the error_code and
whose exception() is the appropriate filesystem_error exception,
complete with the path(s).
This sounds like the design I was suggesting in the "to variant or not to
variant" thread.
Post by Peter Dimov via Boost
Oh, in this case it is clear that result<T> is not enough. We need
Post by Vicente J. Botet Escriba via Boost
expected<T, filesystem::error>
where filesystem:error contains the error_code and the path.
I don't find this particularly appealing, because the whole point of
std::error_code was to avoid the need for each library to define separate
error classes. expected<T, filesystem::error> will work, I'd just not be
willing to recommend it as a general solution.
Is it more attractive if perhaps we had a result<T, E> where E was
constrained to a type derived from std::error_code? (I'm imagining a sort
of orthogonal hierarchy with types derived from std::exception, although
not for error-identification purposes, just to carry additional data
payloads.) Implicit slicing would allow this to be used generically in
contexts where an error_code is expected.
The error_code design is a little unfortunate since users are encouraged
to pass them around by value, which would slice off additional context
information. OTOH even if passed around by reference so the information is
preserved, it wouldn't be readily accessible without dynamic_casts. (The
same is true for exceptions as well, but some language/compiler conspiracy
hides this.)
The idea of error_code having an arbitrary error-info pointer seems
attractive, but the above semantics would probably require it being a
shared_ptr<void>, which would probably annoy everybody for various reasons
(including type safety, atomics, and memory allocation).
Niall's error_code_extended is a sort of half-way point where it provides
some additional fixed data payload which is probably useful in many cases
as a compromise type; additionally defined as POD to avoid dynamic
allocation and so that the ring buffer stomping over the data doesn't upset
too many things. It's a good idea, and I'm not sure if we can come up with
something better (other than arguing about what members it should have),
but perhaps there are some other possibilities to consider.
Perhaps rather than allowing arbitrary Es we could have a single E that
template<typename EI>
struct error_code_info : public error_code
{
// ...
const optional<EI>& error_info() const;
// ...
optional<EI> m_error_info;
};
template<typename T, typename EI>
using result = expected_impl<T, error_code_info<EI>>;
(Again, just a sketch to present an idea; don't get too hung up on the
specifics.)
Could something like this work? An error can be specified with an
arbitrary data payload, and yet consumers could just slice that off and
treat it as a plain error_code if they want. Meanwhile the result/expected
implementation can rely on noexcept/move guarantees provided by
error_code_info.
The main complication I see with this is that you probably don't want to
over-constrain the EI type, to simplify usage -- that's why I suggested it
store optional<EI> rather than EI directly, so that error_code_info could
be noexcept in all cases; if EI doesn't have noexcept move/copy then
error_code_info just discards the info if EI's constructor throws.
(So this does allow an empty state and the empty state should not be
considered weird; consumer code just needs to tolerate that like they would
any other optional<>.)
This does mean that consumers can introduce memory allocation into their
error codes (probably via std::string), which some people won't like -- but
this leaves the choice of doing that or avoiding that up to them (and the
libraries they choose to consume).
The main downside I see of this is that it's less straightforward to pass
around error_code_info<EI>s than error_codes, but since this should be
mostly confined to a well defined call chain (with specific concrete EI
values rather than generics) I hope this wouldn't be a problem in actual
practice. (Using a shared_ptr<void> would avoid that issue but I think
that's probably worse overall.)
It might also result in a proliferation of EI types (as each method of
each library could conceivably define a unique one) but again I doubt that
should be an issue in practice.
But does this preserve the guarantee that result<T, E> is trivially
copyable?

Another alternative to consider is to change something in the ring buffer
records, so that it resembles:

```
struct RingBufferRecord
{
enum {/*name different kind of payloads*/} _discriminator;
union
{
/*define different kinds of payload*/
}
};
```

This way the enumeration tells you how to reinterpret the data. You do not
know all possible payloads when designing the ring buffer, so you probably
need to introduce something akin to virtual functions:

```
struct RingBufferRecord
{
aligned_storage<256> _raw_storage;
void (*_interpret_raw_storage) (aligned_storage<256>&);
};
```

I admit this idea is not well thought over.

Regards,
&rzej;

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-06-06 09:23:51 UTC
Permalink
Post by Andrzej Krzemienski via Boost
But does this preserve the guarantee that result<T, E> is trivially
copyable?
Also, if you want the TRY operation to work (and you really, really do,
it is an enormous boilerplate saver), then your error type needs to be
identical throughout your program. Templating it breaks that.
Post by Andrzej Krzemienski via Boost
Another alternative to consider is to change something in the ring buffer
```
struct RingBufferRecord
{
enum {/*name different kind of payloads*/} _discriminator;
union
{
/*define different kinds of payload*/
}
};
```
This way the enumeration tells you how to reinterpret the data. You do not
know all possible payloads when designing the ring buffer, so you probably
```
struct RingBufferRecord
{
aligned_storage<256> _raw_storage;
void (*_interpret_raw_storage) (aligned_storage<256>&);
};
```
I admit this idea is not well thought over.
You can probably set a custom ring buffer design per process, but as it
would cause error_code_extended's type signature to change in order to
prevent ABI collision, going finer grained than that probably wouldn't
be practical as again you'd lose the TRY operation.

I am currently minded to have error_code_extended no longer store a 191
byte string which lets me pack many more of them into a reasonably sized
ringbuffer. If people want to store a string, I am minded they should be
either supplying a static const char * or else return an outcome<T> with
shared_ptr payload to strings.

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
Gavin Lambert via Boost
2017-06-06 23:02:05 UTC
Permalink
Post by Niall Douglas via Boost
Also, if you want the TRY operation to work (and you really, really do,
it is an enormous boilerplate saver), then your error type needs to be
identical throughout your program. Templating it breaks that.
Why does it need to be identical? Can't you just use
decltype(r.error())? As long as you know that at minimum it supports
the error_code interface this should be sufficient for most purposes.
Post by Niall Douglas via Boost
I am currently minded to have error_code_extended no longer store a 191
byte string which lets me pack many more of them into a reasonably sized
ringbuffer. If people want to store a string, I am minded they should be
either supplying a static const char * or else return an outcome<T> with
shared_ptr payload to strings.
How would they do such a payload for an error case?


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-06-07 10:43:12 UTC
Permalink
Post by Gavin Lambert via Boost
Post by Niall Douglas via Boost
Also, if you want the TRY operation to work (and you really, really do,
it is an enormous boilerplate saver), then your error type needs to be
identical throughout your program. Templating it breaks that.
Why does it need to be identical? Can't you just use
decltype(r.error())? As long as you know that at minimum it supports
the error_code interface this should be sufficient for most purposes.
It doesn't work unfortunately because you need to convert from
error_code interface compliant type A to error_code interface compliant
type B, and what does "convert" mean exactly? Remember, the TRY
implementation can have no knowledge of what the return type of the
enclosing function is. It has to return something, safely, which will
propagate the error code correctly.

Sure, the programmer can build in conversion routines between error code
interface types, we could even use a free function based design so
arbitrary third party controlled types can be converted from and to. But
to be honest at this stage of complexity you're better off using exceptions.
Post by Gavin Lambert via Boost
Post by Niall Douglas via Boost
I am currently minded to have error_code_extended no longer store a 191
byte string which lets me pack many more of them into a reasonably sized
ringbuffer. If people want to store a string, I am minded they should be
either supplying a static const char * or else return an outcome<T> with
shared_ptr payload to strings.
How would they do such a payload for an error case?
For result<T>, your payload will probably be restricted to two 64 bit
pointers. Up to you what those mean (and how you'd garbage collect
them). I may allow people to create a secondary ring buffer and
implement some sort of chaining facility, that way they could set up a
fixed sized string facility if they wanted to.

For outcome<T>, use the shared_ptr facility to deliver arbitrary
type-erased payload.

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
Gottlob Frege via Boost
2017-06-07 15:02:38 UTC
Permalink
On Wed, Jun 7, 2017 at 6:43 AM, Niall Douglas via Boost
Post by Niall Douglas via Boost
Post by Gavin Lambert via Boost
Post by Niall Douglas via Boost
Also, if you want the TRY operation to work (and you really, really do,
it is an enormous boilerplate saver), then your error type needs to be
identical throughout your program. Templating it breaks that.
Why does it need to be identical? Can't you just use
decltype(r.error())? As long as you know that at minimum it supports
the error_code interface this should be sufficient for most purposes.
It doesn't work unfortunately because you need to convert from
error_code interface compliant type A to error_code interface compliant
type B, and what does "convert" mean exactly? Remember, the TRY
implementation can have no knowledge of what the return type of the
enclosing function is.
There was talk (not sure if it was an official proposal or just a
suggestion) of something like
decltype(return)
for getting the type returned from the current function.
Post by Niall Douglas via Boost
It has to return something, safely, which will
propagate the error code correctly.
Sure, the programmer can build in conversion routines between error code
interface types, we could even use a free function based design so
arbitrary third party controlled types can be converted from and to. But
to be honest at this stage of complexity you're better off using exceptions.
Post by Gavin Lambert via Boost
Post by Niall Douglas via Boost
I am currently minded to have error_code_extended no longer store a 191
byte string which lets me pack many more of them into a reasonably sized
ringbuffer. If people want to store a string, I am minded they should be
either supplying a static const char * or else return an outcome<T> with
shared_ptr payload to strings.
How would they do such a payload for an error case?
For result<T>, your payload will probably be restricted to two 64 bit
pointers. Up to you what those mean (and how you'd garbage collect
them). I may allow people to create a secondary ring buffer and
implement some sort of chaining facility, that way they could set up a
fixed sized string facility if they wanted to.
For outcome<T>, use the shared_ptr facility to deliver arbitrary
type-erased payload.
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
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Gavin Lambert via Boost
2017-06-07 23:10:16 UTC
Permalink
Post by Gottlob Frege via Boost
Post by Niall Douglas via Boost
Post by Gavin Lambert via Boost
Post by Niall Douglas via Boost
Also, if you want the TRY operation to work (and you really, really do,
it is an enormous boilerplate saver), then your error type needs to be
identical throughout your program. Templating it breaks that.
Why does it need to be identical? Can't you just use
decltype(r.error())? As long as you know that at minimum it supports
the error_code interface this should be sufficient for most purposes.
It doesn't work unfortunately because you need to convert from
error_code interface compliant type A to error_code interface compliant
type B, and what does "convert" mean exactly? Remember, the TRY
implementation can have no knowledge of what the return type of the
enclosing function is.
There was talk (not sure if it was an official proposal or just a
suggestion) of something like
decltype(return)
for getting the type returned from the current function.
Even without that, couldn't TRY accept the return type as a parameter?
Or perhaps rather than returning directly it could output to a parameter
that the enclosing code declares to be the same as the return type
(especially since you say you're using this two-phase in many cases anyway).

Besides, if the intent is to convey the payload up the call chain, then
it can be left to the responsibility of the method itself to forward
that appropriately -- ie. if you call something that can return an
error_code_info<EI1> then you are required to do one of the following:

1. Deal with the error locally, not calling TRY.
2. Call TRY to pass on the error unmodified, thereby requiring that
you also return an error_code_info<EI1>.
3. Pass on the error, but returning error_code_info<EI2> where EI2 is
constructible from EI1 (forwarding the original payload plus some
additional context). This might be a special case of #1.

(By "returning" here I don't mean directly, I mean wrapped in a result<>
or outcome<>.)


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-06-08 11:36:19 UTC
Permalink
Post by Gavin Lambert via Boost
Post by Gottlob Frege via Boost
Post by Niall Douglas via Boost
Post by Gavin Lambert via Boost
Why does it need to be identical? Can't you just use
decltype(r.error())? As long as you know that at minimum it supports
the error_code interface this should be sufficient for most purposes.
It doesn't work unfortunately because you need to convert from
error_code interface compliant type A to error_code interface compliant
type B, and what does "convert" mean exactly? Remember, the TRY
implementation can have no knowledge of what the return type of the
enclosing function is.
There was talk (not sure if it was an official proposal or just a
suggestion) of something like
decltype(return)
for getting the type returned from the current function.
Even without that, couldn't TRY accept the return type as a parameter?
It doesn't especially help you in practice. What Outcome currently does
is return magic type sugar from TRY which is implicitly constructible by
any basic_monad instance, this works around needing to know what the
return type is for the calling function in the TRY implementation.
Post by Gavin Lambert via Boost
Besides, if the intent is to convey the payload up the call chain, then
it can be left to the responsibility of the method itself to forward
that appropriately -- ie. if you call something that can return an
1. Deal with the error locally, not calling TRY.
2. Call TRY to pass on the error unmodified, thereby requiring that
you also return an error_code_info<EI1>.
3. Pass on the error, but returning error_code_info<EI2> where EI2 is
constructible from EI1 (forwarding the original payload plus some
additional context). This might be a special case of #1.
(By "returning" here I don't mean directly, I mean wrapped in a result<>
or outcome<>.)
Of course you could do all that.

What I'm saying is that for 95% of programmers, when facing the choice
of whether to use different E types in exchange for typing a ton load
more boilerplate all day long every day in everything they write, they
will - I guarantee you - choose just to make the error type the same
throughout the entire program. It makes things *simple*.

All that said, given that my non-variant Outcome plan has much more
compile time budget than before, I certainly can make constructors all
templated and SFINAEd in a way presented Outcome could not without
blowing out compile times. One therefore could enable implicit
construction from some arbitrary other error type if the user has
implemented implicit conversion between error types. I think that would
enable what you are proposing?

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
Gavin Lambert via Boost
2017-06-08 22:53:52 UTC
Permalink
Post by Niall Douglas via Boost
It doesn't especially help you in practice. What Outcome currently does
is return magic type sugar from TRY which is implicitly constructible by
any basic_monad instance, this works around needing to know what the
return type is for the calling function in the TRY implementation.
Fair enough. That could still work for arbitrary EI types though -- you
automatically know what the type used by the method you're calling is,
and you can assume that the caller either returns the same thing or
something compatible with it, as I said below. Otherwise it will be a
compiler error, and rightly so.
Post by Niall Douglas via Boost
Post by Gavin Lambert via Boost
Besides, if the intent is to convey the payload up the call chain, then
it can be left to the responsibility of the method itself to forward
that appropriately -- ie. if you call something that can return an
1. Deal with the error locally, not calling TRY.
2. Call TRY to pass on the error unmodified, thereby requiring that
you also return an error_code_info<EI1>.
3. Pass on the error, but returning error_code_info<EI2> where EI2 is
constructible from EI1 (forwarding the original payload plus some
additional context). This might be a special case of #1.
(By "returning" here I don't mean directly, I mean wrapped in a result<>
or outcome<>.)
Of course you could do all that.
What I'm saying is that for 95% of programmers, when facing the choice
of whether to use different E types in exchange for typing a ton load
more boilerplate all day long every day in everything they write, they
will - I guarantee you - choose just to make the error type the same
throughout the entire program. It makes things *simple*.
People don't seem to have a problem with multiple exception classes
carrying different payloads all derived from std::exception. This is an
orthogonal idea, albeit simplifying it with only a single templated
derived type. (I'm not opposed to having separate derived types either,
but I think the templated design is better since you can provide better
guarantees to the outcome implementation.)

I don't think it'd be as hard to grasp as you're making out. If we take
Filesystem as an example, it's likely that most methods would return an
error_code_info<filename_info> that contains the filename affected by
the error (which you might argue is useless to the caller since it
already knows, but again imagine this being bubbled several levels up
the call stack to code that didn't know the filename any other way).

A few might return a plain error_code because they didn't have a
filename to work on. Something like copy_file might return a type
derived from filename_info that adds a second filename. That sort of thing.

Or maybe Filesystem itself would only return plain error_codes, but then
the caller (who constructed a filename unknown to the grandparent) might
be the one that wants to include the actual filename when passing on the
error, since it knows that its caller has no other way to know it.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Bjorn Reese via Boost
2017-06-11 15:11:57 UTC
Permalink
Post by Gottlob Frege via Boost
There was talk (not sure if it was an official proposal or just a
suggestion) of something like decltype(return)
for getting the type returned from the current function.
It was part of the P0536R0 "Implicit Return Type..." proposal, but it
was rejected. The reason can be found here:


https://botondballo.wordpress.com/2017/03/27/trip-report-c-standards-meeting-in-kona-february-2017/

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Bjorn Reese via Boost
2017-06-05 14:14:26 UTC
Permalink
Post by Peter Dimov via Boost
As I see things, we have a very good std::error_code infrastructure
sitting (underused) in <system_error>, we have a good outline of how
it's used in <filesystem>, and there we have the whole API doubled as in
void api_function( ... ); // throws system_error
void api_function( ..., error_code& ec ); // doesn't throw
for example
bool exists( const std::filesystem::path& p );
bool exists( const std::filesystem::path& p, std::error_code& ec )
noexcept;
Which reminds me, does anybody know why boost::system::throws was not
used to unite the two functions as outlined in N2838 [1]? Was it the
inability to specify noexcept?

[1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2009/n2838.html

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Andrzej Krzemienski via Boost
2017-05-30 21:28:17 UTC
Permalink
Post by Peter Dimov via Boost
Post by Niall Douglas via Boost
1. I have been persuaded to use longer more appropriate naming for
result<T> and outcome<T>, so now the typedefed varieties with implicit
- static_checked_outcome<T> => static_checked_optional_outcome<T>
static_checked_result<T> => static_checked_optional_result<T>
...
Post by Niall Douglas via Boost
- runtime_checked_outcome<T> => runtime_checked_optional_outcome<T>
runtime_checked_result<T> => runtime_checked_optional_result<T>
...
Post by Niall Douglas via Boost
Nobody is proposing that end users actually type out the full name each
and every time they use them, and the documentation will make that clear.
As I already said, for me this takes away the whole point of the library,
which is to provide STANDARD result types which people use in their public
APIs.
If everyone is typedefing their own varieties, this doesn't provide a
standard infrastructure. Sure, it would be useful for people who control
all of their code, but no more than that.
There should be one and only one result<T>, and one and only one
outcome<T>, with those names, which the world should use.
I support this. I like the initial choice of this library. What percentage
does this make of people wanting one outcome and one result?
Post by Peter Dimov via Boost
Let's first see if people who favor a narrow interface are satisfied with
value_if, which for me is a very good compromise. Once you have a raw
pointer, things are as narrow as it gets.
I'll respond to this separately.

Regards,
&rzej;

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Vicente J. Botet Escriba via Boost
2017-05-30 21:48:04 UTC
Permalink
Post by Peter Dimov via Boost
Let's first see if people who favor a narrow interface are satisfied with
value_if, which for me is a very good compromise. Once you have a raw
pointer, things are as narrow as it gets.
Sorry Peter I don't receive your mails.

value_if has a wide contract. I don't have nothing agains it.
value_if It can be constructed from has_value and operator* (call it
deref if you prefer).
So the minimal interface is not value_if. When I kno wthat I have a
value in I don't want to use value_if and dereference the obtained pointer.

Vicente

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman
Andrzej Krzemienski via Boost
2017-05-30 21:38:23 UTC
Permalink
Post by Niall Douglas via Boost
Firstly thanks to everyone who has contributed no less than **618**
emails at the time of writing to the Outcome review. Despite some on
Reddit calling this review a "train wreck" and indeed some lovely things
about me personally, I have found this review deeply helpful, and I
believe so have most of its contributors in helping us all firm up our
differing opinions about what such a vocabulary type ought to be.
With regard to the previous high level summary at
http://boost.2283326.n4.nabble.com/outcome-High-level-
summary-of-review-feedback-accepted-so-far-tp4694767.html,
Thanks Niall, for the summary. But now, after these changes, the review I
am still writing is useless, as it applies to a different library. Maybe we
should schedule another review?
Post by Niall Douglas via Boost
1. I have been persuaded to use longer more appropriate naming for
result<T> and outcome<T>, so now the typedefed varieties with implicit
- static_checked_outcome<T> => static_checked_optional_outcome<T>
static_checked_result<T> => static_checked_optional_result<T>
No, no. I strongly suggest to provide only one outcome, with empty state if
needed, and both narrow and wide contracts. We have learned to live with
such types in C++, even if they are far from perfect.
Post by Niall Douglas via Boost
These have narrow observers which reinterpret_cast and are therefore
undefined behaviour if the state does not match, but use
__builtin_unreachable() to help static analysis tooling like clang-tidy
spot UB where it can be statically deduced. Conceptually, they are a
very thin convenience wrap of std::variant<...>.
- runtime_checked_outcome<T> => runtime_checked_optional_outcome<T>
runtime_checked_result<T> => runtime_checked_optional_result<T>
The runtime_checked_optional_* varieties are identical and unchanged to
the outcome<T> and result<T> in the submitted library. They have wide
observers with a formal empty state and it is never undefined behaviour
to call any observer. This allows the programmer to save typing
considerable boilerplate knowing that the runtime checks save the
programming needing to spell things out. I have not been persuaded to
change a single thing in their design yet apart from removing
unnecessary member functions, but thanks to everybody who has tried, and
if you think I am making a critical mistake, you still have a few days
left to persuade me of it!
For anyone not familiar with the discussion and feels that the names are
excessively long, be aware they are just convenience typedefs of a more
complex to configure "template<...> class outcome_impl". It is expected
that end users of the library will choose the varieties they want for
their code and typedef them into their local namespace as "result<T>" or
whatever. Nobody is proposing that end users actually type out the full
name each and every time they use them, and the documentation will make
that clear.
Regarding other changes that I have accepted since the previous high
2. Outcome's usage of git submodules is felt by some reviewers to be a
showstopper for acceptance as they could bring in non-Boost code
unexpectedly in the future. I have agreed to make a cronjob script
generated Boost.Outcome from standalone Outcome in the same fashion as
Boost.ASIO is script generated from standalone ASIO.
https://github.com/ned14/boost.outcome/issues/51
- Licensing to be BSL only
- No cmake, just bjam
- No git submodules at all
- std-flavoured, not boost-flavoured
- No code to be brought in except that written by me i.e. a hardcoded
whitelist applies.
- Contents of Boost repo to only contain material directly relevant to
Boost. Nothing else.
Thanks. This will make the acceptance easier.
Post by Niall Douglas via Boost
3. Under the assumption that error_code_extended's use of a static
global ring buffer would be controversial in this review, I hacked a
quick and dirty solution expecting to have to remove it. Now we know
that few disapprove, a more serious implementation will be needed.
https://github.com/ned14/boost.outcome/issues/52
I am not sure of that. It was my impression that many potential reviewers
were put off by documentation and legal/structural issues, and stopped with
their review at that point. Once you have move thes obstacles out of the
way, you will likely draw more reviewers, and who knows what they have to
say about the ring buffer.

Regards,
&rzej;

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Gavin Lambert via Boost
2017-05-31 00:10:50 UTC
Permalink
Post by Andrzej Krzemienski via Boost
Post by Niall Douglas via Boost
3. Under the assumption that error_code_extended's use of a static
global ring buffer would be controversial in this review, I hacked a
quick and dirty solution expecting to have to remove it. Now we know
that few disapprove, a more serious implementation will be needed.
https://github.com/ned14/boost.outcome/issues/52
I am not sure of that. It was my impression that many potential reviewers
were put off by documentation and legal/structural issues, and stopped with
their review at that point. Once you have move thes obstacles out of the
way, you will likely draw more reviewers, and who knows what they have to
say about the ring buffer.
FWIW, I don't have a problem with the ring buffer per se, but I think
it's currently too small to be effective.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Niall Douglas via Boost
2017-05-31 12:16:50 UTC
Permalink
Post by Andrzej Krzemienski via Boost
Post by Niall Douglas via Boost
With regard to the previous high level summary at
http://boost.2283326.n4.nabble.com/outcome-High-level-
summary-of-review-feedback-accepted-so-far-tp4694767.html,
Thanks Niall, for the summary. But now, after these changes, the review I
am still writing is useless, as it applies to a different library. Maybe we
should schedule another review?
As I have said in a number of posts previously, I haven't been convinced
to change much yet apart from drop some member functions for the
runtime-checked empty-capable editions which are the ones presented for
review. The behaviours of those are still being actively discussed e.g.
should exception() synthesize an exception_ptr from an errored state?

And even in the added varieties, it's not like I am writing any new
source code. The statically checked varieties just leave out the runtime
checks, otherwise they're literally identical consisting of identical
implementation. The non-empty varieties just leave out the empty state.
It's the same library as presented.
Post by Andrzej Krzemienski via Boost
Post by Niall Douglas via Boost
1. I have been persuaded to use longer more appropriate naming for
result<T> and outcome<T>, so now the typedefed varieties with implicit
- static_checked_outcome<T> => static_checked_optional_outcome<T>
static_checked_result<T> => static_checked_optional_result<T>
No, no. I strongly suggest to provide only one outcome, with empty state if
needed, and both narrow and wide contracts. We have learned to live with
such types in C++, even if they are far from perfect.
I am absolutely sold on the empty-capable vs non-empty-capable being
represented in the type system. Makes a big difference to public API
contracts.

I may choose to drop the typedef for the statically checked varieties,
but the underlying non-public template is capable of all.

I'll see what Charley's report recommends before deciding.
Post by Andrzej Krzemienski via Boost
Post by Niall Douglas via Boost
3. Under the assumption that error_code_extended's use of a static
global ring buffer would be controversial in this review, I hacked a
quick and dirty solution expecting to have to remove it. Now we know
that few disapprove, a more serious implementation will be needed.
https://github.com/ned14/boost.outcome/issues/52
I am not sure of that. It was my impression that many potential reviewers
were put off by documentation and legal/structural issues, and stopped with
their review at that point. Once you have move thes obstacles out of the
way, you will likely draw more reviewers, and who knows what they have to
say about the ring buffer.
Dunno, I got quite a few detailed questions on it, and people seemed
pleased with the design rationale once they'd thought about it.

Anyway, I intend to do something better than the current hack. Certainly
with more storage, anyway.

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
Loading...