On 05/06/2017 05:43, Gottlob Frege via Boost wrote:
I've now read ALL the emails. Here's some thoughts, in no particular order.
Firstly, thank you Tony for wading through all ~750 emails. A lot of people couldn't find the time, but I think that for those who did it was a very worthwhile discussion. I'm currently mostly focused on my final maths exam on Friday - that being the same degree I was studying whilst with you at BlackBerry that meant I started a few months late - so my comments below are more off the top of my head than anything. Still, I've shifted position from where I was at, and I think I mostly have Robert and Gavin to thank for it.
- I wonder, Niall, if your uses of the empty state could/should be compared to how people sometimes use -1 or 0, etc - ie "special values as errors". I typically don't like that pattern (yet I've done it anyhow - performance/laziness/etc - but typically only in localized cases, not in "APIs"). So I'm wary about this empty state thing.
If we do have an empty state, I like it as a separate type - code needs to deal with it separately. I like the idea of taking a possibly-empty type, checking for empty, and creating a never-empty type, and passing that along so future code need not worry.
My view on that is: - Use the non-empty type for function returns. - Maybe use the empty type locally as a local optimisation. - Internal code hidden away in detail can do what it likes as it always does anyway.
- I think the narrow and wide contracts should be in the same type, not separate types. This is C++. I suggest "access" if you want long names. ie T & val = oc.access_value(); // possible UB
(I almost also suggested "deref" but "deref_value" is wrong - you are deref'ing the outcome, not the value)
Nice name choice. As I've mentioned, the wide-by-default observers will have narrow editions too, now quite likely prefixed by .access_*(). But I think there remains use for a dedicated type which is all narrow - for example, imagine a test suite which typedefs in the narrow edition and runs the code through static analysis and makes debug binaries with the sanitisers enabled, and then typedefs in the wide edition for production binaries. I'm not saying that will suit all, but it is a valid use case.
- I agree with Vicente's concern that functions that behave differently need to be named differently. I haven't looked at the new (or old) API carefully enough, but as a (possibly hypothetical) example: wide_type::value() // returns value, or throw if no value narrow_type::value() // returns value, or UB if no value These functions are not the same, and should not be named the same. Since you thus need different names anyhow (ie value and access_value), why not put them both on the same API? I'm not yet sure if you end up with similar examples with possibly_empty_type::something() and never_empty_type::something().
I'm not as bothered about this as some people. From my perspective, different types are different types, .get() in one is allowed to be subtly different from .get() in another.
- I see value in the idea that the narrow contract takes more typing, but I actually *like* using * and -> for the narrow contract because I have 30+ years of training in looking at * and -> and recognizing it as "this may be unsafe, examine the code to see how it is guaranteed". Boost.Optional specifically chose * for that very reason.
My big objection was, and still is, and will continue to be that if you are not fulfilling the pointer/iterator contract, stop behaving partially and brokenly somewhat like a pointer/iterator. Optional is not a pointer, cannot act like a pointer, and making operator*() available on it encourages people to do (*o) all the time instead of binding an lvalue ref to its .value() which is far safer, more explicit as to intent, and better coding.
- exception() returns error, error() doesn't return exception, etc: would it be better/easier/clearer if we introduced another name, such as "failure" or whatever? ie exception() only returns the stored exception, error() only returns the stored error(), and then has_failure() would return true in either case, failure_exception() returns exception, generates an exception from error if necessary, etc?
A number of people have suggested this. I've not been persuaded personally, in Outcome .has_exception() already returns true if error or exception. In Outcome there is always a linear flow of less to greater representation throughout the design, so you can always use the most representative type/observer and you are *guaranteed* to never lose information with the minimum possible boilerplate. Now, some may say that calling .exception() on an errored object is wasteful because you must silently construct an exception_ptr for a system_error. But equally, I'd counter that if you actually care about that, check .has_error() and yank out the error_code before calling .exception(). But I'll come back to this point later down below.
- I dislike "exceptions are for exceptional circumstances". Yes it is a common thing, but it is usually wrong. Using Outcome to enable the pattern of "handle errors, but totally cancel the operation on exceptions" might actually turn out to be a useful pattern, but I think this is actually slightly different from the "exceptions are for exceptional cases" camp, as they typically use that chant to choose whether their API will use exceptions or errors, not both in a single API. So I agree with Emil - for most APIs, error-codes vs exceptions" is basically trying to handle the same thing - failure (or "disappointment").
This certainly is not the case in AFIO v2, nor any of the stuff built on top. There expected failure is something handled at the point of failure. Unexpected failure means abort the current operation entirely. The upcalling of expected failure into unexpected-please-abort is common enough too, for example in AFIO's byte range locks if the unlock operation fails after a corresponding lock, we convert that into an abort because that probably means a network drive has vanished and there is no point doing anything more now.
- this ring buffer thing sounds interesting. I haven't looked at it at all, so maybe this makes no sense: should it use thread-local?
Alas passing error_code instances between threads is extremely common, and we want to keep error_code_extended's move constructor trivial.
- I think std::expected should always throw something deriving from std::exception. So if E derives from std::exception, we can throw it. If E is exception_ptr we can (re)throw it. If E is error_code/system_error/etc we can figure out what to throw. If E is user-type, then we wrap it in bad_expected or whatever. (And/or allow user E-to-exception customization somehow). This sounds complicated, but it is easily summarized/taught as "the standard always throws std::exceptions"
It should be born in mind that all these added layers you suggest, and indeed Peter has been adding to his implementation, well both me and Vicente have been down this road already. Original Expected had lots of complicated special carve outs for certain types in T and/or E. Outcome in the past had more special carve outs too. But I'll come back to this below.
- I HATE the dual interface of std::filesystem. Worse, there is a chance it is setting a precedent for future std APIs (dual APIs have been proposed elsewhere). Let's please kill it before a dual API strikes again.
Networking TS is in some ways worse rather than better because it will supply failures to you in an error_code, but not let you supply failures to it the same way :(. I always wished that ASIO made the const error_code& passed in non-const, and you could set it on return from the handler. You can still throw an exception of course to abort everything, but sometimes you don't want to abort everything. Sometimes you just want to fail.
However, the other problem with trying to "single-fy" the std::filesystem (or any other) API is that users who want error codes sometimes don't want the cost of the extra data (paths, etc). The only way to handle that may be with customizable callback functions that capture the error-info however you like, or something similar to callbacks.
If the function is inlined, the compiler is very clever at totally eliding returning anything unused. It doesn't even compute the value.
- I really look forward to Peter's never-empty variant - and have been hoping for one ever since std::variant was finalized. Not because I dislike std::variant, but just because I think the option should be there for those who need/want it. Regardless, std::variant, in my world, is never empty, since it is only empty when a move operator throws, which should be never and/or when I can't allocate 64 bytes, in which case I'm screwed anyhow. Some days I'm like "man just accept empty, it would be a simple API", but then I think "it is stupid for variant
to be empty". Other days I think "just double buffer when necessary" (I assume that's your direction Peter?), but then I think "I don't want double-buffering variant to go on/off based on whether I use MS std vs libc++ etc" and also "I don't want double buffering for cases that only happen in theory, not in practice". I like the trade-offs of std::variant. I do agree that Boost should have a variant2 with different choices than std::variant - I think that actually helps the standard - we can choose a direction and instead of telling people "tough luck" when they disagree, we can say "use boost variant2 when you want those properties".
Personally speaking, I think the valueless-by-exception state should remain possible if the user feeds std::variant more than one type with a throwing move constructor. For all other cases, you get guaranteed never empty. I think that a reasonable balance. Using std::variant already adds significantly to compile times, sufficiently so I would be cautious of **ever** including that header in another header file. Adding double buffering would make compile time load even worse. And besides, if the user is foolish enough to feed std::variant types with throwing move constructors, they deserve what they get!
- I agree with those who were saying (or thinking) that Boost was - and should again be - the place where APIs go before going into the standard. APIs (mostly) shouldn't bypass boost and go to the standard directly. Or at least an API needs a similar user-base (both in scale and diversity - ie an internal google API has scale, but not necessarily diversity). Since I know a number of LEWG members agree with me on this, don't be surprised to see some push-back from the committee for proposals without that kind of backing, and also don't be surprised if Boost gets flooded with "the committee told me to come here first". Of course I don't speak for LEWG, so maybe none of that happens, we haven't had that discussion yet.
Me personally, I have LONG had serious concerns about authors skipping Boost and going straight to LEWG. I have been banging those concerns for years now. People weren't listening until very recently. What I would also say is that LEWG authors do NOT need to commit to a full Boost library with all that entails. The two most important things they need to do is a peer review and get a reasonable number of users, preferably > 100. That may lead naturally to a Boost library, but it also may not because some libraries are unsuitable for Boost. What I mean is that in the end Boost cannot accept a library like Outcome where a lack of consensus exists about what form such a thing should take. But the peer review was extremely valuable, and the publicity generated brings in users. So from the point of view of LEWG, all boxes are ticked, even if no Boost library results from the submitted library. In other words, **successful rejection** is also a very high value outcome with large benefits to the wider C++ ecosystem.
I actually probably had more to say - I should have made notes as I was reading all... those... emails.
Right, so on to where I am currently at regarding Outcome's future. Some
may have noticed I have been experimenting with a C++ 17 std::variant
based Outcome, but I have been becoming increasingly dismayed with the
compile time costs, and that's with a far from complete implementation.
This is why presented Outcome does so many tricks with the preprocessor,
it was to reduce compile time load to something reasonable. Without
those tricks we are back to where I was before where I think it
unreasonable to ask users with large code bases to use these objects in
their public APIs :(
Robert's and Gavin's thoughts on a much simpler (i.e. non-variant,
non-constexpr) design and implementation have found surprising resonance
with me in the days since the review ended. After all:
sizeof(error_code_extended) = 24
sizeof(exception_ptr) = 8
So for a type predominantly used solely for returning results from
functions where you will only ever be returning one of them at a time,
there is a very strong argument in favour of ditching the variant
storage in exchange for considerable improvements in:
- implementation complexity
- compile times for end users
- load on the compiler's optimiser
... for the hardcoded EC = error_code_extended and E = exception_ptr
Outcome. 32 bytes of overhead over sizeof(T) I suspect will be
unmeasurable statistically (though I will benchmark before deciding).
Plus, class outcome<T> can subclass class result<T> which in turn will
subclass std::optional<T> or T depending on whether it is empty-capable
or not. We would retain a variant-like construction API. I like that
simplicity.
If I go down this route, you would get all narrow observers because that
makes much more sense here - error_code_extended and exception_ptr
default construct to a null object, so just return those directly. Here
a .failure_*() observer family which synthesise failures when needed
would also make lots of sense.
If I do take that route, I think I would drop any Expected
implementation in Outcome, any constexpr programming support, and any
monadic programming support, and with that I can return to C++ 11 only.
I can already see Expected is going to become expected