Re: [boost] [review] Review of Outcome (starts Fri-19-May)

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