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

- What is your evaluation of the design?

The design appears to not have been finalized, the documentation states as much:

Well of course it isn't. I knew reviews here would demand many breaking changes. So users are warned of this. Remember you were looking at the Standalone documentation. Users there needed to be made aware of upcoming instability.

The policy-based design is especially problematic because of the expressed desire to provide interoperability between diverse APIs each using different error reporting. This is a bit counter-intuitive, because the natural inclination is to provide maximum flexibility, but in this case other considerations are more important. There is a reason why in C libraries the default error reporting mechanism is to return int, even though the language does permit programmers to return structs, which would be more flexible.

As the last section of the tutorial covers, there is a non-source-intrusive mechanism for externally specifying interoperation rules to handle libraries using one policy interoperating with libraries with different policies. This lets Eve stitch together the Alice and Bob libraries without having to modify their source code, and without affecting any other libraries. I personally think this Outcome's coup de grace and why it's the only scalable choice for large programs considering using this sort of error handling.

Speaking of interoperability, it is especially tricky to report errors from C-style callbacks. This would be nice to support because C++ exceptions are off-limits in this case, yet it is sometimes desirable to communicate user-specific information across the third-party C callback mechanism (this is sometimes supported by a void * user data pointer, but that is not always the case).

The kind of objects that can survive crossing API boundaries would be of basic types, or have one or two members of basic types. Think std::shared_ptr<T>, which _always_ consists of two pointers, rather than SmartPtr which consist of who knows what. Note that this is not the same problem as "using result<T> from C code", which Outcome allows. The important question is not how do I use result<T> from C, but how do I transport result<T> across a third party context which knows nothing about Outcome (as a side note, C++ exceptions also cause issues when crossing API boundaries, even if we set exception safety requirements aside.)

How do you transport any type across a third party context which knows nothing about that type? There is a long list of standard techniques. Outcome is just another C++/C type, and any of those techniques apply just the same to it for that problem solution. I don't see the issue you're making here.

[snip why Emil loves exceptions] So, the design of "an error handling library that does not use C++ exceptions" has to target the tricky bits where exceptions would be annoying or can't be used, not the general case where they can.

I get Emil that you love exceptions. So you won't like the whole premise behind Outcome, indeed you wrote an alternative to Outcome v1 last year to prove the pointlessness of the Outcome premise. But lots of people don't agree with you, and they want something like this in C++.

Finally, I'll point out that a lot of the positive feedback comes from people who think that it is a good idea to replicate the Rust error reporting mechanism in C++. This seems to be an axiomatic belief, since I've never seen anyone attempt to substantiate it. This is important, as it is typical for programmers coming from other languages to be shocked by various C++ language features, and this should not be confused with problems in the C++ language specification.

If this was being forced on all users across the board, I can see you might have a point. But this is an opt in library, and nobody is claiming nor pretending that anything but a minority will ever want to use it. Using Outcome in your code does come with costs in terms of maintenance and learning curve. Most C++ users will never want nor need it.

- What is your evaluation of the implementation?

Lack of C++11 support could be problematic. The use of macros to disambiguate namespace is cumbersome. Overall the library relies heavily on macros, which is not a good thing for a C++ library.

I have no idea where you get the idea that the library relies heavily on macros. The only obligatory macro is BOOST_OUTCOME_V2_NAMESPACE. All the rest are optional. And as I've explained several times now, the permuting namespace is to force DLLs built against version X of Outcome to interoperate with other DLLs built against Y of Outcome via the ValueOrError Concept interface. I'd have thought Emil given your experience in games that you'd understand the importance of stable ABI guarantees. This makes stable ABI guarantees possible for the standalone Outcome. Now, any Boost.Outcome would eventually decay its macro into boost::outcome::v2 once a final v2 has arrived. That's not a v2 design, that's a v2 *implementation*. I'll be running the ABI Compliance Checker per commit once v2 is decided upon to ensure it remains stable. Again, this is what makes stable ABI guarantees possible. I'm amazed that you don't get this.

It seems like Outcome wants to stay away from Boost. By that I mean that great care has been taken to decouple it from Boost, and I sense that this desire comes from the target audience: the so-called "low latency" crowd wouldn't touch Boost with a 9 foot pole. While it is generally a bad idea to speculate about such things, it seems to me the motivation for submitting Outcome for a Boost review is not to benefit the Boost community; for us the coupling with Boost is not a problem.

The decoupling is purely for ease of maintenance. I can script patchsets from a Boost.Outcome into Outcome, and vice versa. I am sorry that you have read ulterior motives into what is an engineering choice.

- What is your evaluation of the documentation?

The documentation seems complete.

- Are you knowledgeable about the problem domain?

Yes.

- Do you think the library should be accepted as a Boost library?

The library should be rejected.

Thank you for your review. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/