[review] Review of Outcome (starts Fri-19-May)
Hi Everyone,
The formal review of Niall Douglas' Outcome library starts today
(Fri-19-May) and continues until Sun-28-May.
Your participation is encouraged, as the proposed library is uncoupled and
focused, and reviewers don't need to be domain experts to appreciate the
potential usefulness of the library and to propose improvements. Everyone
needs (and has suffered) error handling, and can compose an opinion on that
topic.
Outcome is a header-only C++14 library providing expressive and type-safe
ultra-lightweight error handling, suitable for low-latency code bases.
Key features:
*- Makes using std::error_code from C++11's
Outcome is a header-only C++14 library
Oops, are you sure about that? My understanding is that some of the examples and tests are C++14 but the library itself is C++11 (maybe earlier). Niall?
Outcome makes use of variable templates. There is still a code path using types instead, but it is used exclusively on MSVC and I aim to remove it as soon as Microsoft fix their compiler (I was told it'll be VS2017 Update 1). Outcome also makes heavy use of relaxed constexpr, but you're right it's macroed out for VS2015. I intend to drop support for VS2015 before Outcome enters Boost, if it is accepted, as VS2015 generates a lot of code bloat when using Outcome which VS2017 does not. The MSVC specific workarounds are also brittle and hacky, and I'd like them to go away and use ISO C++ throughout. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On Fri, May 19, 2017 at 7:13 AM, Niall Douglas via Boost
Outcome makes use of variable templates. There is still a code path using types instead, but it is used exclusively on MSVC and I aim to remove it as soon as Microsoft fix their compiler (I was told it'll be VS2017 Update 1).
So Outcome requires C++14 then? Does this mean that compilation with gcc 4.9.x is not supported?
I intend to drop support for VS2015 before Outcome enters Boost,
Although I have not yet attempted my own implementation of expected/outcome, it seems to me that an implementation using only C++11, or even only pre-C++11, is quite possible. There are still many shops that are "stuck" on particular older versions of Visual Studio. Restricting a generally useful library which parallels a standard library proposal, to only the latest version of Visual Studio sounds quite harsh. Thanks
Outcome makes use of variable templates. There is still a code path using types instead, but it is used exclusively on MSVC and I aim to remove it as soon as Microsoft fix their compiler (I was told it'll be VS2017 Update 1).
So Outcome requires C++14 then? Does this mean that compilation with gcc 4.9.x is not supported?
The minimum known working compilers are listed at https://ned14.github.io/boost.outcome/prerequisites.html. As it says, you need GCC 5.4 minimum, and in practice GCC 6 if you want to avoid ICEs, and GCC 7 if you want full functionality (assuming GCC has fixed the bugs Outcome triggers in GCC 6).
I intend to drop support for VS2015 before Outcome enters Boost,
Although I have not yet attempted my own implementation of expected/outcome, it seems to me that an implementation using only C++11, or even only pre-C++11, is quite possible. There are still many shops that are "stuck" on particular older versions of Visual Studio. Restricting a generally useful library which parallels a standard library proposal, to only the latest version of Visual Studio sounds quite harsh.
A proposed Boost library need only conform to the latest C++ standard. And as I hint at above, older compilers ICE a lot with Outcome. Use is much more stable on the most recent POSIX compilers. No point therefore in special casing Visual Studio, basically you need a compiler made last year or newer. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On Fri, May 19, 2017 at 9:00 AM, Niall Douglas via Boost
The minimum known working compilers are listed at https://ned14.github.io/boost.outcome/prerequisites.html. As it says, you need GCC 5.4 minimum, and in practice GCC 6 if you want to avoid ICEs, and GCC 7 if you want full functionality (assuming GCC has fixed the bugs Outcome triggers in GCC 6). ... A proposed Boost library need only conform to the latest C++ standard. And as I hint at above, older compilers ICE a lot with Outcome. Use is much more stable on the most recent POSIX compilers. No point therefore in special casing Visual Studio, basically you need a compiler made last year or newer.
This means that out of the box, Beast cannot use Outcome, as Beast requires only C++11. When I contemplate what an implementation of expected/outcome might look like, I do not imagine anything that requires more than C++11. Possibly even less than C++11. When I look at the implementation of Boost.Outcome it seems needlessly over-engineered. I cannot help but think that the Internal Compiler Errors which result from attempting to build with earlier versions of C++ are a consequence of this over-engineering and could have been avoided with a more straightforward implementation. While it is true the letter of the wording regarding Boost library requirements do not mandate support for earlier compilers, I believe it is in bad taste to use such wording to provide cover for unnecessarily complex code. I feel that Outcome is sufficiently narrow in scope and broad in utility that it should not be limited only to "a compiler made last year or newer."
On 5/19/17 9:18 AM, Vinnie Falco via Boost wrote:
On Fri, May 19, 2017 at 9:00 AM, Niall Douglas via Boost
While it is true the letter of the wording regarding Boost library requirements do not mandate support for earlier compilers, I believe it is in bad taste to use such wording to provide cover for unnecessarily complex code.
I don't think it's a great idea to make presumptions about motives here. Boost has traditionally been willing to accept code which conforms to only to the latest standard. This has been a very good thing. On the other hand, library authors have a natural desire to see their code widely used and have traditionally been desirous of maintaining backward compatibility when reasonable to do so. This is one reason why most widely used libraries support older compilers. For niche libraries it doesn't matter. I don't think that this is an issue. I feel that Outcome is sufficiently narrow maybe you mean wide rather than narrow
in scope and broad in utility that it should not be limited only to "a compiler made last year or newer."
Robert Ramey
This means that out of the box, Beast cannot use Outcome, as Beast requires only C++11. When I contemplate what an implementation of expected/outcome might look like, I do not imagine anything that requires more than C++11. Possibly even less than C++11.
When I look at the implementation of Boost.Outcome it seems needlessly over-engineered. I cannot help but think that the Internal Compiler Errors which result from attempting to build with earlier versions of C++ are a consequence of this over-engineering and could have been avoided with a more straightforward implementation.
You are very wrong on this. There are toy Expected implementations like the many you'll find around the web. Then there are STL quality implementations. They are *significantly* more complex because of all the niche corner cases which must be handled. For example, Expected must work correctly both in constexpr and non-constexpr, and any mix thereof. It must correctly propagate noexcept to its member functions based on the T and E it is configured with. It must inspect, with minimum compile time impact, T and E for what constructors and operators it provides, and what the noexcept and constexpr is on those, and again configure itself correctly as per the LEWG proposal. It must correctly handle throwing copy and move constructors and assignment, ensure that if T and E are trivially destructible then so is the Expected, and all whilst ensuring that nothing causes the optimiser to give up too early. It is that stuff which ICEs older compilers. In particular, MSVC likes to ICE on calculated noexcept. Older GCC likes to ICE on complex variable template expressions. Older clang also has issues with using variable template calculated values in default template parameters. The list goes on. You are probably right that a C++ 11 edition could be made. But it would likely be a completely new codebase and without the two years of maturation this code base has received. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On 5/19/17 10:00, Niall Douglas via Boost wrote:
Outcome makes use of variable templates. There is still a code path using types instead, but it is used exclusively on MSVC and I aim to remove it as soon as Microsoft fix their compiler (I was told it'll be VS2017 Update 1).
So Outcome requires C++14 then? Does this mean that compilation with gcc 4.9.x is not supported?
The minimum known working compilers are listed at https://ned14.github.io/boost.outcome/prerequisites.html. As it says, you need GCC 5.4 minimum, and in practice GCC 6 if you want to avoid ICEs, and GCC 7 if you want full functionality (assuming GCC has fixed the bugs Outcome triggers in GCC 6).
Hi Niall - This is a bit confusing. It sounds like gcc 6 or greater is needed. I can't think of a situation where ICE'ng the compiler is considered working. michael -- Michael Caisse Ciere Consulting ciere.com
The minimum known working compilers are listed at https://ned14.github.io/boost.outcome/prerequisites.html. As it says, you need GCC 5.4 minimum, and in practice GCC 6 if you want to avoid ICEs, and GCC 7 if you want full functionality (assuming GCC has fixed the bugs Outcome triggers in GCC 6).
Hi Niall - This is a bit confusing. It sounds like gcc 6 or greater is needed. I can't think of a situation where ICE'ng the compiler is considered working.
It depends on how you use Outcome in your code. The compiler versions listed at https://ned14.github.io/boost.outcome/prerequisites.html are where Outcome passes all its unit tests clean. You can definitely write a lot of code using Outcome which doesn't ICE the compiler. However I also know from AFIO v2, which makes very extensive use of Outcome, that it blows up older compilers in all sorts of fun ways. If I took the time, I could rewrite how AFIO uses Outcome to not ICE the older compilers. But given how alpha AFIO v2 is, it's just easier to bump the compiler version needed for that library to a non-broken compiler version. That's my observation. I am not sure how to exactly document this informal experience in Outcome's docs. After all, it's the compiler vendors' fault, not Outcome's. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Niall: Over here I see a "boost-lite" submodule which exports public interfaces: https://github.com/ned14/boost.outcome/tree/master/include/boost/outcome But then in this README.md https://github.com/ned14/boost-lite/blob/master/Readme.md It says "Herein is a set of tooling to enable a Boost library to swap in the C++11 standard library instead of Boost..." If Outcome requires C++14 (or maybe C++17?) what is the value of including within Outcome a library which allows you to use C++11 standard library instead of boost? In other words, Outcome could already be written against the C++ standard library facilities, since they are guaranteed to be there in all versions of C++ which Outcome supports. I also see things in boost-lite like array, thread, condition_variable, are those part of the public interfaces being proposed? Thanks
If Outcome requires C++14 (or maybe C++17?) what is the value of including within Outcome a library which allows you to use C++11 standard library instead of boost? In other words, Outcome could already be written against the C++ standard library facilities, since they are guaranteed to be there in all versions of C++ which Outcome supports.
boost-lite is no longer part of this review at all now that tribool support has been dropped, but as you ask, boost-lite requires only C++ 11 because lots of libraries beyond Outcome use it, including some C++ 11 ones.
I also see things in boost-lite like array, thread, condition_variable, are those part of the public interfaces being proposed?
Apart from boost_lite::tribool which has now been dropped due to review feedback, nothing from boost-lite appears in the public interface. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On Fri, May 19, 2017 at 12:12 PM, Niall Douglas via Boost
Apart from boost_lite::tribool which has now been dropped due to review feedback, nothing from boost-lite appears in the public interface.
Is BOOST_OUTCOME_USE_BOOST_ERROR_CODE part of the public interface? I see lines like this: https://github.com/ned14/boost.outcome/blob/master/include/boost/outcome/v1....
On 19/05/2017 20:15, Vinnie Falco via Boost wrote:
On Fri, May 19, 2017 at 12:12 PM, Niall Douglas via Boost
wrote: Apart from boost_lite::tribool which has now been dropped due to review feedback, nothing from boost-lite appears in the public interface.
Is BOOST_OUTCOME_USE_BOOST_ERROR_CODE part of the public interface? I see lines like this: https://github.com/ned14/boost.outcome/blob/master/include/boost/outcome/v1....
If reviewers make acceptance conditional on Outcome using Boost instead of the C++ 11 STL for everything, then that configuration would be activated and made permanent some how. Note it hasn't been tested in well over a year, but it used to work once and it could be resurrected without huge effort.
What does Outcome have to do with Boost.Thread? https://github.com/ned14/boost.outcome/blob/master/include/boost/outcome/v1....
It's stale. I didn't want to invest the effort of removing it until the review decides what form they want to accept Outcome in, if at all. You may have noticed the large matrix of #if statements building an ABI string, all that stuff takes lots of tedious effort to rejig. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Niall Douglas wrote:
What does Outcome have to do with Boost.Thread? https://github.com/ned14/boost.outcome/blob/master/include/boost/outcome/v1....
It's stale. I didn't want to invest the effort of removing it until the review decides what form they want to accept Outcome in, if at all.
It creates a bit of a chicken and egg issue because the reviewer's decision may well depend on the form you submit. Although you have a point in principle.
On 19 May 2017 at 22:03, Vinnie Falco via Boost
Niall: ... what is the value of including within Outcome a library which allows you to use C++11 standard library instead of boost?
C++11 should be banned (please, stop moving). degski -- "*Ihre sogenannte Religion wirkt bloß wie ein Opiat reizend, betäubend, Schmerzen aus Schwäche stillend.*" - Novalis 1798
What does Outcome have to do with Boost.Thread? https://github.com/ned14/boost.outcome/blob/master/include/boost/outcome/v1....
The formal review of Niall Douglas' Outcome library starts today (Fri-19-May) and continues until Sun-28-May.
...
Before proceeding with the review, I'd like to request a clarification here. In the event the library is accepted, what will the contents of the repository boostorg/outcome be, and what will end up in the Boost release under libs/outcome? Stated differently, is the entire ned14/boost.outcome repo being submitted as-is, or are some parts of it not part of the submission? Furthermore, are the two Git submodules, doc/html and include/boost/outcome/boost-lite part of the submission and if so, in what capacity, that is, are they expected to remain Git submodules in boostorg/outcome, or are they expected to be copied there from some fixed revision? As I already asked, will boostorg/outcome be the primary source repository for Boost.Outcome? If there is a PR submitted against boostorg/outcome, what steps will need to be performed in order for that PR to become integrated into the Outcome code base?
In the event the library is accepted, what will the contents of the repository boostorg/outcome be, and what will end up in the Boost release under libs/outcome?
That depends on what conditions the review manager places on acceptance, if acceptance is recommended.
Stated differently, is the entire ned14/boost.outcome repo being submitted as-is, or are some parts of it not part of the submission?
I am submitting it as-is in its present form. Reviewers may wish to impose conditions regarding its boostorg form on acceptance.
Furthermore, are the two Git submodules, doc/html and include/boost/outcome/boost-lite part of the submission and if so, in what capacity, that is, are they expected to remain Git submodules in boostorg/outcome, or are they expected to be copied there from some fixed revision?
The doc/html subrepo is simply the gh-pages branch used to deliver Github Pages for the docs. Almost every recent Boost library submitted does the same. If accepted, I'd do exactly whatever Boost.Hana did to get its doxygen docs into boost.org. As I've mentioned before, during build we assemble a single file include of all the dependencies, including those parts of boost-lite needed. There is therefore no strict need to ship boost-lite in any boostorg repo, which is good as I believe boostorg currently doesn't support project level subrepos. But if that weren't acceptable, we could figure something out. To be honest, I don't think it hugely important how we get Outcome into boostorg, if accepted we'll figure out some solution which the build, test and release managers are happy with and go with that. Personally speaking, and reviewers can disagree if they want, what matters is does the submitted library work well with Boost? I believe the answer is that no collision, conflict nor bad interaction can happen between any Boost code and any Outcome code. It therefore works well with Boost. Again, as I mentioned before, there is precedent here. When Hana was submitted it didn't even use the STL let alone Boost. It also used cmake and had no Boost.Build. Outcome follows exactly the path set by Hana.
As I already asked, will boostorg/outcome be the primary source repository for Boost.Outcome? If there is a PR submitted against boostorg/outcome, what steps will need to be performed in order for that PR to become integrated into the Outcome code base?
I'd prefer not a script generated boostorg/outcome repo, it makes maintenance harder. But if reviews impose conditions which require a script generated repo as they did back in the day with ASIO, then I'll make it work. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Niall Douglas wrote:
In the event the library is accepted, what will the contents of the repository boostorg/outcome be, and what will end up in the Boost release under libs/outcome?
That depends on what conditions the review manager places on acceptance, if acceptance is recommended.
Yes of course. My question was what you intend to happen.
As I've mentioned before, during build we assemble a single file include of all the dependencies, including those parts of boost-lite needed. There is therefore no strict need to ship boost-lite in any boostorg repo, which is good as I believe boostorg currently doesn't support project level subrepos.
This would only work if I use boost/outcome/outcome_v1.0.hpp, right? The headers in v1.0/ do not compile without boost-lite.
To be honest, I don't think it hugely important how we get Outcome into boostorg,
An interesting position. I, however, in order to decide between Yes and No, need to know what, precisely, will happen on Yes. Of course I could say Yes if you do such and so, otherwise No, but it's good to know how you see things and what's your intent.
As I already asked, will boostorg/outcome be the primary source repository for Boost.Outcome? If there is a PR submitted against boostorg/outcome, what steps will need to be performed in order for that PR to become integrated into the Outcome code base?
I'd prefer not a script generated boostorg/outcome repo, it makes maintenance harder.
I'm not sure I understand. If you prefer not a script-generated boostorg/outcome repo, what do you prefer?
To be honest, I don't think it hugely important how we get Outcome into boostorg,
An interesting position. I, however, in order to decide between Yes and No, need to know what, precisely, will happen on Yes.
Of course I could say Yes if you do such and so, otherwise No, but it's good to know how you see things and what's your intent.
My opinion there is that the documentation, code, design and implementation are what is being reviewed. Build stuff and test stuff and maintenance stuff are of course important, but that's *process* stuff, it's secondary in importance relatively speaking. I mean, basically build, test and maintenance stuff is on me to get right, else I will spend enormous amount of my very limited free time on it. And I am highly unlikely to choose that situation. It is possible that the review places such onerous conditions on admission that I cannot see a path to take this codebase into Boost without completely rewriting it from scratch. I hope not, that would be a big ask.
As I already asked, will boostorg/outcome be the primary source > repository for Boost.Outcome? If there is a PR submitted against > boostorg/outcome, what steps will need to be performed in order for that > PR to become integrated into the Outcome code base?
I'd prefer not a script generated boostorg/outcome repo, it makes maintenance harder.
I'm not sure I understand. If you prefer not a script-generated boostorg/outcome repo, what do you prefer?
There's script generated within my build system. That's okay, it's already doing that, indeed that's going to get much worse with issue #25. Then there's script generated whereby a new git repo is one-way synthesised by cron script from another git repo (like ASIO). I'd prefer not to have to do this. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Niall Douglas wrote:
I'm not sure I understand. If you prefer not a script-generated boostorg/outcome repo, what do you prefer?
There's script generated within my build system. That's okay, it's already doing that, indeed that's going to get much worse with issue #25.
Then there's script generated whereby a new git repo is one-way synthesised by cron script from another git repo (like ASIO). I'd prefer not to have to do this.
I still don't understand. Let me describe what I would like to see, and you'll tell me if you see things the same way. What I'd prefer to see is, in boostorg/outcome:include/boost/outcome, in addition to the outcome_v1.0.hpp header as currently generated, an outcome.hpp file that includes the headers in v1.0, as it currently does when BOOST_OUTCOME_DISABLE_PREPROCESSED_INTERFACE_FILE is defined. I also would like this to work without boost-lite, pcpp, gsl-lite being present at all in the boostorg/outcome repo (or on the user's system). Furthermore, when someone issues a pull request against boostorg/outcome:include/boost/outcome/v1.0/something.hpp, I'd like to see this pull request kicking off a travis build as we're now used to, with this travis build actually testing Outcome with the pull request applied. Does this make sense?
I still don't understand. Let me describe what I would like to see, and you'll tell me if you see things the same way.
What I'd prefer to see is, in boostorg/outcome:include/boost/outcome, in addition to the outcome_v1.0.hpp header as currently generated, an outcome.hpp file that includes the headers in v1.0, as it currently does when BOOST_OUTCOME_DISABLE_PREPROCESSED_INTERFACE_FILE is defined. I also would like this to work without boost-lite, pcpp, gsl-lite being present at all in the boostorg/outcome repo (or on the user's system).
Furthermore, when someone issues a pull request against boostorg/outcome:include/boost/outcome/v1.0/something.hpp, I'd like to see this pull request kicking off a travis build as we're now used to, with this travis build actually testing Outcome with the pull request applied.
Does this make sense?
I appreciate your wishes in the above. I should point out that Outcome already is tested per commit by Travis and Appveyor, and it keeps a history at CDash: http://my.cdash.org/index.php?project=Boost.Outcome But it is very hard for me to be more concrete at the present time. Most of the dependency of Outcome on boost-lite is soft, e.g. BOOSTLITE_CONSTEXPR <=> BOOST_CXX14_CONSTEXPR, Boost-lite lightweight unit test <=> Boost.Test and so on. But some, especially error_code_extended's use of boost-lite's ringbuffer_log is hard. If reviewers want what I've done to achieve error_code_extended's static storage removed entirely, that would remove much of the hard dependency on boost-lite and a macro layer could rebind the boost-lite macros etc to Boost ones. In this situation, boost-lite goes away in the boostorg repo, otherwise the repos look identical. If on the other hand people like the static storage, one would really rather keep boost-lite in the Outcome repo because bug fixes to there fix lots of other libraries too, and ringbuffer_log is very heavily used by all my libraries. I being the person with the maintenance load here will do whatever I think makes my life easiest. How the library is internally organised I don't think a concern for the end user unless it negatively affects them. Similarly, if people like the cmake build with all the fancy stuff like automatic C++ Modules, you need boost-lite in there for that. If Boost.Build is felt sufficient, then you don't need it. Again, I'd emphasise that boost-lite has no interaction with Boost or any other code. It is a 100% ideal C++ 14 citizen, right down to ABI permuting to ensure mixed versions do not collide. Nobody will ever need to care it's there under the bonnet as an internal helper library. And finally, I do intend to keep Outcome working completely independent of Boost. The vast majority of my user base explicitly do not want a Boost dependency. You'll have noticed the same with Hana. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Niall Douglas wrote:
And finally, I do intend to keep Outcome working completely independent of Boost. The vast majority of my user base explicitly do not want a Boost dependency.
The vast majority of your user base may change if your library is accepted.
It almost certainly will.
Not wanting to depend on Boost is understandable. Our common infrastructure
however is not just a whim of ours, it's a service to Boost users. Going
through BOOST_ASSERT and BOOST_THROW_EXCEPTION instead of rolling your own
is more convenient for people who have already configured BOOST_ASSERT and
BOOST_THROW_EXCEPTION to do what they want them to do and would rather not
repeat that exercise for every new library, however wonderful.
Similarly, once we get the kinks out of Boost.Stacktrace, it might be better
to use that instead of #including
On Sat, May 20, 2017 at 3:47 PM, Peter Dimov via Boost < boost@lists.boost.org> wrote:
Niall Douglas wrote:
And finally, I do intend to keep Outcome working completely independent of
Boost. The vast majority of my user base explicitly do not want a Boost dependency.
The vast majority of your user base may change if your library is accepted. It almost certainly will.
Not wanting to depend on Boost is understandable. Our common infrastructure however is not just a whim of ours, it's a service to Boost users. Going through BOOST_ASSERT and BOOST_THROW_EXCEPTION instead of rolling your own is more convenient for people who have already configured BOOST_ASSERT and BOOST_THROW_EXCEPTION to do what they want them to do and would rather not repeat that exercise for every new library, however wonderful.
It is possible to make such use of Boost macros the default while allowing people who don't want to depend on other Boost libraries to opt-out.
And finally, I do intend to keep Outcome working completely independent of Boost. The vast majority of my user base explicitly do not want a Boost dependency.
The vast majority of your user base may change if your library is accepted. It almost certainly will.
I would still expect a vast majority to not want a Boost dependency. People want C++ 14 libraries to use the C++ 14 STL, not Boost, though *optional* non-default usage of Boost stuff appears to be popular.
Not wanting to depend on Boost is understandable. Our common infrastructure however is not just a whim of ours, it's a service to Boost users. Going through BOOST_ASSERT and BOOST_THROW_EXCEPTION instead of rolling your own is more convenient for people who have already configured BOOST_ASSERT and BOOST_THROW_EXCEPTION to do what they want them to do and would rather not repeat that exercise for every new library, however wonderful.
Outcome only ever throws an exception via a macro. It can be customised. If reviewers wish it, we can have the default use BOOST_THROW_EXCEPTION.
Similarly, once we get the kinks out of Boost.Stacktrace, it might be better to use that instead of #including
.
Including Outcome does not include
All that's not strictly required for acceptance, in principle; neither is using Boost.Config instead of rolling your own. And yet.
Boost-lite macros can be swapped for Boost.Config ones if reviewers wish it. I should mention that boost-lite derives its macros from the C++ 17 feature macros, which all recent editions of major compilers support even in C++ 14 mode. In other words, it is not compiler version based, except on old compilers, but it does rely on compiler vendors telling the truth (they generally do).
Regarding the ring buffer, I agree that it adds value. But it also adds complexity. There's an argument to be made for result/outcome classes that simply store std::error_code without all the bells and whistles, and provide an optional hook to which the user can attach the ring buffer if needed.
The tutorial covers the need in real world code to attach some sort of payload to error codes. Outcome provides error_code_extended with a reasonable set of the most common likely payloads. If reviewers would prefer a vanilla error_code to be the default, that's very easy to fix. I would add that if you do not use any of the payload, error_code_extended is identical to an error_code, no overhead.
Finally, you don't even need to macro-bind to std::error_code. Inspired by Outcome, I wrote a pull request for Boost.System which makes the Boost error_category, error_code and error_condition convertible to the standard ones, so you can just store std::error_code and be done with it.
I'll leave it to reviewers to decide on whether defaulting to the C++ 14 STL std::error_code or to boost::error_code is the most appropriate. It's very easy to change. And as I've mentioned before, if reviewers really want a completely boostified edition of Outcome, that's no problem, I can make that work. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Niall Douglas wrote:
The vast majority of your user base may change if your library is accepted. It almost certainly will.
I would still expect a vast majority to not want a Boost dependency.
The vast majority get their Boost libraries through some form of a Boost release. For them, the phrase "a Boost dependency" makes no sense at all. You could legitimately talk about depending (or not depending, which is preferable) on a specific Boost module, but not depending on Boost in general has no meaning when you're part of Boost in general.
People want C++ 14 libraries to use the C++ 14 STL, not Boost,
Not a problem at all here, but not what we're talking about. You've eliminated a dependency on Boost, but substituted a dependency on something called Boost Lite. For people who acquire your library as part of Boost, this is not an asset, it's a liability. (For those who don't, it's vice versa.)
Including Outcome does not include
, except on winclang
Yes, I happened to have my toolset set to Clang.
I should mention that boost-lite derives its macros from the C++ 17 feature macros, which all recent editions of major compilers support even in C++ 14 mode.
Does MSVC support SD-6 macros? I thought that it didn't.
The tutorial covers the need in real world code to attach some sort of payload to error codes. Outcome provides error_code_extended with a reasonable set of the most common likely payloads.
Yes, the need to attach more information to error codes is genuine. For exceptions Boost.Exception lets us do that non-intrusively, but for error codes we do not have a similar framework. Speaking of which, can you please give me a few examples of what goes into the two codes (code1 and code2)?
I would add that if you do not use any of the payload, error_code_extended is identical to an error_code, no overhead.
Apart from the additional size_t. But it occurs to me that you can hide that on x64 if (a) you make it 32 bit - should be enough for a ring buffer - and (b) you do not derive from std::error_code but instead store the members directly in the correct order. int32_t cookie_; int value_; std::error_category const * cat_;
I'll leave it to reviewers to decide on whether defaulting to the C++ 14 STL std::error_code or to boost::error_code is the most appropriate.
I'm not sure you understand me here... I'm saying that there's no need to default to boost::error_code or even keep the stl11:: way of choosing between the two.
On 21/05/2017 14:39, Peter Dimov via Boost wrote:
Niall Douglas wrote:
The vast majority of your user base may change if your library is > accepted. It almost certainly will.
I would still expect a vast majority to not want a Boost dependency.
The vast majority get their Boost libraries through some form of a Boost release.
The vast majority do something like: apt-get install libboost-filesystem-dev So they ask for the library they need, and get lots of other stuff too (filesystem is actually pretty good, only four or five dependent boost libraries, others basically drag in libboost-all-dev)
For them, the phrase "a Boost dependency" makes no sense at all. You could legitimately talk about depending (or not depending, which is preferable) on a specific Boost module, but not depending on Boost in general has no meaning when you're part of Boost in general.
All the evidence to date suggests that people far prefer Boost libraries without the monolithic Boost dependency, and especially without a Boost.Build dependency. Standalone ASIO is much more popular than Boost.ASIO Standalone Hana I am told is much more popular than Boost.Hana, but maybe Louis will confirm or deny. I expect the same to be the case with standalone Outcome. An overwhelming majority of developers outside boost-dev want Boost quality libraries without the Boost dependency. This topic comes up **very** frequently on Reddit and Stackoverflow. People have tried to explain this here on boost-dev, but it has not been well received. I don't want to rehash those arguments during this review. I've said all I intend to on that topic. I will reiterate that if reviewers want a boost-flavoured Outcome which doesn't default to the C++ 14 STL, that's no problem to achieve.
People want C++ 14 libraries to use the C++ 14 STL, not Boost,
Not a problem at all here, but not what we're talking about. You've eliminated a dependency on Boost, but substituted a dependency on something called Boost Lite. For people who acquire your library as part of Boost, this is not an asset, it's a liability. (For those who don't, it's vice versa.)
It is not a dependency if it comes bundled internally. Which it does. No external downloads nor installs needed unless you are using the git repo (and even then, fetchRecurseSubmodules is set to true, so a simple git submodule update should work if your git is new enough).
I should mention that boost-lite derives its macros from the C++ 17 feature macros, which all recent editions of major compilers support even in C++ 14 mode.
Does MSVC support SD-6 macros? I thought that it didn't.
I believe they appeared in an update of VS2015, and are present in VS2017. __has_include also appeared and mostly works, though it is buggy.
The tutorial covers the need in real world code to attach some sort of payload to error codes. Outcome provides error_code_extended with a reasonable set of the most common likely payloads.
Yes, the need to attach more information to error codes is genuine. For exceptions Boost.Exception lets us do that non-intrusively, but for error codes we do not have a similar framework.
Boost.Exception allocates memory to do that. Allocation of memory is not possible anywhere in Outcome, it ruins the point of using it for low latency C++ which is a big user base.
Speaking of which, can you please give me a few examples of what goes into the two codes (code1 and code2)?
Totally up to the end user. You could store a void * across both 32 bit codes. You could store a thread id in one and the bottom 32 bits of this in the other. Lots of options. I chose 64 bits as it's the minimum which is versatile. In my own code, I define a local make_errored_result<>() function which wraps the Outcome one, but adds in custom code1 and code2 where appropriate.
I would add that if you do not use any of the payload, error_code_extended is identical to an error_code, no overhead.
Apart from the additional size_t.
I benchmarked the effects of that additional 8 bytes. No statistically measurable difference.
But it occurs to me that you can hide that on x64 if (a) you make it 32 bit - should be enough for a ring buffer - and (b) you do not derive from std::error_code but instead store the members directly in the correct order.
int32_t cookie_; int value_; std::error_category const * cat_;
The ability to implicit type slice an error_code_extended to an error_code may well be felt to be an issue by reviewers. If explicit conversion were felt desirable despite creating boilerplate, then definitely yes you'd optimise storage. But as I mentioned, I found no statistical difference in performance with the additional 8 bytes. After all, one is not creating large arrays of error_code_extended.
I'll leave it to reviewers to decide on whether defaulting to the C++ 14 STL std::error_code or to boost::error_code is the most appropriate.
I'm not sure you understand me here... I'm saying that there's no need to default to boost::error_code or even keep the stl11:: way of choosing between the two.
Retaining standalone usability of Outcome is a high priority for me. A lot of folk from SG14 are interested in using Outcome, and I intend to submit Outcome into SG14's collection of low latency suitable libraries. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Niall Douglas wrote:
The vast majority do something like:
apt-get install libboost-filesystem-dev
... which typically installs all Boost headers. I don't think that maintainers split those, it's a lot of work and Boost releases do not contain the information that's necessary to do it properly.
Speaking of which, can you please give me a few examples of what goes into the two codes (code1 and code2)?
Totally up to the end user.
I know that it's totally up to the end user. I'm asking for a few real-world examples of use. What do you use them for, what do your users use them for?
I'll leave it to reviewers to decide on whether defaulting to the C++ 14 STL std::error_code or to boost::error_code is the most appropriate.
I'm not sure you understand me here... I'm saying that there's no need to default to boost::error_code or even keep the stl11:: way of choosing between the two.
Retaining standalone usability of Outcome is a high priority for me. A lot of folk from SG14 are interested in using Outcome, and I intend to submit Outcome into SG14's collection of low latency suitable libraries.
I'm now sure that you don't understand, because your answer makes no sense. I'm telling you TO NOT USE BOOST::ERROR_CODE, and you tell me that you'd rather retain standalone usability. Hello?
Totally up to the end user.
I know that it's totally up to the end user. I'm asking for a few real-world examples of use. What do you use them for, what do your users use them for?
Oh, ok.
Here's an example from
https://github.com/ned14/boost.kerneltest/blob/5bf4b78569366198af745ec6b7319...:
// If this is empty, workspaces are identical
resultstl1z::filesystem::path workspaces_not_identical =
compare_directories
I'll leave it to reviewers to decide on whether defaulting to the C++ >> 14 STL std::error_code or to boost::error_code is the most appropriate.
I'm not sure you understand me here... I'm saying that there's no need > to default to boost::error_code or even keep the stl11:: way of choosing > between the two.
Retaining standalone usability of Outcome is a high priority for me. A lot of folk from SG14 are interested in using Outcome, and I intend to submit Outcome into SG14's collection of low latency suitable libraries.
I'm now sure that you don't understand, because your answer makes no sense. I'm telling you TO NOT USE BOOST::ERROR_CODE, and you tell me that you'd rather retain standalone usability. Hello?
Unless I have misunderstood your patch, you bring in error_condition
comparisons from
Niall Douglas wrote: ...
If there was an error, we return an error code of kerneltest_errc::filesystem_comparison_internal_failure with a payload of the string message from the source error and its original error code in the first 32 bit code1 integer. That is used later on to print the exact error code returned by the system as we know it will always be system_category.
If the directory structures differed, we return an error code of kerneltest_errc::filesystem_comparison_failed with a payload of the string of the path which did not match.
Is this sufficient, or would you prefer a different example?
Thanks, that's very helpful. The use case here is similar to std::throw_with_nested, you have an error_code that you are overriding with another. You could extend your ring buffer facility to support this natively. For that, you need to store in the buffer entry the error code (a duplicate of error_code_extended's members) _and_ the unique_id of the "parent" error_info_extended. This would allow you to later retrieve the whole chain of ring buffer entries (subject to availability.) In your example, you'd do testret = error_code_extended( kerneltest_errc::filesystem_comparison_internal_failure, workspaces_not_identical.error() ); Since workspaces_not_identical.error() is already in the ring buffer, you only need a 'pointer' to it in testret. There's also no need to pass its message() in testret's message, you could use that for other purposes.
Is this sufficient, or would you prefer a different example?
Thanks, that's very helpful.
The use case here is similar to std::throw_with_nested, you have an error_code that you are overriding with another. You could extend your ring buffer facility to support this natively. For that, you need to store in the buffer entry the error code (a duplicate of error_code_extended's members) _and_ the unique_id of the "parent" error_info_extended. This would allow you to later retrieve the whole chain of ring buffer entries (subject to availability.) In your example, you'd do
testret = error_code_extended( kerneltest_errc::filesystem_comparison_internal_failure, workspaces_not_identical.error() );
Since workspaces_not_identical.error() is already in the ring buffer, you only need a 'pointer' to it in testret. There's also no need to pass its message() in testret's message, you could use that for other purposes.
Actually it's not in the ring buffer, it was an error code raised by the Filesystem TS implementation. No extended info because it had nothing to do with Outcome. There is no way to chain extended info's because the ring buffer keeps the extended information only. If you wanted to implement a chain, as the Outcome FAQ mentions you can convert your error_code_extended into an exception object and chain them via the STL's nested exception framework. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Niall Douglas wrote:
There is no way to chain extended info's because the ring buffer keeps the extended information only.
Well that's what I said, you would be able to chain them if you add the non-extended info to the ring buffer entry. That's 16 bytes. Or 20, if you use size_t for the parent unique_id.
If you wanted to implement a chain, ...
You wanted to, in the example. Not me. :-)
Niall Douglas wrote:
Unless I have misunderstood your patch, you bring in error_condition comparisons from
allowing one to compare error codes to error conditions across both Boost and STL error categories. This is fine, and indeed valuable. But it isn't relevant to Outcome particularly because we cannot say what the end user is doing with their error_code, and certainly not if whatever they are doing has any relation to error_condition.
What the patch does is make boost::system::error_code and boost::system::error_condition convertible to std::error_code and std::error_condition. The idea is that a function that returns std::error_code can use a function that returns boost::system::error_code in its implementation and can then return it directly. In cases where the original boost::error_code can be compared to boost::errc::foo, the converted std::error_code can be compared to the equivalent std::errc::foo. In addition, if the original boost::error_code can be compared to a custom boost::error_condition, the converted one can be compared to the converted std::error_condition. Long story short, the relevance to Outcome is that you can always store std::error_code in result/outcome, even if the user passes you a boost::error_code (because it will convert). There is no need to choose at compile time which of the two to store, because you can take both. That is, the function result<T> make_error_result( std::error_code const & ec ); will now be able to take Boost error codes as well.
Long story short, the relevance to Outcome is that you can always store std::error_code in result/outcome, even if the user passes you a boost::error_code (because it will convert). There is no need to choose at compile time which of the two to store, because you can take both. That is, the function
result<T> make_error_result( std::error_code const & ec );
will now be able to take Boost error codes as well.
Ah now I get it. Your patch adding that support is a great bit of work then. Thank you. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On Sun, May 21, 2017 at 7:35 AM, Niall Douglas via Boost < boost@lists.boost.org> wrote:
Standalone ASIO is much more popular than Boost.ASIO
What do you mean by that? You can't possibly mean that standalone Boost.ASIO is more distributed by Boost -- Boost releases are everywhere!
Yes, the need to attach more information to error codes is genuine. For exceptions Boost.Exception lets us do that non-intrusively, but for error codes we do not have a similar framework.
Boost.Exception allocates memory to do that. Allocation of memory is not possible anywhere in Outcome, it ruins the point of using it for low latency C++ which is a big user base.
Control may have exited low latency contexts. High level contexts may have information relevant to an error detected and reported by low level code. There is no harm in providing the _ability_ to carry it. Secondly, the problem of memory allocation can be dealt with using alocators and shared_ptr. Generally, if a low level context has data that might be needed to handle the error, there should be a mechanism to report it, doubly so because it is tricky. By not dealing with this problem you're pushing that responsibility to the users who may need it. Retaining standalone usability of Outcome is a high priority for me. A
lot of folk from SG14 are interested in using Outcome, and I intend to submit Outcome into SG14's collection of low latency suitable libraries.
Could it be that making it part of Boost is low priority then? Or at least lower? :) The way I see it, if you want to be part of Boost, you should be a "good" member of Boost, and this does mean to *by default* include config, assert with BOOST_ASSERT, throw using BOOST_THROW_EXCEPTION, and a few other things. You can then support others users, for example let them #define OUTCOME_STANDALONE and you #ifdef out all Boost dependencies.
The way I see it, if you want to be part of Boost, you should be a "good" member of Boost, and this does mean to *by default* include config, assert with BOOST_ASSERT, throw using BOOST_THROW_EXCEPTION, and a few other things. You can then support others users, for example let them #define OUTCOME_STANDALONE and you #ifdef out all Boost dependencies.
Outcome is a good member of Boost. It ticks every box on the list of requirements at http://www.boost.org/development/requirements.html except for Boost.Build support. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On Sun, May 21, 2017 at 2:35 PM, Niall Douglas via Boost
Outcome...ticks every box...at http://www.boost.org/development/requirements.html
Including "Aim first for clarity" ?
On Sun, May 21, 2017 at 2:35 PM, Niall Douglas via Boost < boost@lists.boost.org> wrote:
The way I see it, if you want to be part of Boost, you should be a "good" member of Boost, and this does mean to *by default* include config, assert with BOOST_ASSERT, throw using BOOST_THROW_EXCEPTION, and a few other things. You can then support others users, for example let them #define OUTCOME_STANDALONE and you #ifdef out all Boost dependencies.
Outcome is a good member of Boost. It ticks every box on the list of requirements at http://www.boost.org/development/requirements.html except for Boost.Build support.
I did not mean to suggest that this is a requirement, I said "should" not "must". For example, if you don't use BOOST_THROW_EXCEPTION to throw, users won't have the file, line and function name of the throw when calling current_exception_diagnostic_information; and if you don't at least use boost::throw_exception, you create problems for users who use Boost under BOOST_NO_EXCEPTIONS, which seems would include users of Outcome.
Standalone ASIO is much more popular than Boost.ASIO
What do you base this statement on?
Back when I did the research for my C++ Now talk on the future of Boost, I did a search of open source projects #including "asio/asio" as against "boost/asio". The "asio/asio" ones outnumbered "boost/asio" by approx 50% if I remember rightly. I am subscribed to asio questions on stackoverflow, and there is a noticeable pattern that people with beginner level questions use boost.asio, whereas the tricky questions they are using standalone asio. That suggests the serious users of ASIO use the standalone edition. Finally, a quick google search of "boost.asio vs standalone asio" reveals most of the top results recommend using standalone asio if you have C++ 11 or better. The most common reason given is "because it is truly header only and it doesn't drag in Boost". Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Hello, One of the questions that is inevitably asked when a proposal to switch to using something like outcome is that of exceptions. Why outcome over exceptions etc which you briefly touch on in your ACUU talk. With that in mind, when attempting to sell this library having the motivating example on the boost.outcome docs using exceptions really makes starting the discussion harder. I understand one can never escape exceptions, but it shouldn't be used as the first example a new user sees. I would like to see this example updated to show how to handle errors with out the need for exception handling around accessing *value*. Personally, I see something similar to https://ned14.github.io/boost.outcome/md_doc_md_03-tutorial_b.html#expected_... as a much better example to help sell outcome and therefore better suited as the motivating example for the front page of the docs. Jared. -- View this message in context: http://boost.2283326.n4.nabble.com/review-Review-of-Outcome-starts-Fri-19-Ma... Sent from the Boost - Dev mailing list archive at Nabble.com.
2017-05-23 1:56 GMT+02:00 Jared Wyles via Boost
Hello,
One of the questions that is inevitably asked when a proposal to switch to using something like outcome is that of exceptions. Why outcome over exceptions etc which you briefly touch on in your ACUU talk. With that in mind, when attempting to sell this library having the motivating example on the boost.outcome docs using exceptions really makes starting the discussion harder. I understand one can never escape exceptions, but it shouldn't be used as the first example a new user sees. I would like to see this example updated to show how to handle errors with out the need for exception handling around accessing *value*. Personally, I see something similar to https://ned14.github.io/boost.outcome/md_doc_md_03-tutorial_ b.html#expected_payload as a much better example to help sell outcome and therefore better suited as the motivating example for the front page of the docs.
This means, the initial example should use `boost::outcome::result` (instead of `boost::outcome::outcome`). And then function that inspects the result could read: ``` void test() { if (auto r = fun) use_int_value(r.value()); else inspect_error_code(r.error()); } ``` Regards, &rzej;
2017-05-23 8:50 GMT+02:00 Andrzej Krzemienski
2017-05-23 1:56 GMT+02:00 Jared Wyles via Boost
: Hello,
One of the questions that is inevitably asked when a proposal to switch to using something like outcome is that of exceptions. Why outcome over exceptions etc which you briefly touch on in your ACUU talk. With that in mind, when attempting to sell this library having the motivating example on the boost.outcome docs using exceptions really makes starting the discussion harder. I understand one can never escape exceptions, but it shouldn't be used as the first example a new user sees. I would like to see this example updated to show how to handle errors with out the need for exception handling around accessing *value*. Personally, I see something similar to https://ned14.github.io/boost.outcome/md_doc_md_03-tutorial_ b.html#expected_payload as a much better example to help sell outcome and therefore better suited as the motivating example for the front page of the docs.
This means, the initial example should use `boost::outcome::result` (instead of `boost::outcome::outcome`). And then function that inspects the result could read:
``` void test() { if (auto r = fun) use_int_value(r.value()); else inspect_error_code(r.error()); } ```
The initial example could look like this: https://github.com/akrzemi1/__sandbox__/blob/master/outcome_intro.md Regards, &rzej;
Le 23/05/2017 à 12:35, Andrzej Krzemienski via Boost a écrit :
2017-05-23 8:50 GMT+02:00 Andrzej Krzemienski
: 2017-05-23 1:56 GMT+02:00 Jared Wyles via Boost
: Hello,
One of the questions that is inevitably asked when a proposal to switch to using something like outcome is that of exceptions. Why outcome over exceptions etc which you briefly touch on in your ACUU talk. With that in mind, when attempting to sell this library having the motivating example on the boost.outcome docs using exceptions really makes starting the discussion harder. I understand one can never escape exceptions, but it shouldn't be used as the first example a new user sees. I would like to see this example updated to show how to handle errors with out the need for exception handling around accessing *value*. Personally, I see something similar to https://ned14.github.io/boost.outcome/md_doc_md_03-tutorial_ b.html#expected_payload as a much better example to help sell outcome and therefore better suited as the motivating example for the front page of the docs.
This means, the initial example should use `boost::outcome::result` (instead of `boost::outcome::outcome`). And then function that inspects the result could read:
``` void test() { if (auto r = fun) use_int_value(r.value()); else inspect_error_code(r.error()); } ```
The initial example could look like this: https://github.com/akrzemi1/__sandbox__/blob/master/outcome_intro.md
THis is a better example of what Outcome can do, yes. Would the following works as well? I have changed just outcome by expected. What if fun returns an expection ptr? auto my_fun()noexcept -> outcome::expected<int> { std::error_code ec; int i =Library1::fun(ec); if (ec) return outcome::make_errored_outcome(ec);// error code returned inside outcome BOOST_OUTCOME_TRY(rslt2,Library2::fun());// if fun() succeeds, rslt2 of type int contains result // otherwise returns the result of fun() up BOOST_OUTCOME_TRY(rslt3,Library3::fun());// Similarly, either rslt3 is an int with successful result, // or my_fun() returns what Library3::fun() has returned return i + rslt3;// return a result with a value }; Best, Vicente
This means, the initial example should use `boost::outcome::result` (instead of `boost::outcome::outcome`). And then function that inspects the result could read:
``` void test() { if (auto r = fun) use_int_value(r.value()); else inspect_error_code(r.error()); } ```
The initial example could look like this: https://github.com/akrzemi1/__sandbox__/blob/master/outcome_intro.md
THis is a better example of what Outcome can do, yes.
You both would prefer that the landing page motivating example not show how outcomes can also transport caught C++ exceptions through non-C++ exception code? Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
2017-05-24 17:00 GMT+02:00 Niall Douglas via Boost
This means, the initial example should use `boost::outcome::result` (instead of `boost::outcome::outcome`). And then function that inspects the result could read:
``` void test() { if (auto r = fun) use_int_value(r.value()); else inspect_error_code(r.error()); } ```
The initial example could look like this: https://github.com/akrzemi1/__sandbox__/blob/master/outcome_intro.md
THis is a better example of what Outcome can do, yes.
You both would prefer that the landing page motivating example not show how outcomes can also transport caught C++ exceptions through non-C++ exception code?
Actually, what I am suggesting goes beyond the initial example. My suggestion is driven by the advice form Robert Ramey: that the potential users are impatient: you have like 1 minute to attract their attention, and like 5-10 minutes to go through the features without loosing their interest. So, I would do something like we have in the documentation for Boost.Optional. The initial page is very short. Short description and a really small example. In fact too small to sell the power of this library: http://www.boost.org/doc/libs/1_64_0/libs/optional/doc/html/index.html And then in a number of subsequent pages, we describe feature by feature with short examples: http://www.boost.org/doc/libs/1_64_0/libs/optional/doc/html/boost_optional/q... http://www.boost.org/doc/libs/1_64_0/libs/optional/doc/html/boost_optional/q... http://www.boost.org/doc/libs/1_64_0/libs/optional/doc/html/boost_optional/q... http://www.boost.org/doc/libs/1_64_0/libs/optional/doc/html/boost_optional/q... http://www.boost.org/doc/libs/1_64_0/libs/optional/doc/html/boost_optional/q... I would do the same for Outcome: In the landing page just give this example: https://github.com/akrzemi1/__sandbox__/blob/master/outcome_intro2.md It only introduces `outcome::expected` (lot of users will not have heard of `std::expected`) and the TRY operation to attract attention of more informed users. Then in the second page I would provide a number of short capters each with one annotated example, each introducing one feature: - one for using payload wit error_code - one for all TRY operations - one for interaction with exception_ptr - one for catching exceptions into `outcome` - one for different ways of accessing the state of `outcome` If you like this idea, I would volunteer to write up the initial pages. Regards, &rzej;
- one for using payload wit error_code - one for all TRY operations - one for interaction with exception_ptr - one for catching exceptions into `outcome` - one for different ways of accessing the state of `outcome`
If you like this idea, I would volunteer to write up the initial pages.
A little too ASIO-y for me personally. But more than one person has asked for it, so I accept. I've created an issue for you at https://github.com/ned14/boost.outcome/issues/41. Don't do any work until after the review ends though, I am getting the feeling big naming changes are coming. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
One of the questions that is inevitably asked when a proposal to switch to using something like outcome is that of exceptions. Why outcome over exceptions etc which you briefly touch on in your ACUU talk. With that in mind, when attempting to sell this library having the motivating example on the boost.outcome docs using exceptions really makes starting the discussion harder. I understand one can never escape exceptions, but it shouldn't be used as the first example a new user sees. I would like to see this example updated to show how to handle errors with out the need for exception handling around accessing *value*. Personally, I see something similar to https://ned14.github.io/boost.outcome/md_doc_md_03-tutorial_b.html#expected_... as a much better example to help sell outcome and therefore better suited as the motivating example for the front page of the docs.
The motivating example on the landing page is very tricky to get right. It can't be too long, else people give up instantly. It can't use too much library specific stuff, else it overwhelms. It has to assume a very wide range of viewer knowledge and capability. Yet it must still communicate the library in an instant, including that it works well with mixed C++ exceptions enabled and disabled codebases. The current motivating example was written by Andrzej because my previous one wasn't representative enough. I am not a good person to write these landing page code examples, I never look at them when looking at a library and they play zero part in my evaluation of a library when considering to use it. So I have no idea what other people, who do rely on these heavily, look for. Vicente suggested a different example by Andrzej instead. I will re-ponder that one. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
-----Original Message----- From: Boost [mailto:boost-bounces@lists.boost.org] On Behalf Of charleyb123 . via Boost Sent: 19 May 2017 14:43 To: boost@lists.boost.org Cc: charleyb123 . Subject: [boost] [review] Review of Outcome (starts Fri-19-May)
Hi Everyone,
The formal review of Niall Douglas' Outcome library starts today (Fri-19-May) and continues until Sun-28-May.
Your participation is encouraged, as the proposed library is uncoupled and focused, and reviewers don't need to be domain experts to appreciate the potential usefulness of the library and to propose improvements. Everyone needs (and has suffered) error handling, and can compose an opinion on that topic.
Outcome is a header-only C++14 library providing expressive and type-safe ultra-lightweight error handling, suitable for low-latency code bases.
I see no problem at all in requiring C++14 (or C++17, or even C++20). C++14 will soon be the default for LLVM and GCC. I don't see Outcome being retro-fitted to those stuck with older compilers. If a project is being retrofitted, then it is also a good time to upgrade to the latest compiler.
Key features:
*- Makes using std::error_code from C++11's
more convenient and safe *- Provides high-quality implementation of proposed std::expected (on C++20 standards track) *- Good focus on low-latency (with tests and benchmarks) *- Error-handling algorithmic composition with-or-without C++ exceptions enabled *- No dependencies (not even on Boost) This review is timely, as C++17 brings us std::optional<T>. The upcoming std::expected
(an implementation provided in Outcome) is a generalization of std::optional<T> that provides a value, where the unhappy result is a 'std::error_code' or an instance of "your-chosen-error-type". The library further provides 'outcome
' for handling to safely wrap throwing APIs. Documentation: https://ned14.github.io/boost.outcome/index.html
ACCU 2017 talk including design rationale: https://www.youtube.com/watch?v=XVofgKH-uu4
GitHub: https://github.com/ned14/boost.outcome
Latest tarball: https://github.com/ned14/boost.outcome/releases/download/boost_peer_review3/...
Note: Tarball might be easiest; but if you want to clone from GitHub directly, after the clone you should run the following command to get the source zip exactly: git submodule update --init --recursive
Please post your comments and review to the boost mailing list (preferably), or privately to the Review Manager (to me ;-). Here are some questions you might want to answer in your review:
- What is your evaluation of the design?
Terrifying. But making things simple and efficient for the user usually requires that.
- What is your evaluation of the implementation?
Way above my pay grade. Complex for reasons I don't pretend understand. I'd hope that any actual released stuff would be somewhat simpler. I understand that the requirement for as-yet-unfinished standards and compilers makes life very difficult - chicken'n'eggy. It is clear that a massive amount of work has gone into this, including a lot of worthwhile worrying about efficiency. So I think it much be considered quite refined. (It might still be wrong, but I have faith). It doesn't fit well into the Boost framework, but perhaps that is intentional? Having an include outcome.hpp that only consists of macros is a bit alarming.
- What is your evaluation of the documentation?
Hard going. Niall knows far, far, far too much already? Get someone else to write a tutorial on 'how to use outcome'?
- What is your evaluation of the potential usefulness of the library?
Very useful and very important.
- Did you try to use the library? With what compiler? Visual Studio Pro 2015 update 3.
Yes, via git to a Boost develop tree. I did modular-boost/libs> git clone https://github.com/ned14/boost.outcome.git modular-boost/libs> git submodule update --init --recursive I:\modular-boost\libs>git clone https://github.com/ned14/boost.outcome.git Cloning into 'boost.outcome'... remote: Counting objects: 7692, done. remote: Compressing objects: 100% (145/145), done. remote: Total 7692 (delta 179), reused 229 (delta 139), pack-reused 7397 Receiving objects: 100% (7692/7692), 3.49 MiB | 102.00 KiB/s, done. Resolving deltas: 100% (4929/4929), done. Checking connectivity... done. (I expected a folder in /libs called outcome, but I got one called boost.outcome so it wasn't in the alphabetical order or format that I expected, but I found it :-) I:\modular-boost\libs>cd ./boost.outcome I:\modular-boost\libs\boost.outcome>git submodule update --init --recursive Submodule 'doc/html' (https://github.com/ned14/boost.outcome.git) registered for path 'doc/html' Submodule 'include/boost/outcome/boost-lite' (https://github.com/ned14/boost-lite.git) registered for path 'include/boost/outcome/boost-lite' Cloning into 'I:/modular-boost/libs/boost.outcome/doc/html'... Cloning into 'I:/modular-boost/libs/boost.outcome/include/boost/outcome/boost-lite'... Submodule path 'doc/html': checked out 'caf37a25ea541ca2c8309df17329bdb23037e7fd' Submodule path 'include/boost/outcome/boost-lite': checked out 'ed8678b7b2c17f1064301e3f713a6593fe91f7f1' Submodule 'include/gsl-lite' (https://github.com/martinmoene/gsl-lite.git) registered for path 'include/boost/outcome/boost-lite/include/gsl-lite' Submodule 'pcpp' (https://github.com/ned14/pcpp.git) registered for path 'include/boost/outcome/boost-lite/pcpp' Cloning into 'I:/modular-boost/libs/boost.outcome/include/boost/outcome/boost-lite/include/gsl-lite'... Cloning into 'I:/modular-boost/libs/boost.outcome/include/boost/outcome/boost-lite/pcpp'... Submodule path 'include/boost/outcome/boost-lite/include/gsl-lite': checked out 'e6fb512f372d58a8f11dd99f74e79a3c9f0a9d15' Submodule path 'include/boost/outcome/boost-lite/pcpp': checked out '488dcd2ca0588bac267cfc32b8ec17c2620c4d5a' and I could then read the documentation at modular-boost/libs/boost.outcome/doc/html/index.html I expected to be able to run the examples using b2 but sadly this is not supported. Is there any reason why not (apart from an aversion to bjam?) Boost has a system (even if not the best or Niall's preference) for running on multiple platforms and this doesn't fit with it which feels Boost-unfriendly at least. So I fired up MSVC IDE and added a new project and added the simple_example.cpp and built and ran it in my normal way. No need for adding modular-boost to the search list. (But there is an worrying load of Boost-lite stuff hanging on which did not feel good. 1>------ Rebuild All started: Project: outcome, Configuration: Debug x64 ------ 1> simple_example.cpp 1> outcome.vcxproj -> J:\Cpp\Misc\x64\Debug\outcome.exe 1> outcome.vcxproj -> J:\Cpp\Misc\x64\Debug\outcome.pdb (Full PDB) 1>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\V140\Microsoft.CppCommon.targets(203,5): error MSB3073: The command ""J:\Cpp\Misc\x64\Debug\outcome.exe" --build_info=yes 1>C:\Program Files (x86)\MSBuild\Microsoft.Cpp\v4.0\V140\Microsoft.CppCommon.targets(203,5): error MSB3073: :VCEnd" exited with code 20. ========== Rebuild All: 0 succeeded, 1 failed, 0 skipped ========== I noted the lack of a licence and copyright statement throughout. However, it appears to run OK from the IDE 'start without debugging' but produced no output. Not comforting. It really isn't my idea of a simple example. So I tried expected_payload1.cpp since it looks (and was recommended) as a better working example. (VS moans about deprecated Posix names, but hey... and boost.outcome\example\expected_payload2.cpp(27): warning C4267: 'argument': conversion from 'size_t' to 'unsigned int', possible loss of data is ugly). I'm cool with namespace versioning, but I would hope it was a temporary bit of clutter/complexity. I also tried expected_payload2.cpp and that was more informative. I'd put this example first. The output "Failed to open file somefile due to 2" isn't an exemplary error message, even if it is only trying to show how outcome works. (Putting this string "Failed to open file somefile due to 2" as a comment would make it easy for the reader to understand without actually running the example). I tried to run the test expected_pass.cpp but got modular-boost\libs\boost.outcome\test\expected_pass.cpp(15): fatal error C1083: Cannot open include file: '../boost-lite/include/boost/test/unit_test.hpp': No such file or directory but it looks plausible. Again I'd expect a jamfile to do this.
- How much effort did you put into your evaluation?
Re-reading documentation and another quick skirmish.
- Are you knowledgeable about the problem domain?
No.
And most importantly:
- Do you think the library should be accepted as a Boost library?
I dream of a world where error codes had never been conceived and there was only one way of doing things where exceptions worked efficiently using hardware better. (I have seen convincing evidence that the cost of exceptions is over-estimated). I always wake up and curse the extra work that any error handling causes (usually 2 * pi * however-long-it-took-to-write-the-code-to-start-with). I found Outcome hard going, but I fear that is because it *is* hard. As Donald E. Knuth warned "Software is HARD". It is certainly a major weakness of much software that the result of an error is pretty much "Sorry - something went wrong :-( ". At best one gets "File not found." - but no clue t which F*&^ing file, or "trying to write outside array bounds", but not what array with what bounds, nor to which element, so Niall is right to put delivering 'payload' as a major goal. The proof of the pudding is in the eating. So far there are few tasters. For acceptance it really must have overall boostification to fit into Boost conventions, where possible or appropriate. I am sympathetic to authors reluctance to do this until acceptance is assured. In the end, we now have to decide if this is all going to be 'OK in the end' and have faith in Niall's skill, judgement, and determination to see this functional and finished. I do. So that's a yes. Paul --- Paul A. Bristow Prizet Farmhouse Kendal UK LA8 8AB +44 (0) 1539 561830 Nits noted ========== Lots of files missing Boost licence. This is of course essential for Boost. Some punctuation and a nicer: This eliminates the fragile switch statements converting between error code domains in favour of an information loss-free transmission. The cost is once again a loss of type safety because if a function might return an error code, it should never be able to return and the compiler will not complain.
- What is your evaluation of the documentation?
Hard going. Niall knows far, far, far too much already? Get someone else to write a tutorial on 'how to use outcome'?
This saddens me. Can you suggest particular hard going parts?
(I expected a folder in /libs called outcome, but I got one called boost.outcome so it wasn't in the alphabetical order or format that I expected, but I found it :-)
You need to do "git clone https://github.com/ned14/boost.outcome.git outcome" if you want to rename the output directory.
I expected to be able to run the examples using b2 but sadly this is not supported. Is there any reason why not (apart from an aversion to bjam?) Boost has a system (even if not the best or Niall's preference) for running on multiple platforms and this doesn't fit with it which feels Boost-unfriendly at least.
Outcome provides a very rich cmake experience. If accepted, I will implement bjam support.
However, it appears to run OK from the IDE 'start without debugging' but produced no output. Not comforting. It really isn't my idea of a simple example. So I tried expected_payload1.cpp since it looks (and was recommended) as a better working example. (VS moans about deprecated Posix names, but hey... and boost.outcome\example\expected_payload2.cpp(27): warning C4267: 'argument': conversion from 'size_t' to 'unsigned int', possible loss of data is ugly).
You hit on a good observation. More than half of the examples are not usable as programs, they are only there to compile successfully and get sucked into the documentation as code example. I've logged the issue to https://github.com/ned14/boost.outcome/issues/37.
The output "Failed to open file somefile due to 2" isn't an exemplary error message, even if it is only trying to show how outcome works. (Putting this string "Failed to open file somefile due to 2" as a comment would make it easy for the reader to understand without actually running the example).
Grr. Your STL's printer for error_code is not being helpful.
I tried to run the test expected_pass.cpp but got
modular-boost\libs\boost.outcome\test\expected_pass.cpp(15): fatal error C1083: Cannot open include file: '../boost-lite/include/boost/test/unit_test.hpp': No such file or directory
but it looks plausible. Again I'd expect a jamfile to do this.
If you don't want to use cmake, I placed a batch file in test called "withmsvc.bat" which speaks the right incantation for MSVC.
In the end, we now have to decide if this is all going to be 'OK in the end' and have faith in Niall's skill, judgement, and determination to see this functional and finished. I do. So that's a yes.
Many thanks for the review, and I am sorry that compiling and running code went so badly for you.
Nits noted ==========
Lots of files missing Boost licence. This is of course essential for Boost.
All of the implementation files, bar the auto generated one, should have licence boilerplate. The boilerplate is actually generated by a python script incidentally, saves me having to remember. You're right that the examples are missing those, historically due to doxygen. Logged to https://github.com/ned14/boost.outcome/issues/38 The test/expected_pass.cpp file is not my code, it's Vicente's from his test suite. If he prepends some licence boilerplate to his, it'll get picked up in mine.
Some punctuation and a nicer:
This eliminates the fragile switch statements converting between error code domains in favour of an information loss-free transmission. The cost is once again a loss of type safety because if a function might return an error code, it should never be able to return and the compiler will not complain.
Fixed in develop branch. Thank you. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Hi everyone, this is my review of Niall's Outcome library. It is my fist review, so better keep that in mind when considering my answers for Outcome's acceptance into Boost.
The formal review of Niall Douglas' Outcome library starts today (Fri-19-May) and continues until Sun-28-May.
Your participation is encouraged, as the proposed library is uncoupled and focused, and reviewers don't need to be domain experts to appreciate the potential usefulness of the library and to propose improvements. Everyone needs (and has suffered) error handling, and can compose an opinion on that topic.
Outcome is a header-only C++14 library providing expressive and type-safe ultra-lightweight error handling, suitable for low-latency code bases.
I think it is a good decision to target a more modern C++ standard, especially if it eases the implementation. (I am not such a big fan of re-implementing what is already in a particular standard, just to support old compilers. At least not for a new library.)
Key features:
*- Makes using std::error_code from C++11's
more convenient and safe *- Provides high-quality implementation of proposed std::expected (on C++20 standards track) *- Good focus on low-latency (with tests and benchmarks) *- Error-handling algorithmic composition with-or-without C++ exceptions enabled *- No dependencies (not even on Boost)
The last point, I think, is not really bad. Even if Outcome will be accepted into Boost and therefore becomes a part of Boost I think it is good idea to only depend on other Boost modules if it is absolutely necessary. (For example, I would not consider anyone re-inventing Boost.Hana. But if a simple Tribool class is needed, I am all for providing a simple one, especially, if it is just an implementation detail.) And as the library targets C++14 I am all for using smart-pointers from the standard instead of having a dependency on Boost.SmartPtr or Boost.Move. etc.
This review is timely, as C++17 brings us std::optional<T>. The upcoming std::expected
(an implementation provided in Outcome) is a generalization of std::optional<T> that provides a value, where the unhappy result is a 'std::error_code' or an instance of "your-chosen-error-type". The library further provides 'outcome
' for handling to safely wrap throwing APIs. [...]
Here are some questions you might want to answer in your review:
- What is your evaluation of the design?
I like it. The idea of combining different error-/return-code and exception handling seems great to me. Ensuring, that it can be compiled into very efficient machine-code is a very nice plus. However, I would like Niall to recheck if several of his code-optimizations that make the code hard to understand or maintain are still really needed for the set of supported compilers. As was mentioned in previous email-discussions this might not always be the case. (Maybe, it is even possible instead to provide some hints to compiler-vendors where to try to improve their optimizers?)
- What is your evaluation of the implementation?
It fascinates me to see so many macros in headers which auto-generate the names of header-files to be included. I personally like such automatisms where one only has to modify the values of some few macros in a single file to get a totally different behavior or implementation. (I myself happen to implement such automatisms quite often.) However, that always carries the risk of confusing the user/reviewer (or the future maintainer) who tries to understand what is going on. In general, this might be solvable with even more documentation/comments, but is still more work to grasp what's going on. However, a general user might not care at all as long as everything works fine out of the box. (So let's hope it always does.) Other than that, the implementation looks solid. Although, I must confess that I did not look into it in-depth. (It at least seems to be better documented with comments than many other Boost-libraries that I have looked into and tried to understand what is going on.)
- What is your evaluation of the documentation?
The "tutorial" is quite long but also quite good. I think I understood everything, because it explains most things in great detail. That's, why I would not consider it a "tutorial" but an in-depth documentation. I would expect a tutorial being more of a short introduction which explains the basic usage in not more than 3 to 4 screen-pages. The "landing-page" is more what I would have thought a tutorial would look like; a little longer maybe with smaller examples and some more text between them. But then again, that could just be me having a wrong impression what a tutorial in general should look like. At least, the entire documentation (and also the link to the ACCU-talk) helped me to understand what Outcome is for.
- What is your evaluation of the potential usefulness of the library?
I think, it is very useful. I myself would probably just need the main functionality "expected" provides (value or error) but having some extensions which can handle exceptions, too, is a good thing. With "outcome" and "result" being possibly empty I am not so sure. It probably is really better to report the empty-case through the error-code. (But I think, Niall is already considering this.) The benefit of having "option" is not totally clear to me. It looks similar to "std::optional". Is its implementation just more efficient than the one found in different standard-libraries? (Then again, std::optional comes with C++17 and Outcome targets C++14.) How about Boost.Optional? Would it make sense to have a dependency on it instead (and possibly improve its implementation with some tricks of Outcome's "option")? If "outcome" and "result" report the empty-case through the error-code then that would make "option" obsolete, because one could use "result" instead with an error_code that will be set to empty_value (or how it would be called). That would also remove a possible dependency on Boost.Optional.
- Did you try to use the library? With what compiler? Did you have any problems?
No, sorry, I did not.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
Several hours reading the docs, watching the ACCU-talk (and reading the mail-discussions on this mailing-list). A quick look into the code.
- Are you knowledgeable about the problem domain?
Just as much as a developer could be who has worked with code that uses exceptions, error-codes, return-codes and has lazy-co-developers (me included) that forget to check the return-codes or to catch exceptions.
And most importantly:
- Do you think the library should be accepted as a Boost library?
Yes, I would say so. This would be an unconditional yes, even though I would suggest to add a small "introductory-tutorial" to the documentation and would like to hear what Niall (and others) think about my above remarks concerning removal of "option" and re-checking of the specific compiler-optimizations.
For more information about Boost Formal Review Process, see: http://www.boost.org/community/reviews.html
Thank you very much for your time and efforts.
--charley
Thank you very much for putting so much work into Outcome, Niall, and Charley for being its review-manager. Deniz Bahadir
(Maybe, it is even possible instead to provide some hints to compiler-vendors where to try to improve their optimizers?)
Back some years ago I submitted bug reports with repros to clang and MSVC regarding the low quality of optimisation with Outcome. I have no idea if the repros were used, but the most recent versions of both compilers do significantly better than previous versions.
- Do you think the library should be accepted as a Boost library?
Yes, I would say so. This would be an unconditional yes, even though I would suggest to add a small "introductory-tutorial" to the documentation and would like to hear what Niall (and others) think about my above remarks concerning removal of "option" and re-checking of the specific compiler-optimizations.
Option is just a typedef, very easy to remove. Andrzej I think just volunteered to redo the landing for me to match what you want. Thank you for your review! Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Charley wrote:
Question: Is the intention to include it in Boost under the Boost License only? I see files that have the Apache license in addition to the Boost license. Glen -- View this message in context: http://boost.2283326.n4.nabble.com/review-Review-of-Outcome-starts-Fri-19-Ma... Sent from the Boost - Dev mailing list archive at Nabble.com.
Question: Is the intention to include it in Boost under the Boost License only? I see files that have the Apache license in addition to the Boost license.
It is licensed under your choice of either the Apache 2.0 or Boost licences. As the licence.txt file says. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On 5/19/17 6:43 AM, charleyb123 . via Boost wrote:
Hi Everyone,
The formal review of Niall Douglas' Outcome library starts today (Fri-19-May) and continues until Sun-28-May.
I was sort of astounded at the volume of comment this submission received. I can't go through it all. I can barely skim it. Most of this went into implementation details which I wasn't prepared to invest the time to understand. But it piqued my interest, so I resolved to write a review from the stand point of a naive user such as myself who might want to improve his approach to error handling. I confess embarked on this task with a certain dread. I had previously gone through Naill's documenation of AFIO and found it completely unintelligible. Right away, I'll say that this is much, much better.
Outcome is a header-only C++14 library providing expressive and type-safe ultra-lightweight error handling, suitable for low-latency code bases.
LOL - This is from the doc - and right away is off putting. It includes a lot which may not be true. I would have preferred something like: "Output is a form of variant. It can hold either some value type or some type indicating an error result. Usage of Output will ease the coding and usage of functions which might return errors."
Key features:
*- Makes using std::error_code from C++11's
more convenient and safe
The documentation provides useful explanatory information which isn't really available anywhere else in a convenient manner. But lots of system_error aren't addressed - like a key part - error_condition. It's not reallypart of outcome even though outcome supports it. It's a separate subject. I think almost all reference to it should be dropped from outcome. Useful information regarding error_code should be completed and added to boost::system_error via a PR to the boost.system library documentation.
*- Provides high-quality implementation of proposed std::expected
(on C++20 standards track)
Personally, I find it distracting when authors describe their own work as high quality, or other laudatory phraseology. Spend the effort demonstrating the problems that it solves for me other rather than how great you believe it is.
*- Good focus on low-latency (with tests and benchmarks)
The benchmarks seem to show that outcome is similar in performance to returning an integer or error_code. This is what I would expect. The benchmarks also purport to show that exception handling is much more expensive than returning an integer or error_code. Well we know that if the exception is thrown. But what if it isn't as is usual? What about the cost of checking the error at each level? This isn't addressed which is not surprise as it would be difficult to make a convincing benchmark which shows these numbers. Since the timings for exceptions provide no useful information and the other ones are all the same, I would just drop the whole section and replace it with a sentence "outcome provides similar efficiency as returning a returning any other error indicator."
*- Error-handling algorithmic composition with-or-without C++ exceptions enabled
I would drop just about all the references to exceptions. It's an orthogonal issue. If you've decided you need exceptions or that they are convenient in your context, you're not going to change to outcome. If you've already decided on returning error indicators (for whatever reason), you're not going to switch to exceptions. You might want to switch to outcome. So comparison with exceptions isn't really relevant at all. Pretty much the same for error codes. You might or might not use them - but this is a totally orthogonal question as to whether you use outcome or not as it is (I think) designed to handle any type as an error return types. Eliminating all the irrelevant material would make the package much, much easier to evaluate and use. FWIW - I touched upon some of these points in https://www.youtube.com/watch?v=ACeNgqBKL7E
*- No dependencies (not even on Boost)
Yowwww. This terrible. I looked a little through boost-lite. config.hpp - repeats a lot of stuff from boost/config. Doesn't seem to provide for compilers other than gcc, clang and some flavors of msvc. Looks like some stuff might have some value - but with not docs/comments it's pretty much useless if something goes wrong - like a compiler upgrade. chrono.hpp - what is this doing in here? precompiling <tuple> etc..... Looks like you're trying to solve some other problem here - goes on an on. The questions of pre-compiled headers/modules/C++11 subset of boost have nothing to do with outcome and shouldn't be included here. Focus on one and only one thing: The boost.outcome library. It's quite a small library and trying to make it carry the freight for all these other ideas will deprive it of the value it might have.
This review is timely, as C++17 brings us std::optional<T>. The upcoming std::expected
(an implementation provided in Outcome) is a generalization of std::optional<T> that provides a value, where the unhappy result is a 'std::error_code' or an instance of "your-chosen-error-type". The library further provides 'outcome
' for handling to safely wrap throwing APIs.
I didn't understand this either before or after reading the documentation. I should confess that I don't understand the value of exception_ptr, nested_exceptions, etc. either. Another good reason for excluding discussion of exceptions and probably some code from the library documentation and code.
Documentation: https://ned14.github.io/boost.outcome/index.html
The documentation is built with Doxygen - which, though convenient, works against producing helpful readable documentation.
Note: Tarball might be easiest; but if you want to clone from GitHub directly, after the clone you should run the following command to get the source zip exactly: git submodule update --init --recursive
Please post your comments and review to the boost mailing list (preferably), or privately to the Review Manager (to me ;-). Here are some questions you might want to answer in your review:
- What is your evaluation of the design? LOL - it's hard to discern the design from the docs and code. But
Another bad idea. Git is complex enough without adding submodules at
the library level. I would have to spend more time than I want to
uderstand what good this is doing for us. It's likely this would go
away of the scope of the library were limited to what it should be: outcome.
there's not many ways to implemented. The questions I would have would
be about never empty states and stuff like that but I don't see any of
that in the docs.
Another very weird thing. In spite of the size of the documents, there
is not reference page for outcome
::implementation_type implementation_type
What the heck is this - and why would I need to know it? It's marked
public which suggests that as a user I might need to access it - but the
documentation does tell me what it does or why I would like to mess with it.
OK - now I turn to the class definition of outcome. I see a boatload of
constructors. Hmmm I had thought that outcome was a template of the form:
boost::outcome
- What is your evaluation of the implementation?
Needs to be slimmed down and all the extraneous stuff removed. This isn't a vehicle for upending boost. This is trying to get as modest simple header only library into boost in a way which makes it useful to users nothing more.
- What is your evaluation of the documentation?
A lot better then I expected!
- What is your evaluation of the potential usefulness of the library? I think it would be useful. It's basically a specialization of variant. As such it doesn't bring much we don't already have. BUT, a good explanation and simple usage would promote a good and consistent way of addressing returned error codes ans so might be useful
- Did you try to use the library? With what compiler? Did you have any problems?
I only read the documentation and perused the source code.
- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
A couple of hours reading the docs and source code.
- Are you knowledgeable about the problem domain?
somewhat. I understand error_code and have implement a version of this facility to address a need in my own code.
And most importantly:
- Do you think the library should be accepted as a Boost library?
No There is a temptation to say Yes - with conditions. I would hope that this temptation is resisted. It should just be rejected. There is no way that this library can be fixed without separating out huge parts of it: preprocessing all the headers - it's very hard to follow this, it even looks like it includes parts of the standard library as well as boost. What if every library did this? error_code looks intertwined at least in the docs. The class versioning scheme is interesting. I could live with it but I'm not convinced that boost should adopt it. And I'm less convinced that just one library - especially one that looks so simple as outcome. If one thinks I'm being two harsh, I propose the following experiment: Take a pair of left over interns from C++Now and direct them to the outcome git hub package. Give them a simple task to use the outcome library. See if they can complete it in one hour or less. On the other hand, I believe it wouldn't be too tough to take the pieces of this library already made and compose them into a more tightly focused package that would fit into boost in a natural way that users would useful and easy to use. But the look and feel of the package would be entirely different than the current one so It would really have to be reviewed again. I also believe that if the package were re-formulated as I suggest, the review process would be much, much less onerous for everyone involved. FWIW - my views on what a library has to have are summarized here: https://www.youtube.com/watch?v=ACeNgqBKL7E
On Fri, May 26, 2017 at 5:22 PM, Robert Ramey via Boost
The documentation is built with Doxygen - which, though convenient, works against producing helpful readable documentation.
I must object. Both Beast and NuDB use doxygen in their toolchain, and I think these pages are perfectly helpful and readable: http://vinniefalco.github.io/beast/beast/ref/websocket__stream/async_read.ht... http://vinniefalco.github.io/nudb/nudb/ref/basic_store/fetch.html If these pages look familiar, its because they use the same doxygen, xslt, and boost.book toolchain that Boost.Asio uses: http://www.boost.org/doc/libs/1_64_0/doc/html/boost_asio/reference/basic_str... So don't blame the tool, but blame the configuration. Doxygen generated HTML is terrible but with a decent XSLT program you can turn that worm into a butterfly. In fact, I have written such a library that can be included in anyone's C++ project to produce documentation that resembles that of Boost.Asio and Beast. Its called docca: https://github.com/vinniefalco/docca With some effort, Outcome could integrate docca and get documentation similar to the examples I provided above. I tried such an integration but I was stymied by the need to run a python preprocessor over the Outcome source code in order to produce valid C++ which Outcome requires, so I gave up. I am sure that the Outcome author could perform an integration since he is much more knowledgeable about his own code base - I am available to offer assistance with the docca portion. Thanks
Le 27/05/2017 à 03:02, Vinnie Falco via Boost a écrit :
On Fri, May 26, 2017 at 5:22 PM, Robert Ramey via Boost
wrote: The documentation is built with Doxygen - which, though convenient, works against producing helpful readable documentation. I must object. Both Beast and NuDB use doxygen in their toolchain, and I think these pages are perfectly helpful and readable:
http://vinniefalco.github.io/beast/beast/ref/websocket__stream/async_read.ht...
http://vinniefalco.github.io/nudb/nudb/ref/basic_store/fetch.html
If these pages look familiar, its because they use the same doxygen, xslt, and boost.book toolchain that Boost.Asio uses:
http://www.boost.org/doc/libs/1_64_0/doc/html/boost_asio/reference/basic_str...
So don't blame the tool, but blame the configuration. Doxygen generated HTML is terrible but with a decent XSLT program you can turn that worm into a butterfly. In fact, I have written such a library that can be included in anyone's C++ project to produce documentation that resembles that of Boost.Asio and Beast. Its called docca:
https://github.com/vinniefalco/docca
With some effort, Outcome could integrate docca and get documentation similar to the examples I provided above. I tried such an integration but I was stymied by the need to run a python preprocessor over the Outcome source code in order to produce valid C++ which Outcome requires, so I gave up. I am sure that the Outcome author could perform an integration since he is much more knowledgeable about his own code base - I am available to offer assistance with the docca portion.
I believe what Robert mean is that the generated documentation of Boost.Outcome using doxygen has a lot of flaws. Of course it is tempting to say that it is because of doxygen. As you showed in the examples we can do better. Using Doxygen requires to be more careful and sometimes replace some possible issues by workaroounds. Nevertheless Doxygen on code using the pre-processor is a nightmare. Have you experience on these cases? I will tale a deeper look at your examples :) Vicente
On 5/26/17 11:57 PM, Vicente J. Botet Escriba via Boost wrote:
I believe what Robert mean is that the generated documentation of Boost.Outcome using doxygen has a lot of flaws. Of course it is tempting to say that it is because of doxygen.
As you showed in the examples we can do better. Using Doxygen requires to be more careful and sometimes replace some possible issues by workaroounds.
Nevertheless Doxygen on code using the pre-processor is a nightmare. Have you experience on these cases?
I will tale a deeper look at your examples :)
I understand the appeal of Doxygen. The idea if annotating the code with narrative comments which can accessed in a user friendly form is very appealing. It's natural for a programmer, easy to do, easier to keep up to date and is less tedious than building a giant reference after the fact. It also embodies the ideas and principles of literate programming in a practical way. So I see a lot of good in it. BUT a) It's support for templates is limited to specification of template parameters. It has no way to specify Type Requirements (aka concepts) aside from just typing in the narrative. It has no way to specify a concept and refer to it. b) It's doesn't seem a convenient method for specifing things like tutorials, examples, design concepts etc. c) By providing a place to add a little narative to each method it gives the programmer the impression that he's documenting his library - which he is not actually doing. In most cases all he is doing is making the comments in the code visible through some browser. Nothing wrong with this, but he's not really adding anything that one couldn't gain from just browsing the source code. d) If your source code isn't readable, you can create the impression that it's designed when it fact it isn't. That is it's useful for obscuring the fact that your code actually doesn't have any kind of design. So, for fans of Doxygen who have nothing else to do: a) Figure out how Doxygen customization works. I see this is huge slew of macros and settings which .... - well I don't know what it does. I guess it needs better documentation or perhaps design. b) Figure out how to express a concept in Doxygen and refer to it. c) Restrict usage of Doxygen to building xml reference pages for classes and class templates, functions and function templates. d) figure out how to meld this reference boost book using some other tool like quickbook, xml_mind, emacs or whatever. e) Be sure you make a good explanation so some naive person can create library documentation with your tools. e) If you get all this right - a tall order - be prepared to invest a lot of effort to evangelize your solution. This will be a very long and frustrating exercise. Or if you want to strike out on your own separate path - generally my own personal preference - you might consider an approach like caramel which would build on the idea of embedding boost book tags in comments and post processing the code. That's all I have to say about that. Robert Ramey
On Sat, May 27, 2017 at 11:09 AM, Robert Ramey via Boost < boost@lists.boost.org> wrote:
On 5/26/17 11:57 PM, Vicente J. Botet Escriba via Boost wrote:
I believe what Robert mean is that the generated documentation of Boost.Outcome using doxygen has a lot of flaws. Of course it is tempting to say that it is because of doxygen.
As you showed in the examples we can do better. Using Doxygen requires to be more careful and sometimes replace some possible issues by workaroounds.
Nevertheless Doxygen on code using the pre-processor is a nightmare. Have you experience on these cases?
I will tale a deeper look at your examples :)
Or if you want to strike out on your own separate path - generally my own personal preference - you might consider an approach like caramel which would build on the idea of embedding boost book tags in comments and post processing the code.
You can also just include Quickbook docs in your source and include those in a coherent manner in your regular quickbook docs. Since there's no "automatic" class, function, etc. mechanics it forces you to write real documentation in your source where it's also accessible to people just browsing code. It also make it easier for people to contribute to your project as they submit docs along with code changes. This approach has been essential in getting a good number of external submissions to Predef, for example. -- -- Rene Rivera -- Grafik - Don't Assume Anything -- Robot Dreams - http://robot-dreams.net -- rrivera/acm.org (msn) - grafikrobot/aim,yahoo,skype,efnet,gmail
On 5/27/17 9:19 AM, Rene Rivera via Boost wrote:
On Sat, May 27, 2017 at 11:09 AM, Robert Ramey via Boost <
Or if you want to strike out on your own separate path - generally my own personal preference - you might consider an approach like caramel which would build on the idea of embedding boost book tags in comments and post processing the code.
You can also just include Quickbook docs in your source and include those in a coherent manner in your regular quickbook docs. Since there's no "automatic" class, function, etc. mechanics it forces you to write real documentation in your source where it's also accessible to people just browsing code. It also make it easier for people to contribute to your project as they submit docs along with code changes. This approach has been essential in getting a good number of external submissions to Predef, for example.
So that's even a fourth way. and there are probably more. I would like to emphasize that this is not an easy task. It's not just a question inserting a new, clever and cool little tool into the documentation tool chain. It's about changing the developer mindset about what documentation is and should be. The reason this is very, very difficult is that everyone thinks that they're not the problem - that other people are. But if that were true, there would be no complaints about documentation. It's also my view that difficulties in writing documentation reflect bad design decisions. But I'm having problems convincing developers of that. (I wanted to move this to a root thread but it's not showing that way on my news server output) Robert Ramey
It's also my view that difficulties in writing documentation reflect bad design decisions. But I'm having problems convincing developers of that.
Another way of looking at this is this: if you can explain the library concisely and completely in simple documentation that makes sense to the user, then the design is likely to be a good one. Or to put it another way, writing documentation is an important part of the design process, because it flushes out the sloppy thinking that we all fall for from time to time... John. --- This email has been checked for viruses by AVG. http://www.avg.com
On Sat, May 27, 2017 at 9:09 AM, Robert Ramey via Boost
I understand the appeal of Doxygen. ... BUT ... c) By providing a place to add a little narative to each method it gives the programmer the impression that he's documenting his library - which he is not actually doing. In most cases all he is doing is making the comments in the code visible through some browser. Nothing wrong with this, but he's not really adding anything that one couldn't gain from just browsing the source code.
Right. I'd like to make it clear to all readers of my posts which promote Doxygen and my docca library, that I am advocating for its usage specifically to build ONLY a "reference." That is, a synopsis of each public symbol (functions, classes, constants, etc...) exposed by the library, with appropriate helpful descriptions. Example: http://vinniefalco.github.io/beast/beast/ref/http__read_some/overload1.html Good documentation MUST include more than just this reference, it should also include the exposition that we have become accustomed to in Boost library documentation. An explanation of the audience, what problems the library solves, holistic tutorial on how to put things together to achieve a goal, and so forth. This of course will not be captured by Doxygen and instead should be developed as its own separate source code which is edited by hand much the way someone might write a novel. The approach used in docca and Beast is to incorporate this additional exposition not generated by doxygen as a set of ".qbk" files. The reference is then included as a pice of this larger work. To reiterate, Doxygen is a very useful piece (but not the only part) of a documentation tool chain in order to generate a reference from Javadoc comments attached to declarations in source code. It should be part of a larger work that includes the traditional exposition found in high quality documentation.
So don't blame the tool, but blame the configuration. Doxygen generated HTML is terrible but with a decent XSLT program you can turn that worm into a butterfly. In fact, I have written such a library that can be included in anyone's C++ project to produce documentation that resembles that of Boost.Asio and Beast. Its called docca:
https://github.com/vinniefalco/docca
With some effort, Outcome could integrate docca and get documentation similar to the examples I provided above. I tried such an integration but I was stymied by the need to run a python preprocessor over the Outcome source code in order to produce valid C++ which Outcome requires, so I gave up. I am sure that the Outcome author could perform an integration since he is much more knowledgeable about his own code base - I am available to offer assistance with the docca portion.
I probably didn't tell you I tried a docca build of Outcome's docs. I also gave a go of standardese, and doxyrest which is another doxygen XML to Sphinx converter. In all cases I felt that while the appearance of the documentation improved, but I also felt that the **utility** of it dropped. Part of that would be the additional work to fill in the missing gaps of doxygen markup support in each of the tools above, but also because things like keyword search don't work right, or member function documentation is too bunched together visually, or too many really long pages get generated which drown out detail. doxygen's output is ugly, but you can find what you want fairly quickly. I thus settled on doxygen as being the least worst of the alternatives available to me. Don't get me wrong, it sucks, but it is less painful to maintain than any of the others. One of my biggest peeves with all these doxygen alternatives is how much pain they cause me to update the documentation. If I have to run anything more than a single command which does it all, I don't like it. If Outcome is accepted, I may make an attempt at my own documentation generator based on libstandardese for the code parsing, but generating a https://gohugo.io/ website. I'm as sick and tired of awful documentation as everyone here, but I also want something with **zero** maintenance overhead for me, ideally something which goes into the cmake build cycle and I literally never, ever have to think about it. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On 5/27/17 6:55 AM, Niall Douglas via Boost wrote:
I thus settled on doxygen as being the least worst of the alternatives ...
I do believe that a number of us - and like more users of boost - see a problem. That is that they agree with me. But it's not nearly as easy to address as one would first think. I don't want to denigrate Doxygen, it's a real sincere effort and not without value - but the whole literate programming thing needs more work. Robert Ramey
On 27/05/2017 01:22, Robert Ramey via Boost wrote:
The formal review of Niall Douglas' Outcome library starts today (Fri-19-May) and continues until Sun-28-May.
I was sort of astounded at the volume of comment this submission received. I can't go through it all. I can barely skim it. Most of this went into implementation details which I wasn't prepared to invest the time to understand. But it piqued my interest, so I resolved to write a review from the stand point of a naive user such as myself who might want to improve his approach to error handling.
I confess embarked on this task with a certain dread. I had previously gone through Naill's documenation of AFIO and found it completely unintelligible. Right away, I'll say that this is much, much better.
You should thank Reddit. My only contribution was not choosing to break my arm instead of rewriting the docs for a fourth time.
Outcome is a header-only C++14 library providing expressive and type-safe ultra-lightweight error handling, suitable for low-latency code bases.
LOL - This is from the doc - and right away is off putting. It includes a lot which may not be true. I would have preferred something like:
"Output is a form of variant. It can hold either some value type or some type indicating an error result. Usage of Output will ease the coding and usage of functions which might return errors."
Reddit feedback didn't like that description BTW, I did try it. And the current description was not written by me in fact, it was donated through dislike of my previous descriptions.
Key features:
*- Makes using std::error_code from C++11's
more convenient and safe The documentation provides useful explanatory information which isn't really available anywhere else in a convenient manner. But lots of system_error aren't addressed - like a key part - error_condition.
The entire of https://ned14.github.io/boost.outcome/md_doc_md_03-tutorial_b.html is about nothing other than error_condition and how to use it.
*- Good focus on low-latency (with tests and benchmarks)
The benchmarks seem to show that outcome is similar in performance to returning an integer or error_code. This is what I would expect.
The benchmarks also purport to show that exception handling is much more expensive than returning an integer or error_code. Well we know that if the exception is thrown. But what if it isn't as is usual? What about the cost of checking the error at each level? This isn't addressed which is not surprise as it would be difficult to make a convincing benchmark which shows these numbers. Since the timings for exceptions provide no useful information and the other ones are all the same, I would just drop the whole section and replace it with a sentence "outcome provides similar efficiency as returning a returning any other error indicator."
There have been repeated calls for hard numbers with repeatable benchmarks others can run, usually with implication that I am lying to people or deceiving them somehow by giving them an open source library free of cost. I personally agree with you that such toy benchmarks are not useful, but the people cry for red meat, if anything they want more useless benchmarks and hard numbers. Donations of more benchmarks are welcome!
*- Error-handling algorithmic composition with-or-without C++ exceptions enabled
I would drop just about all the references to exceptions. It's an orthogonal issue. If you've decided you need exceptions or that they are convenient in your context, you're not going to change to outcome. If you've already decided on returning error indicators (for whatever reason), you're not going to switch to exceptions. You might want to switch to outcome. So comparison with exceptions isn't really relevant at all.
You may have a missed a major use case for outcome: calling STL code in a C++ project which is C++ exceptions disabled. Much of the games and finance industry are interested in Outcome precisely because of the ease Outcome enables passing through C++ exceptions thrown in a STL using island through code with exceptions disabled. Even if you're not doing that, another major use case is to keep exception throws, and having to deal with control flow inverting unexpectedly, isolated into manageable islands. The rest of your intermediate code can then assume no exception throws, not ever. That means no precautionary try...catch, no need to employ smart pointers, no need to utilise RAII to ensure exception safety. It can be a big time saver, especially during testing for correctness because it eliminates a whole load of complexity.
Pretty much the same for error codes. You might or might not use them - but this is a totally orthogonal question as to whether you use outcome or not as it is (I think) designed to handle any type as an error return types.
You are thinking of expected
Eliminating all the irrelevant material would make the package much, much easier to evaluate and use.
With respect Robert, I don't think you understood all the use cases. Most have called for significantly *more* documentation to cover in detail even more use cases, not less. (Incidentally I tried less documentation three times previously. For some reason, people need to be hand held more when it comes to describing really simple and obvious-to-me at least stuff)
*- No dependencies (not even on Boost)
Yowwww. This terrible. I looked a little through boost-lite. config.hpp - repeats a lot of stuff from boost/config. Doesn't seem to provide for compilers other than gcc, clang and some flavors of msvc.
It actually provides for all the C++ 14 compilers currently available. If they are not MSVC, GCC nor clang, they pretend to be one of those, so the same detection routines work. You may also have missed the extensive use of C++ 17 feature test macros. All C++ 14 compilers, even MSVC, support those now.
Looks like some stuff might have some value - but with not docs/comments it's pretty much useless if something goes wrong - like a compiler upgrade.
The C++ 17 feature test macros ought to be observed by all future compiler upgrades.
chrono.hpp - what is this doing in here?
precompiling <tuple> etc..... Looks like you're trying to solve some other problem here -
goes on an on.
I think you didn't realise those are the namespace bind files allowing the C++ 11 STL to be switched with the Boost STL. They are auto generated by a clang AST parser.
The questions of pre-compiled headers/modules/C++11 subset of boost have nothing to do with outcome and shouldn't be included here. Focus on one and only one thing: The boost.outcome library. It's quite a small library and trying to make it carry the freight for all these other ideas will deprive it of the value it might have.
That would not be a widely held opinion. Most end users of Outcome don't want any Boost connection at all. They do want excellent top tier cmake support, with automatic usage of compiler technologies such as precompiled headers, C++ Modules, clang-tidy passes, unit test dashboarding, sanitisers, ABI guarantees, and lots of other goodies. boost-lite provides lots of common cmake machinery used by all my libraries to save me reinventing the wheel for each. Again, none of this matters to an end user who just wants to #include and go. You can do exactly this with Outcome. It neither uses nor interferes with anything in Boost. It is an excellent neighbour, including to other versions of itself.
The library further provides 'outcome
' for handling to safely wrap throwing APIs. I didn't understand this either before or after reading the documentation. I should confess that I don't understand the value of exception_ptr, nested_exceptions, etc. either. Another good reason for excluding discussion of exceptions and probably some code from the library documentation and code.
Just because you don't understand the value of exception_ptr and nested exceptions doesn't mean you are in a majority. Nested exceptions support in the C++ 11 STL wouldn't be there unless WG21 thought them highly useful.
I expect to see type requirements (aka concepts) for T and E. Can any types be substituted? It's a mystery. Hmmm maybe initial example and/or looking at the source code will be useful. First of all, the example has no #include
or some such. I go looking for that and .... I can't find it. It's somewhere hidden through a maze of ... preprocessing?
There are static asserts which fire if the types used don't meet their requirements. You are correct they are not documented in the reference API docs. There is an open issue to fix that.
- What is your evaluation of the implementation?
Needs to be slimmed down and all the extraneous stuff removed. This isn't a vehicle for upending boost. This is trying to get as modest simple header only library into boost in a way which makes it useful to users nothing more.
As I have said many times now, end users can #include and go. They don't care, nor need to care, how the implementation works so long as it cannot cause them misoperation.
If one thinks I'm being two harsh, I propose the following experiment: Take a pair of left over interns from C++Now and direct them to the outcome git hub package. Give them a simple task to use the outcome library. See if they can complete it in one hour or less.
I have seen completely uninitiated users get up and running with Outcome
in less than five minutes. If the end user has programmed in Rust or
Swift before, they just need to be told:
#include "boost.outcome/include/boost/outcome.hpp"
boost::outcome::expected
On the other hand, I believe it wouldn't be too tough to take the pieces of this library already made and compose them into a more tightly focused package that would fit into boost in a natural way that users would useful and easy to use. But the look and feel of the package would be entirely different than the current one so It would really have to be reviewed again. I also believe that if the package were re-formulated as I suggest, the review process would be much, much less onerous for everyone involved.
Thanks for the review. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
On 5/27/17 6:29 AM, Niall Douglas via Boost wrote:
On 27/05/2017 01:22, Robert Ramey via Boost wrote:
*- Error-handling algorithmic composition with-or-without C++ exceptions enabled
I would drop just about all the references to exceptions. It's an orthogonal issue. If you've decided you need exceptions or that they are convenient in your context, you're not going to change to outcome. If you've already decided on returning error indicators (for whatever reason), you're not going to switch to exceptions. You might want to switch to outcome. So comparison with exceptions isn't really relevant at all.
You may have a missed a major use case for outcome: calling STL code in a C++ project which is C++ exceptions disabled. These applications are not using exceptions right now. They might use outcome as an alternative to an integer return code. But exceptions are not relevant to this case.
Much of the games and
finance industry are interested in Outcome precisely because of the ease Outcome enables passing through C++ exceptions thrown in a STL using island through code with exceptions disabled.
I have to confess it never occurred to me what happens when a program compiled with exceptions disabled uses the "throw" statement. I would have assumed that it would fail to compile.
Even if you're not doing that, another major use case is to keep exception throws, and having to deal with control flow inverting unexpectedly, isolated into manageable islands. The rest of your intermediate code can then assume no exception throws, not ever. That means no precautionary try...catch, no need to employ smart pointers, no need to utilise RAII to ensure exception safety. It can be a big time saver, especially during testing for correctness because it eliminates a whole load of complexity.
I get this. But this is likely already being addressed with code like:
// presume that sqrt throws when passed a negative number or Nan or???
float safe_sqrt(float x, int & error_flag){
float y;
try {
y = sqrt(x);
error_flag = 0;
}
catch(const std::domain_error & de){
error_flag = 1;
}
return y;
}
which you would recommend replacing with something like:
// presume that sqrt throws when passed a negative number or Nan or???
outcome
Pretty much the same for error codes. You might or might not use them - but this is a totally orthogonal question as to whether you use outcome or not as it is (I think) designed to handle any type as an error return type You are thinking of expected
where E is selectable by the end user.
Outcome's outcome<T> and result<T> hard code the error type to error_code_extended. So error codes are the only game in town for those refinements. Hmmm - it didn't get that the E parameter had to be error_code. I
I wasn't thinking of Expected. I saw Vicent's presentation of it some years ago and found it unconvincing so I forgot about it. thought it could be anything - which would have decoupled outcome from error_code.
Eliminating all the irrelevant material would make the package much, much easier to evaluate and use.
With respect Robert, I don't think you understood all the use cases.
Right - that's my complaint. I read the documentation. I saw a 3 cases in the introduction. It wasn't apparent that it would be useful for things other than that.
Most have called for significantly *more* documentation to cover in detail even more use cases, not less.
Maybe - Either there other use cases that aren't in the docs or they're wrong. If they're other uses cases which aren't obvious, that's sort of red flag for me. It suggests that either the library might have non-obvious behavior. But I'm just commenting on what I currently see.
(Incidentally I tried less documentation three times previously. For some reason, people need to be hand held more when it comes to describing really simple and obvious-to-me at least stuff)
More is not necessarily better
*- No dependencies (not even on Boost)
Yowwww. This terrible. I looked a little through boost-lite. config.hpp - repeats a lot of stuff from boost/config. Doesn't seem to provide for compilers other than gcc, clang and some flavors of msvc.
It actually provides for all the C++ 14 compilers currently available. If they are not MSVC, GCC nor clang, they pretend to be one of those, so the same detection routines work.
You may also have missed the extensive use of C++ 17 feature test macros. All C++ 14 compilers, even MSVC, support those now.
Right - this is the wrong approach. If you think that boost/config.hpp needs fixing you can address that. But mixing configuration into a particular library just generates more work rather than factoring out into the best implementations.
Looks like some stuff might have some value - but with not docs/comments it's pretty much useless if something goes wrong - like a compiler upgrade.
The C++ 17 feature test macros ought to be observed by all future compiler upgrades.
chrono.hpp - what is this doing in here?
precompiling <tuple> etc..... Looks like you're trying to solve some other problem here -
goes on an on.
I think you didn't realise those are the namespace bind files allowing the C++ 11 STL to be switched with the Boost STL. They are auto generated by a clang AST parser.
gratuitious complexity.
The questions of pre-compiled headers/modules/C++11 subset of boost have nothing to do with outcome and shouldn't be included here. Focus on one and only one thing: The boost.outcome library. It's quite a small library and trying to make it carry the freight for all these other ideas will deprive it of the value it might have.
I'd like to. But it looks like accepting outcome into boost will effectively mean accepting a boat load of other unrelated stuff. So if you want to address just outcome, remove all the other stuff.
That would not be a widely held opinion. Most end users of Outcome don't want any Boost connection at all.
No problem. If you believe that don't submit it to Boost.
They do want excellent top tier cmake
support, with automatic usage of compiler technologies such as precompiled headers, These are available to users - not required to add to libraries C++ Modules, Not ready yet clang-tidy passes, don't know what that is. unit test All libraries support unit test without adding confusing stuff to their
LOL - which would require CMake being quality product - which I don't believe it is. library
dashboarding, not related to the library sanitisers, already supported. ABI guarantees, This is interesting and I might accept this. But but I'm not crazy about it being snuck in the back door. and lots of other goodies. nope
boost-lite provides lots of common cmake machinery used by all my libraries to save me reinventing the wheel for each. LOL - that's not what it looks like. I don't think library code should be tied to anything outside it.
Again, none of this matters to an end user who just wants to #include and go. You can do exactly this with Outcome. It neither uses nor interferes with anything in Boost. It is an excellent neighbour, including to other versions of itself.
Just because you don't understand the value of exception_ptr and nested exceptions doesn't mean you are in a majority. Nested exceptions support in the C++ 11 STL wouldn't be there unless WG21 thought them highly useful.
Right - I'm just relating the experience of one user - myself. But I don't think I'm untypical. I have to say I've never seen exception_ptr or nested_exception in any code. If you think this is important, you need to provide information on or at least point to references which describe how this stuff because most of us who don't read the standard text don't even knows it exists.
I expect to see type requirements (aka concepts) for T and E. Can any types be substituted? It's a mystery. Hmmm maybe initial example and/or looking at the source code will be useful. First of all, the example has no #include
or some such. I go looking for that and .... I can't find it. It's somewhere hidden through a maze of ... preprocessing? There are static asserts which fire if the types used don't meet their requirements.
LOL - so one doesn't know what the requirements are until you compile the code? How can write code this way?
You are correct they are not documented in the reference API docs. There is an open issue to fix that.
OK - but
- What is your evaluation of the implementation?
Needs to be slimmed down and all the extraneous stuff removed. This isn't a vehicle for upending boost. This is trying to get as modest simple header only library into boost in a way which makes it useful to users nothing more.
As I have said many times now, end users can #include and go.
That's certainly not apparent from the examples.
They don't care,
LOL - this user cares nor need to care, I'm not convinced of this.
how the implementation works so long as it cannot cause them misoperation.
Up to a point this is true. But for a simple type such as this one I have some expectation what the implemenation should look like and when I look here - I can't see anything that makes much sense to me.
If one thinks I'm being two harsh, I propose the following experiment: Take a pair of left over interns from C++Now and direct them to the outcome git hub package. Give them a simple task to use the outcome library. See if they can complete it in one hour or less.
I have seen completely uninitiated users get up and running with Outcome in less than five minutes.
Honestly I can't believe this. I would have to see it for myself.
If the end user has programmed in Rust or Swift before, they just need to be told:
#include "boost.outcome/include/boost/outcome.hpp"
boost::outcome::expected
... and they're good to go.
LOL - I'll start learning Rust so I can use outcome.
On the other hand, I believe it wouldn't be too tough to take the pieces of this library already made and compose them into a more tightly focused package that would fit into boost in a natural way that users would useful and easy to use. But the look and feel of the package would be entirely different than the current one so It would really have to be reviewed again. I also believe that if the package were re-formulated as I suggest, the review process would be much, much less onerous for everyone involved.
Thanks for the review.
You're welcome Robert Ramey
Niall
Le 27/05/2017 à 19:56, Robert Ramey via Boost a écrit :
On 5/27/17 6:29 AM, Niall Douglas via Boost wrote:
On 27/05/2017 01:22, Robert Ramey via Boost wrote:
Pretty much the same for error codes. You might or might not use them - but this is a totally orthogonal question as to whether you use outcome or not as it is (I think) designed to handle any type as an error return type You are thinking of expected
where E is selectable by the end user. I wasn't thinking of Expected. I saw Vicent's presentation of it some years ago and found it unconvincing so I forgot about it. :(
Vicente
participants (17)
-
Andrzej Krzemienski
-
Bjorn Reese
-
charleyb123 .
-
degski
-
Deniz Bahadir
-
Emil Dotchevski
-
Glen Fernandes
-
Jared Wyles
-
John Maddock
-
Michael Caisse
-
Niall Douglas
-
Paul A. Bristow
-
Peter Dimov
-
Rene Rivera
-
Robert Ramey
-
Vicente J. Botet Escriba
-
Vinnie Falco