[review] Outcome Review Report
# Boost.Outcome REVIEW REPORT
In conclusion to the Outcome Library Review (19-May to 02-June, 2017): The
library is REJECTED.
Rejection is principally due to:
(1) Unresolved key behavior at conclusion of the review period
(2) Misunderstanding/disagreement regarding library’s intended purpose
(3) Unclear precedent/implications from accepting this library in its
current configuration into the Boost distribution
The following weighed heavily to consider accepting the library into Boost:
(a) The library demonstrates solutions to fundamental issues (error and
exception handling)
(b) The library effectively addresses constraints to serve the needs of
several user-bases
(c) New paradigms/idioms are enabled
(d) Implementation is of high quality
Some 20+ participants sent 850+ email messages to the public list
discussing the Outcome library, plus additional (non-trivial) off-list
discussion. Public reviews sent to the list were from:
*- Paul Bristow -- ACCEPT (Tue-23-May)
*- Deniz Bahadir -- ACCEPT (Wed-24-May)
*- Robert Ramey -- REJECT (Fri-26-May)
*- Peter Dimov -- REJECT (Sun-28-May)
*- Emil Dotchevski -- REJECT (Sun-28-May)
*- Vicente J. Botet Escriba -- REJECT (Tue-30-May)
*- Andrzej Krzemienski -- REJECT (Wed-31-May)
*- Gavin Lambert -- REJECT (May 31)
*- Bjorn Reese -- REJECT (Fri-02-June)
The remainder of this report is a summary of discussion with select issues
specific to the Outcome library, plus some possible topics for further
consideration by the Boost Community and Boost Steering Committee.
This further elaboration is not intended to be comprehensive regarding
issues raised during the review; Rather, it is to:
(a) Provide historical “summary/context” for the level of discussion
exhibited during this review, and which contributed to the final decision;
(b) Highlight issues that may provide assistance in considering
“readiness” for a later review for an evolved form of this library (or for
a similar library intending to address the same domain);
(c) Provide “discussion-points” for the Boost Community and Boost
Steering Committee regarding possible future evolution or guidance
associated with Boost tooling, distribution, and individual library
dependencies (related to topics raised during this review).
The conclusions will be:
(1) Outcome Library Author: Consider re-submitting for review when...
(a) Key behavior/design issues are resolved (see below)
(b) Integration plan with Boost is more clear (and consistent with
Boost Steering Committee Guidance)
(2) Boost Community / Boost Steering Committee: Consider providing
guidance to future library submissions regarding...
(a) Attempted dual-license, this submission attempted:
*- BSL+Apache2 dual-license
(b) “Extra-build-steps”, this submission attempted:
*- Custom preprocessing to “generate-source” (to significantly
improve compile-time speed) (Note: During review, a template-based
solution was proposed as a possible replacement mechanism, but code-gen
should probably be explicitly addressed)
(c) Class/namespace versioning, this submission attempted:
*- Stable ABI/versioning mechanism through a framework provided
with this submission (to enable multiple versions of this library used by
differing third-party libraries to correctly coexist within the same binary)
(d) Use of outside source; use of git (git-submodules), this
submission attempted:
*- Dependency to an outside (git-submodule) lib archive (gsl-lite:
https://github.com/martinmoene/gsl-lite.git )
(e) Use of (non-test/Jamfile) for test, this submission attempted:
*- Custom test mechanism (because “Boost.Test” won’t work with C++
exceptions disabled; lib needed “boost-lite” to run the test suite)
(f) Use of CMake, Dependency upon Boost, this submission attempted:
*- No Boost dependency (but provided a “boost-lite” shim to coexist
in user-workspaces with-or-without Boost, providing common cmake machinery
and test-running)
# SUMMARY OVERVIEW
As demonstrated through the volume and depth of discussion, the library
attracted wide interest, which is consistent with a perception that it
attempts to address an active need.
Comments suggested the library’s purpose (enabling public APIs to pass
<quote> (1) User wants failure reason type to be local to only the code to which it applies. So think custom E type, nonconvertible to any other E type, and E is never error_code nor exception_ptr. They specifically want to keep failure handling local a small patch of code e.g. constexpr evaluation contexts.
All-narrow observers make sense for this use case as object usage is tightly integrated to the code using it, and failure is handled entirely locally.
(2) User wants failure reason type to be globally understood and will be choosing either E = std::error_code, or E = std::exception_ptr. They specifically want failure handling to be able to traverse any or all code, just like C++ exception throws. This is where Outcome was primarily aimed at.
All-wide observers make sense for this use case, as due to handling a wide degree of uncertainty, most failure handling code is *generic* i.e. we test for some specific failure causes, and for everything else we either ignore or hand it off to other code to handle.
(3) User wants to write functional programming logic using the basic vocabulary of Maybe, Either and i/o monads and basic operators of bind, fmap, do etc and probably some subset or refinement of Hana for the collections monads, though my GSoC student may be making the Ranges TS a choice here as well later this summer.
All-narrow observers make sense for this use case as the monadic operators ensure your function will never be called with the wrong state. </quote>
The Outcome library uses a, “single implementation, multiple personalities”
to solve this problem, and makes the intentional decision to attempt
targeting (2).
# ISSUE: Unclear precedent/implications from including this library in its
current configuration into the Boost distribution
The library in its current configuration is perceived as setting precedent
for changes to the Boost distribution, which reviewers commented added
complexity to their analysis.
Specific topics:
*- Dual-license (BSL and Apache2) confused reviewers
*- Add bjam support (author agreed)
*- Packaging relied upon git-submodules to an outside lib archive
(gsl-lite, https://github.com/martinmoene/gsl-lite.git)
*- Library does tricks with the preprocessor to avoid significant
compile-time costs; but these tricks were commented by some reviewers as
being unnecessarily complex.
*- The Author found these tricks necessary to reach reasonable
compile metrics for large projects using Outcome in their public APIs, and
believes some mechanism must address this build-speed-need. A proposed
metaprogramming mechanism may eliminate the need for these preprocessor
tricks in the future.
# FINAL THOUGHTS
In the very active years of “Classic Boost” (such as 2004-2008),
fundamental libraries had a good chance of becoming vocabulary types that
attempt to establish new Best Practice for the industry (and not merely be
“just another library”). This awareness drove much of the review
discussion, and underscored the importance of experimentation and “lessons
learned” to result in a library that offered: (1) behavior and idioms that
are understood; and (2) sufficient experience to make use recommendation to
the wider user base in contrast to existing alternative approaches.
Consistent with this historical context, it should be relatively
uncontroversial to recognize that the Outcome library does not reach that
threshold at this time (so it is rejected).
However, this review also suggests:
(1) A similar library is needed;
(2) The community lacks agreement on direction (e.g., “Multi-Modal” vs.
“Single-Modal”)
In support of experimentation and to expand on our “lessons-learned”, it
seems somewhat obvious that Outcome provides a strongly-implemented
Multi-Modal low-latency library that will assist the community in gaining
experience with these idioms proposed.
Since conclusion of the review period, discussion continues (on and
off-list) regarding possible evolution to the Outcome library, related to
feedback provided during this review, and related to discussions of design
evolution for pertinent libraries (e.g., possible changes to
std::expected<>, and evolution of a never-empty ‘variant2<>’). It is
promising that such a strong library as Outcome continues to receive
critical attention and evolution by multiple interested parties.
Great thanks to the Boost Community for their tremendous efforts,
disciplined review, and detailed exploration of topics in considering this
library submission.
Sincerest appreciation to Niall Douglas (Author of the Outcome Library) for
pushing the envelope for low-latency
On Tue, Jun 6, 2017 at 8:34 AM, charleyb123 wrote:
Great thanks to the Boost Community for their tremendous efforts, disciplined review, and detailed exploration of topics in considering this library submission.
Charley, thank you for your efforts in managing the review! Niall, thank you both for the submission, as well as for injecting life (and generating some exceptionally useful discussion) in our community. Glen
charleyb123 wrote:
It is perhaps ironic that the suitability of a proposed library as providing vocabulary types and possibly evolving into a future standard demands that the Library Author change the library’s stated purpose due to this potential popularity;
The demand in this case was to actually _not_ change the library, as it fulfilled the vocabulary role well in its initial, submitted for review, design.
charleyb123 wrote:
It is perhaps ironic that the suitability of a proposed library as providing vocabulary types and possibly evolving into a future standard demands that the Library Author change the library’s stated purpose due to this potential popularity;
Peter Dimov respondeth:
The demand in this case was to actually _not_ change the library, as it fulfilled the vocabulary role well in its initial, submitted for review, design.
Fair point. My intended comment was to suggest that the library's potential popularity caused, "the bar to be raised" regarding the need for critical API and design scrutiny, and the need for consensus (whereas a more niche library would have fewer implied consequences for failing to identify missed-opportunities). Thank you (Peter) for your heroic efforts, analysis, and comments in this review period (they were formative and fundamental to the discussion). --charley
charleyb123 wrote:
My intended comment was to suggest that the library's potential popularity caused, "the bar to be raised" regarding the need for critical API and design scrutiny, and the need for consensus (whereas a more niche library would have fewer implied consequences for failing to identify missed-opportunities).
Yes, it is sometimes ironic how the potential for greatness leads people to expect and demand greatness, rather than merely adequacy.
Thank you (Peter) for your heroic efforts, analysis, and comments in this review period (they were formative and fundamental to the discussion).
Thank you for having the presence of mind to read the volumes of discussion the review generated, and thanks to Niall for being such a good sport. Having one's library criticized in the merciless manner characteristic of Boost's reviews can be quite taxing on one's nerves and he took it in stride. I'll await the next submission of this, on balance, excellent library, and hope that it will stay true to the spirit of its original design.
On 06/06/2017 11:38 PM, Peter Dimov via Boost wrote:
Yes, it is sometimes ironic how the potential for greatness leads people to expect and demand greatness, rather than merely adequacy.
Damn-well said. We all should learn to temper our expectations and more so demands... especially if/when it is somebody else to actually fulfill those. Disappointed the lib is rejected. I very much hope Niall has the courage and perseverance to address and to re-submit.
Since conclusion of the review period, discussion continues (on and off-list) regarding possible evolution to the Outcome library, related to feedback provided during this review, and related to discussions of design evolution for pertinent libraries (e.g., possible changes to std::expected<>, and evolution of a never-empty ‘variant2<>’). It is promising that such a strong library as Outcome continues to receive critical attention and evolution by multiple interested parties.
class result { T _value; EC _error; // 24 bytes
Thank you so much Charley for review managing one of the lengthiest and most complex reviews we have had here at Boost in some year. I like to take a month's break from all programming after a review has ended, as it happens I have lots of non-programming stuff backlogged anyway, so it's good timing. But for those not following the continuing discussions, the rough plan moving forward is this: 1. Expected implementation to be no longer implemented by Outcome, maybe by Peter Dimov's Variant2 library at some future point instead. 2. Outcome to drop the variant based storage. Outcome gets simplified into this synopsis (design still evolving, this is a snapshot of current thinking): ``` template< class T, class EC = error_code_extended public: // Constructors to be inspired by std::variant's // Narrow observers T &access_value() noexcept; EC &access_error() noexcept; // Wide observers T &value(); EC &error(); }; template< class T, class P = shared_ptr<void>, class EC = error_code_extended, class E = exception_ptr
class outcome
: private result
Great thanks to the Boost Community for their tremendous efforts, disciplined review, and detailed exploration of topics in considering this library submission.
Sincerest appreciation to Niall Douglas (Author of the Outcome Library) for pushing the envelope for low-latency
handling and for submitting his work to the Boost Review process.
Thank you to the Boost community so much for helping me with the design of this library. It helps a great deal to return to the drawing board armed with so much high quality feedback. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
Consider also "assume" instead of "accept":
T & t = oc.assume_value();
?
On Tue, Jun 6, 2017 at 11:15 AM, Niall Douglas via Boost
Since conclusion of the review period, discussion continues (on and off-list) regarding possible evolution to the Outcome library, related to feedback provided during this review, and related to discussions of design evolution for pertinent libraries (e.g., possible changes to std::expected<>, and evolution of a never-empty ‘variant2<>’). It is promising that such a strong library as Outcome continues to receive critical attention and evolution by multiple interested parties.
Thank you so much Charley for review managing one of the lengthiest and most complex reviews we have had here at Boost in some year.
I like to take a month's break from all programming after a review has ended, as it happens I have lots of non-programming stuff backlogged anyway, so it's good timing. But for those not following the continuing discussions, the rough plan moving forward is this:
1. Expected implementation to be no longer implemented by Outcome, maybe by Peter Dimov's Variant2 library at some future point instead.
2. Outcome to drop the variant based storage. Outcome gets simplified into this synopsis (design still evolving, this is a snapshot of current thinking):
class result { T _value; EC _error; // 24 bytes
``` template< class T, class EC = error_code_extended public: // Constructors to be inspired by std::variant's
// Narrow observers T &access_value() noexcept; EC &access_error() noexcept;
// Wide observers T &value(); EC &error(); };
template< class T, class P = shared_ptr<void>, class EC = error_code_extended, class E = exception_ptr
class outcome : private result
{ using base = result ; union { P _payload; // 16 bytes E _exception; // 8 bytes }; public: // Constructors to be inspired by std::variant's // Explicit construction from compatible result<_T, _EC> template
explicit outcome(result<_T, _EC>); // Narrow observers using base::access_value; using base::access_error; P &access_payload() noexcept; E &access_exception() noexcept;
// Wide observers using base::value; using base::error; P payload(); E exception(); // Returns exception state, but if errored, synthesises // an exception_ptr of the error code E failure(); };
static_assert(sizeof(outcome<void>) == 40); // bytes overhead ```
I've left out some detail, but the above gives the idea.
Great thanks to the Boost Community for their tremendous efforts, disciplined review, and detailed exploration of topics in considering this library submission.
Sincerest appreciation to Niall Douglas (Author of the Outcome Library) for pushing the envelope for low-latency
handling and for submitting his work to the Boost Review process. Thank you to the Boost community so much for helping me with the design of this library. It helps a great deal to return to the drawing board armed with so much high quality feedback.
Niall
-- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
2017-06-06 14:34 GMT+02:00 charleyb123 . via Boost
# Boost.Outcome REVIEW REPORT Great thanks to the Boost Community for their tremendous efforts, disciplined review, and detailed exploration of topics in considering this library submission.
Sincerest appreciation to Niall Douglas (Author of the Outcome Library) for pushing the envelope for low-latency
handling and for submitting his work to the Boost Review process. -- charley (Outcome Review Manager
Charley, thank you for the immense work. This review was tough. Also, thanks to Niall, for creating and sharing this cool library; and for an inspiring discussion. Looking forward to the second review. Regards, &rzej;
participants (7)
-
Andrzej Krzemienski
-
charleyb123 .
-
Glen Fernandes
-
Gottlob Frege
-
Niall Douglas
-
Peter Dimov
-
Vladimir Batov