- Boost - lists.preview.boost.org

[outcome] non-interface-related concerns
by Andrzej Krzemienski 05 Jun '17

05 Jun '17

The discussion mostly revolves around the design of the interface of `outcome::expected` and even `std::expected`. My questions here are about the way this library will be distributed to users. And I am uncomfortable with the situation. I am not necessarily saying it is wrong, but I do not understand what is going on here. 1. The current GitHub repository consists of sub-repos. Do you (Niall) plan to have it like this when the library is accepted? Is it relevant? 2. This macro BOOST_OUTCOME_DISABLE_PREPROCESSED_INTERFACE_FILE, it controls whether I use this "preprocessed" file versus if I use... what? Normal header files? What is gained by this preprocessed file? Not having to include more than one header, or something else? Can I get different results when compiling with and without this macro? 3. I am recommended to use this macro BOOST_OUTCOME_V1_NAMESPACE. I am told it has something to do with ABI stability? Can you elaborate on how this guarantees ABI stability. Or is its purpose different? I have a certain problem with understanding this philosophy. A couple of days back, I reported the compilation failure in MinGW. I can see that the issue (https://github.com/ned14/boost.outcome/issues/36) is reorted as fixed in branch 'develop'. I would like to confirm that the bug is fixed. I checkout branch develop. There is no branch develop for the sub-repos, so what am I supposed to checkout, master, or some particular commit? I went with master. I do not understand a lot of CMake, but since it is a header-only library, I assume that I can just #include the file "include/boost/outcome.hpp", and my programs (compiled manually) should compile. But the only change in the commit that was supposed to fix the bug, was in a .cmake file, so I cannot see how it would help fix a header only library. Also, would the fix be visible in a "precompiled" header, or normal headres, or both? I am not complaining about this particular fix. I would like to know how in this system of mcros, sub-modules and preprocessed headers, I can figure out what changes will be observed in my program, when I see a particular change in git sub-repo. Regards, &rzej;

10 42

[outcome] Review
by Bjorn Reese 03 Jun '17

03 Jun '17

I apologize in advance for the rather meek review below; I did not have the time to be more thorough. > - What is your evaluation of the design? The review discussions provided many insightful arguments in favour or against various aspects of the design, and I have not formed a final opinion about most of these. I do believe that fixing E for result<T> to error_code is the right decision. I would prefer a design without an empty state, because we already have a vocabulary type for it. The choice of C++14 seems motivated solely by performance considerations. A slower C++11 version should be considered. > - What is your evaluation of the implementation? I did not have enough time to review it. > - What is your evaluation of the documentation? The introduction (Description) only provides a vague description of the purpose of the library. For example, it does not mention that its types are used to carry either a value or an error. Instead, it assumes that the reader can derive this from prior familiarity with expected<T> (or Rust.) The first motivating example is too large (as indeed are the examples in the tutorial.) Instead I suggest a "Tony table" that shows the essense of the library (either value or error.) For example: Before: FILE *file = fopen("myfile", "r"); if (!file) throw std::system_error(errno, std::system_category(), "Error delivered out-of-band by errno"); fclose(file); After: result<FILE *> file = fopen_wrapper("myfile", "r"); if (!file) throw std::system_error(file.error(), "Error delivered in-band by file.error()"); fclose(file.value()); This may not look overly convincing, but it does demonstrate the either value or error aspect in a few lines. The library is weakly motivated -- the motivation boils down to a less expensive alternative to throwing exceptions. A stronger motivation would have been to mention use cases were errors cannot be propagated by throwing exceptions. Here are two examples: One motivating use case where exceptions cannot be throw are callbacks, such as Boost.Asio asynchronous handlers. Here the return value is not returned by the asynchronous operation (the initiating function), but will be passed to the provided callback once the operation completes. In other words, the following synchronous operation return_type operation(arg_type, error_code&); is turned into this asynchronous operation: void async_operation(arg_type, void (*)(return_type, error_code)); Exceptions cannot be thrown into the functions (here the callback), so errors have to be passed as arguments (whether error_code or exception_ptr.) The above works if the return_type has a reasonable "unengaged" value that can be passed as the first argument in case of an error. While this is true for Boost.Asio, where return_type is a std::size_t, it is not necessarily true in general. A much better solution is to pass an outcome<T>, result<T>, or expected<T>: void async_operation(arg_type, void(*)(result<result_type>)); Another motivating use case is passing results between threads. For instance, in the Actor model of concurrency information is passed between actors/threads via a message queue. Some of these messages are replies to requests that may or may not have failed. A queue containing one of the Outcome types is well-suited for this kind of interaction. Apart from the above-mentioned shortcomings, I found the remainder of the documentation quite readable. I did not find the inheritance diagrams in the reference documentation useful though. > - What is your evaluation of the potential usefulness of the library? The idea of an either-value-or-error return type is very useful. > - Do you think the library should be accepted as a Boost library? No, not in its current form. I have two reasons for this rejection. First, some of the design discussions (especially on narrow/wide contracts) are not settled yet, so it would be premature to vote for the design. Second, whilst the lastest offer from the author about a fully-fledged Boost.Outcome repository without a dependency on the non-reviewed library called "boost-lite" is a step in the right direction, it is unclear to me how that repository will end up. Having been surprised by the current submission and the author's opinions, I will defer any vote of acceptance until the a repository is submitted for review.

2 2

[outcome] expected, etc. why are these assignable
by Robert Ramey 03 Jun '17

03 Jun '17

It just occurred to me that for a similar component I created some time ago for my own purposes, I (intentionally?) support of the assignment operator. Copy construction was permitted however. So the only way to use the component is: checked_result<int> = function(...); something void function(checked_result & cr, ....) will be the same as void function(const checked_result & cr, ...) because checked_result only contains const functions. That is, once constructed, it's immutable. Personally I never use an interface which passes a mutable type to a function. To me it violates the mathematical idea of what a function is. Doesn't this simplify things? Doesn't this eliminate the whole questions about about empty state, etc. Doesn't this better model the "mondad" concept we're trying to leverage on. And why the hell are we modifying and an object designed to model a result in the first place? It doesn't make logical sense in terms of we expect a "result" to be. I plunk down my money (a function) and ask for a pepperoni pizza. Either they say they don't have pepperoni and give me my money back or give me a pizza. I don't think of modifying this result - it's an artifact of history. And wouldn't making these things immutable, eliminate a large part of the whole controversy? Just a thought. I think sometimes something is hard because we haven't thought enough about what we're really trying to do - what problem we're trying to solve. Robert Ramey

6 16

[outcome] To variant, or not to variant?
by Gavin Lambert 03 Jun '17

03 Jun '17

I probably should have thought of asking this earlier, but it occurs to me now that my own mental model of how an "outcome"-ish type should act is probably not suited to variant storage at all. So just out of curiosity I thought I'd ask whether people prefer this sort of interface: template<typename T> class outcome { public: bool has_value() const { return m_storage.has<T>(); } T& value() { return m_storage.get<T>(); } bool has_error() const { return m_storage.has<error_code>(); } error_code& error() { return m_storage.get<error_code>(); } bool has_exception() const { return m_storage.has<exception_ptr>(); } exception_ptr& exception() { return m_storage.get<exception_ptr>(); } void set(none_t) { m_storage = none; } void set(const T& val) { m_storage = val; } void set(error_code err) { m_storage = err; } void set(exception_ptr ep) { m_storage = ep; } private: variant<none_t, T, error_code, exception_ptr> m_storage; }; Or this sort of interface: template<typename T> class outcome { public: bool has_value() const { return !!m_value; } T& value() { ensure(); return m_value.value(); } //bool has_error() const { return !!m_error; } const error_code& error() const { return m_error; } //bool has_exception() const { return !!m_exception; } const exception_ptr& exception() const { return m_exception; } void ensure() const { if (m_exception) { rethrow_exception(m_exception); } if (m_error) { throw system_error(m_error); } } void set(none_t) { m_value = none; m_error = error_code(); m_exception = nullptr; } void set(const T& val) { m_value = val; m_error = error_code(); m_exception = nullptr; } void set(error_code err) { m_value = none; m_error = err; m_exception = nullptr; } void set(exception_ptr ep) { m_value = none; m_error = ep ? error_code(errc::has_exception) : error_code(); m_exception = ep; } private: optional<T> m_value; error_code m_error; exception_ptr m_exception; }; (Don't get too hung up on the specifics. This is a sketch, not a real implementation. I've obviously omitted things that a real implementation would need such as more const methods and move support, and construction and assignment rather than using set methods. The focus is on variant vs. non-variant.) Anyway, the point is that this could actually transport multiple things; in particular as above both an error_code and an exception_ptr; perhaps set(error_code) could construct an exception as well, although I've explained elsewhere why I don't like that option. Perhaps this could even be exposed even more so that user code could explicitly provide both a value and an error -- think "here's a value, but it was truncated" or "here's the 5 values you asked for, but there's more", both fairly common in system APIs. Or for things like ambiguous matches where you still return the most likely candidate. Or dictionary insertion (duplicate key, but here's the value that's already there). Or many more such examples. (Though obviously in this case value() couldn't call ensure(). But that should make some people happy.) Also note has_error() and has_exception() don't even need to be provided in this version since you can always call error() and exception() in a boolean context anyway, with the same result. (Or actually a slightly better result than the implementation shown here.) The obvious downside is that it now uses more storage, which might hinder some inlining cases.

9 63

[sort] Timsort review
by Steven Ross 03 Jun '17

03 Jun '17

The review for C++ Timsort in Boost will commence tomorrow, June 3, and continue through June 12. I am the review manager. The code can be found here: https://github.com/boostorg/sort/pull/12 To merge it into a local copy of the Boost.Sort library: git checkout -b ZaMaZaN4iK-feature_branch/TimSort develop git pull https://github.com/ZaMaZaN4iK/sort.git \ feature_branch/TimSort If you want to see a proposed version of the Sort library source with Francisco's changes and Timsort included, download the zip file <https://drive.google.com/file/d/0B4s-GPKYVV0jOGx5ckJUTkZzMWM/view?usp=shari…>. Feel free to compare flat_stable_sort with Timsort. Please answer these questions in reply to this thread: 1. What is your evaluation of the Timsort implementation? 2. What is your evaluation of the comments? 3. Did you try to use the library? With what compiler? Did you have any problems? 4. How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? 5. How familiar are you with the details of hybrid and stable sorts? 6. Would you actually use Timsort if it was in Boost.Sort? If so, how is it better than the next best alternative? 7. Do you think Timsort should be included in Boost.Sort? Please ask Alexander Zaitsev any questions you have about Timsort for this review before submitting your final review. Thanks, Steven Ross

1 0

[poly_collection][VS 2017] Potential compiler bug
by Joaquin M López Muñoz 02 Jun '17

02 Jun '17

Hi, Boost.PolyCollection tests have started to cycle and I found VS 2017 aka MSVC 14.1 fails at test_algorithm.cpp: http://tinyurl.com/yb4zdkys whereas this test compiles and runs OK with VS 2015 aka MSVC 4.0 (_MSC_FULL_VER = 190023026). I've distilled down the problem to this test case: #include <algorithm> #include <utility> template<typename... Ts> struct all_of { template<typename... Args> auto operator()(Args&&... args)const-> decltype(std::all_of<Ts...>(std::forward<Args>(args)...)) { return std::all_of<Ts...>(std::forward<Args>(args)...); } }; int main() { int x; all_of<> alg; alg(&x,&x,[](int){return true;}); } which works fine with GCC and VS 2015 but fails in webcompiler.cloudapp.net with Compiled with /EHsc /nologo /W4 /c main.cpp main.cpp(9): error C3546: '...': there are no parameter packs available to expand main.cpp(18): note: see reference to class template instantiation 'all_of<>' being compiled (_MSC_FULL_VER = 191125331, which I think is VS 2017.3 or something). This looks like a regression to me. Deleting the ->decltype(...) bit makes the problem go away but I need to stay within C+11. * Can someone (STL?) confirm this is indeed a compiler bug? * Any rewording of the code that might be generally applicable to C++11 so that I don't have to include a specific workaround for VS 2017? Thank you, Joaquín M López Muñoz

2 1

[outcome] Outcome Review Period Closes Today (Fri-02-June)
by charleyb123 . 02 Jun '17

02 Jun '17

Hello Everyone, Today concludes the Outcome library review period (Fri-02-June-2017). Great thanks to all that invested so much time and effort to explore in such a disciplined and professional manner the dark corners of error/exception composition and a paradigm/API/implementation proposed by the Outcome library. Your efforts are truly appreciated. Reviews sent to the public list are the following (PLEASE contact me if I've missed your review): *- Paul Bristow (Tue-23-May) *- Daniz Bahadir (Wed-24-May) *- Robert Ramey (Wed-24-May) *- Peter Dimov (Sun-28-May) *- Emil Dotchevski (Sun-28-May) *- Vicente J. Botet Escriba (Tue-20-May) *- Andrzej Krzemienski (Wed-31-May) *- Gavin Lambert (Wed-31-May) My understanding is that as the review concludes today, further reviews are permitted only until the end-of-day UTC. In addition, many notable discussions and topics raised during the review period will also contribute to a final submission decision and summary report. Preparing this report will take some time. I've accumulated extensive notes, but my intention is to review (re-read) all discussion in the context of the "story-arc" and (in some cases) the shifted opinions of some persons during the course of the review (as topics were clarified and alternatives were explored). I assume other "satellite discussions" will continue (e.g., evolution of std::expected<>, suitability of wide vs. narrow contract in APIs, generalization of variant<>, etc.), which I will endeavor to continue to follow as the Outcome Review summary report is prepared (because these continued discussions may further clarify issues previously raised during the Outcome Review); but no more reviews can be accepted after the review period closes. Thank you all (again) for your (often thankless) contributions to the community, where the personal sacrifice in time and effort (and perhaps stressful emotional investment) is quite obviously extensive in many cases. --charley

2 2

[outcome] where is the monadic style tutorial?
by Mathias Gaunard 02 Jun '17

02 Jun '17

Hi, I just took a quick look at outcome today and I don't really understand what it is doing. I expected to have some kind of mechanism to manage control flow with propagation between states only happening if the previous expression in the chain resulted in an expected value, like with the Expected monad. I searched throughout the documentation and didn't find any. Why is there no example of this? More than how the error state is captured and stored or how to capture custom errors, I believe the programming model is what the tutorial should really focus on.

3 4

[outcome] Formal review
by Gavin Lambert 01 Jun '17

01 Jun '17

With the extended review period ending soon (or perhaps slightly overdue, depending on timezone), and given my many posts regarding the library, I feel like I should probably express my thoughts in review form. This is my first formal review and I am not myself a Boost library author (just a user) so please take that into account. - What is your evaluation of the design? A lot of thought has clearly gone into it, though I'm not sure I agree with some of the decisions made. Empty state I'm on the fence about; I can see it's convenient both for optional<T> success return semantics and for not-yet-populated/called pending result semantics, but I'm not sure these are consistently applied as it stands, and they seem at least partly opposed to the "empty state is weird, let's aggressively throw" behaviour in the actual implementation. Simplifying expected<T, E> into result<T> and outcome<T> makes sense, and the design of error_code_extended seems reasonable, including the content and ring buffer history. However I wonder if perhaps some middle ground might be better, where result<T, E> and outcome<T, E> exist but E is constrained to a type with specific characteristics (such as being constructible from error_code and having noexcept move). This might allow for more flexible error payloads. Alternately some way to use a similar error_code_extended-linked-to-ring-buffer system but with a user-defined payload data type. I don't consider this to be a defect or reason to reject the library, though, just something that would be useful if possible. I don't like that result<error_code{,_extended}> is not legal, as this seems like unnecessary asymmetry, though again I don't consider this a rejection reason. - What is your evaluation of the implementation? I have not looked in depth at it. The dual licensing and dual source distribution (preprocessed vs. not) bother me, but that will apparently be addressed in future. I don't like the total duplication of interface methods (get vs. value, error vs. get_error), but this will apparently also be addressed in future. (I am not opposed to having both narrow and wide accessors in the same type, though I prefer that the ones that can cause UB if abused require more typing. I *am* opposed to having multiple names for the exact same behaviour.) I'm not sure why it would be useful to provide const&& overloads of some of the value methods. Usually the only time I've seen these is to explicitly delete that overload. (Unless that is what's actually happening and doxygen is just not showing that correctly.) I like the use of wide contracts by default, such that it is safe to call value(), error(), or exception() without inspecting the underlying type and they will react sensibly (eg. throwing) rather than having UB, although some of the finer points of what a sensible reaction means are still being debated. But I can understand the desire of some people to have the other versions available for specific cases or in environments where static checking is more common. Even so, I hope that the narrow versions will at least BOOST_ASSERT their preconditions. The lack of use of Boost facilities such as Config, Assert, and Core (esp. BOOST_NO_EXCEPTIONS) bothers me more, and I do consider that a rejection reason. Use of StackTrace would be good as well but at this point I don't consider that a defect since it's not quite "in" Boost yet. (Possibly also Exception for boost::throw_exception, though using std::exception_ptr is the correct choice.) There is a critical defect in error_code_extended's backtrace API, which has already been brought to the author's attention and should hopefully be resolved later. - What is your evaluation of the documentation? Generally good, although it seems to spend a lot of time and pages talking about expected<T, E>, and only towards the end adds "but you should probably use outcome<T> instead" as a sort of afterthought, which seems a bit backwards. This is perhaps the correct order in a presentation, but it feels wrong in library documentation somehow. I think it needs to be more up-front about outcome and result (since those are the core types of the library) and recommended usage, and cover the other things as background material afterwards. The documentation refers to "monads" a lot, which is an unfamiliar concept to me. Not sure if this needs more explanation or just rewording. Doxygen's habit of putting hyperlinks that go nowhere in the reference pages is a bit disconcerting as well. - What is your evaluation of the potential usefulness of the library? Direct usefulness to me right now will be limited due to compiler support, but this seems like the sort of thing that would be quite useful in the future, especially to tidy up dual APIs such as those in ASIO. They have a fairly good chance of becoming standard vocabulary types, which of course has also helped drive a lot of the discussion thus far. - Did you try to use the library? With what compiler? Did you have any problems? I have not, as the compilers I use day-to-day are only C++11 or less, not C++14. - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? Primarily a fairly detailed reading of the documentation, combined with discussion here. - Are you knowledgeable about the problem domain? Mixed. I have experience with exception handling internals in MSVC and regularly work with software in both contexts of using error codes everywhere with exceptions banned, and of allowing exceptions but preferring they never actually be thrown. I also regularly work with both low-latency and fixed-latency environments, in addition to more "normal" software. But other than interaction with ASIO I have not made much use of error_codes to date, or mixing the two environments in the "sea of noexcept" fashion, though I can see the appeal. - Do you think the library should be accepted as a Boost library? Eventually, yes, but currently no (rejection, with encouragement to resubmit). While the author claims that the actual implementation has changed little since initial submission, the changelog in the "review accepted feedback" threads sounds fairly serious, and I'm not sure I like some of its directions (such as separate types for contract variations). Perhaps it will indeed end up being no big deal, but I don't feel like I have a good handle on what the end result will look or behave like so I don't feel comfortable endorsing it until I can get a better look at it. Primarily though I think the Boost integration was done backwards; the library should make use of Boost facilities first (as previously mentioned); if also distributed separately then that separate distribution should have the Boost emulation layer in it, and not the reverse. (To clarify: using std:: facilities is preferred, so using <system_error> is better than using <boost/system/system_error.hpp>, although it would be nice if compatible with the latter without actually depending on it unless requested explicitly. But the main problem is lack of use of Config, Assert, and Core, unless I missed something.)

2 2

[units] Compilation fails on quantity copy-construction
by Pavel Kretov 31 May '17

31 May '17

Hi all! I'm trying to make use of Boost.Units in a simple FDTD simulation program, but found a strange behavior of some kind. Consider the following demonstration example where I try to calculate time step from spatial steps according to the CFL condition [1]: #include <iostream> #include <boost/units/physical_dimensions.hpp> #include <boost/units/systems/si.hpp> #include <boost/units/cmath.hpp> #include <boost/units/io.hpp> int main() { namespace bu = boost::units; namespace si = boost::units::si; using dimless = bu::quantity<si::dimensionless, float>; bu::quantity<si::length, float> dx = 1.0f * si::meter, dy = 1.0f * si::meter; bu::quantity<si::velocity, float> v = 1.0f * si::meter_per_second; bu::quantity<si::time, float> dt = 0.5f / (v * bu::sqrt(1.0f/(dx*dx) + 1.0f/(dy*dy))); std::cout << dt << std::endl; } The code looks okay to me, but when I try to compile it, I get a scary error message: main.cpp:20:19: error: no match for ‘operator/’ (operand types are ‘float’ and ‘<THE_TERROR_THAT_FLAPS_IN_THE_NIGHT>’) dt = 0.5f / (v * bu::sqrt(1.0f/(dx*dx) + 1.0f/(dy*dy))); ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ where the second operand type looks like boost::units::multiply_typeof_helper< boost::units::quantity< boost::units::unit< [{length_base_dimension, static_rational<1l>}, {time_base_dimension, static_rational<-1l, 1l>}], boost::units::si::system, float>, boost::units::quantity< boost::units::unit< [{length_base_dimension, static_rational<-1l, 1l>}], boost::units::si::system, double>>::type In the above expression [{}, {}, ...] is a pseudo-code for static head-tail list of boost::units::dim<Unit, Rat> pairs. Note the 'double' type materialized from barely nowhere: the code explicitly defines everything as 'float', it's the 'bu::sqrt' to blame for it to appear. Note that replacing 'bu::sqrt' with 'bu::pow<1, 2>()' does not help. As I didn't feel happy with the above error message, I mutated the code a bit and made it to work with: bu::quantity<si::time, float> dt; dt = dimless(0.5f) / (v * bu::sqrt(1.0f/(dx*dx) + 1.0f/(dy*dy))); I had to both (a) add 'dimless', and (b) separate variable declaration and definition. Doing only (a) didn't help. Mkay, but what if I need to have a const quantity? (I'm a big fan of making everything 'const'!). The only one thing I could do was to roll out my own 'sqrt' and 'pow' functions. Here they are: #include <boost/units/get_system.hpp> #include <boost/units/get_dimension.hpp> #include <boost/units/static_rational.hpp> #include <boost/units/quantity.hpp> #include <boost/units/dimension.hpp> template<typename Unit, int N, int M = 1> using static_power_helper = boost::units::unit< typename boost::units::static_power< typename boost::units::get_dimension<Unit>::type, typename boost::units::static_rational<N, M>::type>::type, typename boost::units::get_system<Unit>::type>; template<int N, typename Unit, typename Y> auto ratpow(boost::units::quantity<Unit, Y> const &x) -> boost::units::quantity<static_power_helper<Unit, N>, Y> { using std::pow; return decltype(ratpow<N>(x))::from_value(pow(x.value(), N)); } template<int N, int M, typename Unit, typename Y> auto ratpow(boost::units::quantity<Unit, Y> const &x) -> boost::units::quantity<static_power_helper<Unit, N, M>, Y> { using std::pow; return decltype(ratpow<N, M>(x))::from_value(pow(x.value(), (Y) N / M)); } template<typename Unit, typename Y> auto msqrt(boost::units::quantity<Unit, Y> const &x) -> decltype(ratpow<1, 2>(x)) { return ratpow<1, 2>(x); } Using them I was able to compile the original form of the expression (as seen in the very first listing): bu::quantity<si::time, float> const dt = 0.5f / (v * msqrt(1.0f/(dx*dx) + 1.0f/(dy*dy))); The questions: 1. Is it possible to work around the problem without custom 'sqrt'? 2. Is the described behavior intentional, -or- Is there an error in Boost.Units' implementation of 'sqrt'? 3. If the 'boost::unit::sqrt' is fine, is there something wrong with my implementation? ――― Pavel K. [1]: https://en.wikipedia.org/wiki/Courant-Friedrichs-Lewy_condition

2 1

[poly_collection] Questions about merging into Boost
by Joaquin M López Muñoz 31 May '17

31 May '17

Boost.PolyCollection was recently accepted into Boost and I'm planning to integrate it into https://github.com/boostorg/ asap to begin test cycling. I am not sure if the lib will be in a good enough shape for launching with Boost 1.65, though. Questions: 1. The current repository is https://github.com/joaquintides/poly_collection , which has branches master (code) and website (for the online docs). Is the following the right procedure for integration with Boost? a. Rename master to develop (https://gist.github.com/lttlrck/9628955 ) b. Transfer repo to boostorg (https://help.github.com/articles/about-repository-transfers/ ) c. Ask for write permission to github.com/boostorg/poly_collection 2. Can the repo not have a master branch until I decide the lib is ready for launch? Will this result in poly_collection simply not being in Boost releases while still develop tests are cycling? Thank you, Joaquín M López Muñoz

4 4

[outcome] Andrzej's review
by Andrzej Krzemienski 31 May '17

31 May '17

Hi Everyone, This is my review of Boost.Outcome library. First, I want to thank Niall for taking the effort to write the library, promoting it, sharing with the community, and submitting for Boost review. I think that the library has already proven useful regardless of this review's outcome (or should I say 'result'). I recommend to REJECT the library at this point, and encourage the author to re-submit it after the list of what I consider issues -- described below -- has been addressed. Why reject and not conditionally accept? It is possible to say that I accept the library if this "delta" is applied to the current submission's state. Niall has indicated a couple of times that he is willing to apply even big changes, if this is what reviewers wish. But the amount of change that would be required, plus the uncertainty what the "delta" actually is, plus a general confusion as to what library is for, as reported by others (the effect of the current shape of documentation), in total make the conditional acceptance too fragile. What are we reviewing? It would be fair to assume that we are reviewing the master branch, SHA 4851926c1c601c0e9391b2d0d07fb17f634b3ecb, but there is also a "delta" described by Niall in high-level in one of the messages: http://boost.2283326.n4.nabble.com/outcome-High-level-summar y-of-review-feedback-accepted-so-far-td4694767.html. So maybe the master + that delta. It describes that we will have like 10 types, like outcome_e, checked_outcome_e, etc.. Now in another thread I hear there will only be 5 types: optional_outcome and outcome. This makes me believe that the compromise solutions and promises are being made in a hurry, and this will likely result in (1) suboptimal design and (2) misunderstandings as to what we are actually agreeing to. Am I knowledgeable about the problem domain? If the domain is "reporting function failures in predictable-latency environments", then no, I have no experience with this domain. I also have no experience with `expected`, although having worked on formal specification of `std::optional` I am familiar with a number of issues that `expected` also needs to solve. Also, a number of controversies revolves around git sub-modules, dual licensing and Boost distribution process. I do not feel competent in this area either. How much effort did I put into the evaluation? I have read a number of versions of the documentation. Had a long email exchange with Niall in order to understand what the library does. I had a glimpse at the source code to figure out the pieces that were missing from documentation. I have tried running my own test examples on GCC 6.3.0 with MinGW w64 5.0.0 (doesn't work) and GCC 6.3.1 on Fedora (works fine). Is the library useful? Yes. Apparently it solves an important problem in predictable-latency environments. This is confirmed by the user base the library already has managed to acquire. It provides an implementation of the to-be standardized `exceptional`, which allows for experimentation. I believe Boost would benefit from including this library (after the issues have been resolved). I am confident Niall has knowledge about problem domain. Documentation. It has two serious deficiencies. It lacks an introductory part that would clearly and shortly describe what problem the library is solving an how, how can I use it and when should I use it. It is also missing a reference with documented semantics. One cannot tell what outocme's default constructor does, or what function error() does when you have an exception_ptr inside. I believe Niall when he says his users do not need this documentation: they immediately grasp what and how it is doing. But of the Boost library I expect a high-quality documentation. I think that these deficiencies in the documentation prevent a successful review of the library. I bet there are people who were put off by the docs and will not proceed to reviewing the library further, even though they could have given you many valuable insights, or point out defects. Some will stop at criticizing the docs. Any potential flaws in the library will not even be revealed. This may be the reason why things like ring buffer were not opposed: the potential critics may have not gotten that far. This is one of the reasons that I would recommend fixing the docs and staring the review again without the distraction. On the other hand, there is a lot of useful material in the documentation. For sure it is a way better that the original versions. For instance, the usage of ring buffer is documented clearly. Design. The idea to represent either a returned value or a reason for the lack thereof as `variant<T, error_code, exception_ptr>` is sound. Fixing the `E` to a single type is sound. using_error_code extended (along with ring buffer) for carrying payload is sound. I also like the fact that you only have three "monads": `outcome`, `result` and `expected`, and that they are maximally different: expected and result make the opposite decision on practically any aspect. This is a reasonable compromise between many possible combinations of micro-decisions, and the expectation to have a small number of types. `option` should go, I think. I can think of no use case for it. However, the design still changes throughout the review, and has not stabilized yet. The last message is that we may have varieties for narrow versus wide contract or we will have duplicated observers, and this has not been settled on yet. The consequence of using the ring buffer, where the error message can suddenly disappear is unfortunate, but then again, I know of no way to make it better. It might be the best choice of those available. I wish someone from low-latency users stepped in and commented on whether this is the right choice and whether the choice of these particular members (a short string, two unsigned ints) is the optimum in practice. Some aspects of design, to me seem unclear. E.g., what does it mean that `outcome` has value but I am calling `o.error()`? Is it a programmer error, which the function is trying to conceal, or does `o.error()` mean "convert `outcome` to `error_code`, whatever the state? Similarly, when `exception()` also recognizes error_codes, and `error()` does not recognize exception_ptrs looks quite random: I cannot see a mental model behind it. You could document detailed semantics of each function, but this would not be enough. I need a clear mental model, like "every error_code or exception_ptr is collectively called 'exception' in this library". If you do not support `outcome<T&>, I need a clear indication of it and a rationale. Implementation. These dual licensing, testing and git sub-module issues. I am not an expert in how Boost is organized, but a number of people I consider qualified have reported strong objections, and this has to be considered. A Boost library must be compliant with "the way things work in Boost" even if it is felt that things work wrong in Boost. I did not look much, at the implementation, but it looks acceptable on the surface. The assignment (most tricky part, I think) is implemented quite clearly. Sometimes static_assert's constrain too much. A non-default-constructible types should be allowed. Types that throw upon copy/move construction should be allowed provided I do not call the assignment on outcome/expected. I am told `outcome<error_code>` does not work. I am not sure if this is fortunate. It is no longer an `outcome<T>` that works for any Regular T. What if I have a function that optains an error_code and might potentially fail. How can I express this in the interface? (Or do I have to wrap in `outcome<unique<error_code>>`?) `outcome` has function swap overloaded in namespace std. I am not sure if it is legal, but even if so, I would recommend implementing it in `outcome`'s namespace, in order not to mess with `std`. Again, thanks Niall for this library, and for a very educational discussion. Regards, &rzej;

2 1

[units] Compilation fails on quantity copy-construction
by Pavel Kretov 31 May '17

31 May '17

Hi all! I'm trying to make use of Boost.Units in a simple FDTD simulation program, but found a strange behavior of some kind. Consider the following demonstration example where I try to calculate time step from spatial steps according to the CFL condition [1]: #include <iostream> #include <boost/units/physical_dimensions.hpp> #include <boost/units/systems/si.hpp> #include <boost/units/cmath.hpp> #include <boost/units/io.hpp> int main() { namespace bu = boost::units; namespace si = boost::units::si; using dimless = bu::quantity<si::dimensionless, float>; bu::quantity<si::length, float> dx = 1.0f * si::meter, dy = 1.0f * si::meter; bu::quantity<si::velocity, float> v = 1.0f * si::meter_per_second; bu::quantity<si::time, float> dt = 0.5f / (v * bu::sqrt(1.0f/(dx*dx) + 1.0f/(dy*dy))); std::cout << dt << std::endl; } The code looks okay to me, but when I try to compile it, I get a scary error message: main.cpp:20:19: error: no match for ‘operator/’ (operand types are ‘float’ and ‘<THE_TERROR_THAT_FLAPS_IN_THE_NIGHT>’) dt = 0.5f / (v * bu::sqrt(1.0f/(dx*dx) + 1.0f/(dy*dy))); ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ where the second operand type looks like boost::units::multiply_typeof_helper< boost::units::quantity< boost::units::unit< [{length_base_dimension, static_rational<1l>}, {time_base_dimension, static_rational<-1l, 1l>}], boost::units::si::system, float>, boost::units::quantity< boost::units::unit< [{length_base_dimension, static_rational<-1l, 1l>}], boost::units::si::system, double>>::type In the above expression [{}, {}, ...] is a pseudo-code for static head-tail list of boost::units::dim<Unit, Rat> pairs. Note the 'double' type materialized from barely nowhere: the code explicitly defines everything as 'float', it's the 'bu::sqrt' to blame for it to appear. Note that replacing 'bu::sqrt' with 'bu::pow<1, 2>()' does not help. As I didn't feel happy with the above error message, I mutated the code a bit and made it to work with: bu::quantity<si::time, float> dt; dt = dimless(0.5f) / (v * bu::sqrt(1.0f/(dx*dx) + 1.0f/(dy*dy))); I had to both (a) add 'dimless', and (b) separate variable declaration and definition. Doing only (a) didn't help. Mkay, but what if I need to have a const quantity? (I'm a big fan of making everything 'const'!). The only one thing I could do was to roll out my own 'sqrt' and 'pow' functions. Here they are: #include <boost/units/get_system.hpp> #include <boost/units/get_dimension.hpp> #include <boost/units/static_rational.hpp> #include <boost/units/quantity.hpp> #include <boost/units/dimension.hpp> template<typename Unit, int N, int M = 1> using static_power_helper = boost::units::unit< typename boost::units::static_power< typename boost::units::get_dimension<Unit>::type, typename boost::units::static_rational<N, M>::type>::type, typename boost::units::get_system<Unit>::type>; template<int N, typename Unit, typename Y> auto ratpow(boost::units::quantity<Unit, Y> const &x) -> boost::units::quantity<static_power_helper<Unit, N>, Y> { using std::pow; return decltype(ratpow<N>(x))::from_value(pow(x.value(), N)); } template<int N, int M, typename Unit, typename Y> auto ratpow(boost::units::quantity<Unit, Y> const &x) -> boost::units::quantity<static_power_helper<Unit, N, M>, Y> { using std::pow; return decltype(ratpow<N, M>(x))::from_value(pow(x.value(), (Y) N / M)); } template<typename Unit, typename Y> auto msqrt(boost::units::quantity<Unit, Y> const &x) -> decltype(ratpow<1, 2>(x)) { return ratpow<1, 2>(x); } Using them I was able to compile the original form of the expression (as seen in the very first listing): bu::quantity<si::time, float> const dt = 0.5f / (v * msqrt(1.0f/(dx*dx) + 1.0f/(dy*dy))); The questions: 1. Is it possible to work around the problem without custom 'sqrt'? 2. Is the described behavior intentional, -or- Is there an error in Boost.Units' implementation of 'sqrt'? 3. If the 'boost::unit::sqrt' is fine, is there something wrong with my implementation? ――― Pavel K. [1]: https://en.wikipedia.org/wiki/Courant-Friedrichs-Lewy_condition

1 0

[config] C++17 macros wanted
by Tal Lancaster 31 May '17

31 May '17

I have been adding C++17 features to date_time. There are two config macros, I would like to have: Test for C++17’s constexpr inline feature. Maybe call it something like: BOOST_NO_CXX17_CONSTEXPR_INLINE Test for C++17’s structured binding feature. Maybe call it something like: BOOST_NO_CXX17_STRUCT_BIND

3 7

[outcome] How to drop the formal empty state
by Niall Douglas 31 May '17

31 May '17

There has been a fair bit of review feedback that people don't like the formal empty state in outcome<T> and result<T>. I still think this opinion daft, just because it's there doesn't mean you have to use it, but here are some options: 1. Like Expected, we could require the E types to provide a nothrow move constructor. This would allow us to guarantee no empty state, not ever. Default constructing an outcome<T> or result<T> would set a default constructed T state, just like Expected. The only remaining difference now between result<T> and expected<T> would be the wide vs narrow observer functions. outcome<T> still can also carry a std::exception_ptr. 2. We could do the above, but default constructor constructs to some constexpr undefined state that is not state T nor E. 3. We could only provide a guarantee of no empty state if the E types provide nothrow move construction, but if they do not we fall back to an empty state arising if throws occur during assignment. 4. The option for an empty state becomes a template parameter defaulted to false i.e. disabled. This can be combined with any of the above. Thoughts? Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/

6 47

[review] Review of Outcome (ends Fri-June-2)
by charleyb123 . 31 May '17

31 May '17

Reminder -- On Fri-26-May the Review Wizards approved extending the Outcome review until Fri-02-June. (Notification was sent to the list, but considering the high traffic volume lately, one can be forgiven for having missed that message.) Thank you to everyone for your tremendous efforts in taking part in the review; for keeping up with high message traffic; and for the professional and deep exploration of the many (very broad!) topics touched on during this review. The review continues for several more days, ending Fri-02-June (it will not be extended further). Please consider posting a review to the boost mailing list, 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? - What is your evaluation of the implementation? - What is your evaluation of the documentation? - What is your evaluation of the potential usefulness of the library? - Did you try to use the library? With what compiler? Did you have any problems? - How much effort did you put into your evaluation? A glance? A quick reading? In-depth study? - Are you knowledgeable about the problem domain? And most importantly: - Do you think the library should be accepted as a Boost library? 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

3 2

[gil] where is the mysterious boost::gil::read_image function?
by Frédéric Bron 31 May '17

31 May '17

I would like to read a jpeg image from a memory location, not from a file. According to this post http://boost.2283326.n4.nabble.com/gil-io-new-review-Reading-images-from-in… there is a boost::gil::read_image(input_stream, image, image_tag) function but I cannot find it in the header files. Thanks, Frédéric

1 0

[future.then] Should deferred continuation respect status of original future?
by Tomas Sturm 31 May '17

31 May '17

Hi, consider this code that wraps some library returning a future: // some async library boost::future<intermediate> DoLongAsyncTask() { return myPromise.get_future(); } // intermediate translation layer boost::future<result> GetResult() { return DoLongAsyncTask().then([] (boost::future<intermediate> f) { return TranslateToResult(f.get()); }); } // client event loop void RunInClientContext() { auto myFuture = GetResult(); while (stuffToDo()) { doStuff(); if (SafeToCallFutureGetWithoutBlocking(myFuture)) ProcessResult(myFuture.get()); } } What should the implementation of 'SafeToCallFutureGetWithoutBlocking' be? Normally, one would write something like: myFuture.is_ready() or myFuture.wait_for(chrono::seconds(0)) != future_status::timeout But this doesn't work in case the implementation of intermediate translation layer calls .then with launch::deferred! In Boost 1.63 the future returned from .then(launch::deferred, ...) is always deferred, so wait_for(...) always returns deferred no matter what (and is_ready() always returns false). In fact, there is no way to implement 'SafeToCallFutureGetWithoutBlocking' if the translation is to happen in the context of the client (launch::deferred). Wouldn't it be more natural to respect the status of the original future in this case? The documentation is already very clear that the continuation is executed only after the original future is ready. So in case a client asks whether the continuation future is ready, and the original future is not ready yet, the call should forward to the original future instead. What I propose is this: promise<int> p; future<int> f1 = p.get_future(); future<int> f2 = f1.then(launch::deferred, [] (auto f) { return f.get(); }); assert(f2.wait_for(0) == future_status::timeout); p.set_value(42); assert(f2.wait_for(0) == future_status::deferred); Is there any reason for the continuation future to not behave this way? Regards Tomas

2 1

[outcome] narrow-contract, wide-contract, and value_if
by Andrzej Krzemienski 31 May '17

31 May '17

Hi Everyone, Some comments about `outcome`'s observers with narrow and wide contracts. First, the wide-contract functions. We have a situation when someone assumes that `o` currently stores an error_code, and with this assumption in mind calls `o.error()`, but somehow his assumption is incorrect. What do we want to do? 1. Try to report programmer's bug at run-time? 2. Since in this case it is a bug anyway and are screwed, at least let's make such behavior useful for other situations where `error()` is called conciously on `o` without an error code? The first scenario attempts at identifying a bug, bringing it to attention of developers (maybe in form of a log), and minimizing its consequences by perhaps skipping the execution of some part of the program in hope that the bug will no show symptoms again. The second scenario in a way ignores the bug, and lets the program continue, as though it was correct, and from now on the program is doing something else than what the programmer intended. But the semantics of the function are useful in itself. Users who check `o.has_error()` before calling it can use it as if `error()` had a single purpose and a narrow contract; and users who know the additional semantics can use it for other useful things, like saving themselves of writing some boilerplate code. It is my impression that some of these observer functions in `outcome` follow this second scenario: just do something useful for people who use it unchecked. Saving boilerplate is useful, but let's state it clear: these functions offer no protection against inadvertent mistakes: no run-time or static protection. If I call `o.error()` and it throws when `o.is_empty()`, it is a form of protection. If `o.error()` returns `E{}` when `o.has_value()`, this is no protection, it is convinience function that can be used as a narrow-contract observer. Maybe the interface of `outcome` should make this distinction and offer two sets: one for observers with run-time protection, and the other for nice convenience functions? Second, narrow-contract functions. Narrow functions acheive the following goals: 1. Simple design: function only does one thing, it does not have any if inside. 2. The interface of the library makes a clear distinction of what is valid and what is an invalid usage of the library. It helps in code reviews. For instance, you are looking for a bug in a big code, you find the following piece: ``` auto e = o.error(); ``` and it turns out that `e` contains a value. Did you just find a bug, or was it the author's intention to get an `E{}` at this point? You do not know with wide contracts. With narrow contracts you know: it is a bug. 3. Narrow contracts assist instrumentation in test builds and runs. If I have function `just_value` with a narrow contract, I leave myself the freedom: I can do whatever I want when has_value() is false. What can I do with this freedom: A. In test build, I can check the precondition at run-time and launch the debugger B. I can (run-time) check the precondition, and std::termiante() or throw an exception. C. I can ignore the potential precondition violation, check nothing and aim at some performance gain. D. I can put some code that assists the static analyser (like __builtin_unreachable()) E. I can put some instrumentation code that assists an UB sanitizer (like __builtin_unreachable()) F. I can put hints for optimizer (like __builtin_unreachable(), again) G. Finally, I can put a macro `PRECONDITION(has_value())` which can do any of the above based on the build configuration. Given that, the proposed function `value_if` (which returns a pointer to value) will not satisfy my needs. It has a wide contract, so I cannot put macro `PRECONDITION()` inside. It rturns a pointer. The pointer's operator-> inded does have a narrow contract, but it gives me no room where I can put macro `PRECONDITION()`. So I get some narrow contract, but without the benefits I am looking for. So that is my view of the observer functions. Now my proposal is to provide a pair of observers for each value, error_code and exception -- narrow and wide: `value()` and `just_value()`, `error()` and `just_error()`, `exception()` and `just_exception()` Or maybe, even three for narrow-contract, wide-contract with run-time protection, and for convenience function: `error()` -- wide-contract, runtime-checked precondition `just_error()` -- narrow contract `as_error()` -- convenience function, e.g. returns E{}, on value, monad::has_exception on exception, etc.. Regards, &rzej;

2 2

[outcome] user's experience
by Andrzej Krzemienski 30 May '17

30 May '17

Hi Everyone, In order to test Boost.Outcome, I applied it to the problem I had to solve at work a while ago. For people who do not know what Boost.Outcome is for, this could be an introduction. For me it can be an opportunity to obtain feedback from Niall and Vicente, If I am using `expected` correctly, and they can get some feedback from me. Here is the task. I get an std::string as input: it is a data structure encoded in ASCII text: "1-10|11-20|21-99;1-10|10-50|51-99" Semicolons separate "distributions". Within distribution, vertical bars separate "ranges". Within ranges, dashes separarate integer numbers. There are some additional constrains. like ranges within distribution cannot overlap, no gaps between ranges are allowed, and a couple more. I need to validate if the input matches the required format, and build a corresponding data structure. But the vaidation cannot be just binary valid-invalied answer: I want to give user a description of what exactly is wrong with her input, as detailed as possible. So, I have function parse, that internally calls parse_distribution, which in turn calls parse_range. Each of them can return different parsing error that needs to be propagated up. My original implementation used "out" function parameters for signalling errors. Code was unnecessarily long and it was easy to forget to check results from functions, but I didn't want to use exceptions to indicate validation failures, because there is nothin exceptional about it. You expect input validation to fail -- quite often. Now, this is a more philosophic question: when to use (or not) exceptions. One citerion I sometimes use is, "do I want my debugger to automatically break if this function reports faiure?" So I didn't use exceptions for input validation, but the ugliness of the code forced me to go back to rewrite it with exceptions. The code was shorter, and "saer" in the sense that I could not forget ot pass the failure up the stack. Now with Boost.Outcome I have a third option. It is almost as concise as exceptions, but does not use exceptions. Here is my implementation: https://github.com/akrzemi1/__sandbox__/blob/master/outcome_practical_examp… Note a couple of things. I am creating a number of temporay strings under way, they all can throw bad_alloc. I am fine with that. I am not affraid of throwing an exception, it is only the validation failures that I want to report with `expected`. For other failures I want exceptions. This is input validation. It does not have to be super fast. In the public API of this file, I will not be exposing the information about parsing failues. `process` will consume the error information and just output something. So, the choice of `E` in `expected<T, E>` is internal to a file, and I can choose something specific to my use case. I provid my own type, which encodes different erroneous situations in an enum, and two ints for additional details. ``` struct BadInput { enum { no_distrib = 1, range_count_not_3, distrib_starts_with_no_1, distrib_ends_with_no_99, gap_between_ranges, overlap_between_ranges, inverted_range, no_2_numbers_in_range, not_a_number, } type; int val1; int val2; }; ``` It is quite similar to Outcome's error_code_extended. Now, I also wanted to have an std::string member inside, but `expected` forbids it: It says as per LWG requirements `E` in `expected<T, E>` must not kave a throwing copy constructor. Is that right? In my use case it wouldn't harm anyone: if copying of `expected` throws, entire function `process` is exited, with a bad_alloc, which is fine. No `expected` with partial state will be ever observed. Do we need this restriction? Also, at some point I will be returning an `expected<vector<T>, E>`, which makes me uneasy. Returning vectors wrapped in something: will I get runtime penalty for it? So, I naturally arrived at a pattern that Niall described in the other thread: ``` out::expected<vector<T>, E> parse (...) { out::expected<vector<T>, E> ans {out::value}; vector<T> & vec = ans.value(); // will if statement be ellided?} // fill vec return ans; } ``` (BTW, this constant out::value is not documented in the docs, unlike out::value_t.) It has been mentioned a number of times that an optimizing compiler can remove a redundant if-statement. I know that the if-statement inside `value()` is redundant, but in this particular case, will compiler know it to? Nobody has called `has_value()` before. Also, the following piece of code illustrates how concise the implementation of a single function can be: ``` // I use a shortet alias for macro BOOST_OUTCOME_TRYX. // It is not portable, though. It only works on Clang and GCC. #define TRY(ex) BOOST_OUTCOME_TRYX(ex) out::expected<Range, BadInput> parse_range (const std::string& input) { tokenizer numbers_s {input, sep_3}; auto it = numbers_s.begin(), itEnd = numbers_s.end(); int count = std::distance(it, itEnd); if (count != 2) return out::make_unexpected(BadInput{BadInput::no_2_numbers_in_range, count}); Range r {TRY(parse_int(*it++)), TRY(parse_int(*it++))}; if (r.lo > r.hi) return out::make_unexpected(BadInput{BadInput::inverted_range, r.lo, r.hi}); return r; } ``` It is almost as short as if I used exceptions, except for the TRY macro. Note in particular this line: ``` Range r {TRY(parse_int(*it++)), TRY(parse_int(*it++))}; ``` `parse_int()` returns `out::expected<int, BadInput>`.Now, `TRY(parse_int(...))` is an expression and means, "if parse_int() failed immedaitely return function `parse_range` with the same BadInput, otherwise use its `value()` to initialize `r`. What do you think. Am I using the library correctly? Regards, &rzej;

4 7

[outcome] Exception safety guarantees
by Andrzej Krzemienski 30 May '17

30 May '17

Hi All, This is in connection with Vicente's question: what exception safety should we expect of copy assignment of `expected`? Now, this is really funny because we are talking about exception safety in something that is used as a substitute for exceptions. That is, one of the use cases for `expected` is to be able to get rid of exception handling altogether (while still reporting failures). But this fundamental problem, "what state my object is in if this mutating operation fails" is not specific to exceptions. It is speciffic to operations that might fail; and how (and if) they signal failures is of secondary importance. So, let's talk about *failure safety* rather than 'exception safety'. First, let's consider the use case for `expected`, where we want to disable exception handling altogether. This means `T` or `E` cannot throw on any operation. But copy assignment can still fail, right? Or maybe in these domains you are only using trivially-copyable types as `T` and `E`. Or maybe in these domains you never have a need to copy-assign instances of `expected`? Is so, they provide a *no-fail* guarantee, and any implementation of `expected` will be good and offer no-fail guarantee also. Bus if some copying operation on T or E can fail, how is the failure reported? Through a return value? Output parameter? But whatever the answer, we are arriving at the "nested failure" problem: we are processing a (potential) failure report, and this processing fails. What should we do? report the new error condition and ignore the previous? This is very close to a double-exception during stack unwinding. In C++ it std::terminates, other languages ignore the original error, or build a combined error report. All these solutions not satisfactory, and maybe no satisfactory solution exists. I would like to hear an opinion from people who deal with `expected` in exception-disabled environments. On the other extreme, you fave my example with parsing input (which Vicente observed is not parsing, but matching): https://github.com/akrzemi1/__sandbox__/blob/master/outcome_practical_examp… In that case, If I get an exception anywhere (not only upon copying T or E) I want stack to be unwound so far, that all not `expected` objects will remain. So I only care about basic failure guarantee: just let me correctly destroy these objects. In the middle: you have the situation where you copy-assign an `expected` and a copy-constructor of assignment of T or E throws. But where did this exception come from, given that you are using `excepted` for signalling failures? Or are you signalling some failures with exceptions and some with `expected`? And if so, are exceptions not more panic-like? And in that case yu would like to abandon the processing of any `expected`? Anyway, the most difficulties stem from the case where you are storing an E in `expected` and you want to assign an `expected` storing T. You have to first destroy E, and then may not be able to construct a T. My solution to this would be to go to the advice from the first days of forming exception safety guarantees: provide basic guarantee by default, and strong guarantee only if it does not cost too much. We are used to STL containers providing strong assignment, but this is because they are pointers, and they can implement it for free. But does std::touple provide a strong guarantee? No. Do aggregate types provide stron guarantee? No. And can it result in inconsistent data? It can: ``` struct Man { std::string fist_name, last_name; }; Man m1 = {"April", "Jones"}; Man m2 = {"Theresa", "May"}; try { m2 = m1; } catch(...) { } ``` `m2` may end up being {"April", "May"}. And we are taught to write types like this. But if this hapens, the blame is on whoever allowed these objects to outlive the "stack unwinding bubble". So my view, as of today, is not to strive for a strong or even never-empty guarantee. Provide a conditional guarantee, if types T and E don't throw, you get no-fail guarantee. If they do, you only have a basic guarantee: you can destroy, assign to, or maybe call valueless_by_exception(). Nothing more. I think `std::variant` made the optimal choice. And people should code so that instances of `expected` (at least those with T or E throwing on copy/move) should not outlive the "stack unwinding bubbles". Also, note what vector::push_back does when T is non-opyable and its move constructor is potentially throwing: if T's move throws upon reallocation, the behavior is undefined. Regards, &rzej;

8 42

[outcome] Review of Outcome
by Peter Dimov 30 May '17

30 May '17

What follows is my review of the Outcome library. I. DESIGN --------- The general idea of an error handling framework is sound, extremely appealing, and I found it well explained in the documentation. I like the concept very much and agree with many of the design decisions taken throughout. Specifically, constraining the error type to be the fixed std::error_code (or equivalent) and providing a wide accessor interface. I don't think that option<T> should have been included at all. It duplicates optional<T> and has no good reason to exist other than to showcase that the underlying policy-based implementation can produce it. I don't consider expected<T, E> a particularly needed part of the library; the focus should be on result<T> and outcome<T>, the two classes that represent the error handling philosophy on which the library is built. As already stated, I consider the interface of result<T> and outcome<T> a bit too cluttered. Not everyone shares my design preferences for minimal but complete interfaces, so there's legitimate argument to be had over whether, for instance, operator*, operator-> or value_or ought to be included, but the needless duplication between get() and value() is, well, needless. For an initial submission, it's better to err on the side of too little, rather than too much, because it's easy to add later, hard to remove. The empty state has been a source of controversy, and with reason. My initial stance was that it simply needs to go. This however presents a legitimate design question as to what the behavior of the default constructor ought to be. After seeing the suggestions of leaving the object uninitialized there, I now see the empty state as a significantly lesser evil. Having an empty state without the default constructor putting the object into it however is pointless. I do not agree with the direction things have taken of providing a multitude of result classes instead of a single one. Components that are intended to be used in interfaces should provide one, carefully designed option, which encourages what the designer deems best practices. The idea here, assuming that we shoot for standard acceptance at some point, is to provide std::result<T> and have APIs return it, not provide a bag of options and let everyone pick its own thing. (They already do that today and we're not better for it.) I do not agree with making value() unchecked by default. As I explained, one expression of the error handling philosophy behind the library is the following snippet: auto r = function().value(); where function() is noexcept and returns result<T>, whereas the place where the above line appears is a higher level component that signals errors not via error_code, but via exceptions. This one-liner provides the transition in an elegant way; losing it obscures the vision behind the library. Instead of using std::error_code, the library provides its own error_code_extended, complete with an interesting and very useful ring buffer logging mechanism that stores additional information along with the standard code/category values. Specifically, it returns error_code_extended from result<T>::error(). I would separate things a bit further here, attaching the additional information not to a separate error_code_extended class, but to result<> itself. That is, result<>::error() would return std::error_code, and I'd provide a separate accessor, result<>::error_info(), say, that would return the ring buffer entry associated with this result<>. This for me provides a cleaner separation between the basic result<> functionality, and the extended one, although this is a matter of taste. I would also support chaining result<>s by storing the index of the parent ring buffer entry in the current ring buffer entry. This is a straightforward and very useful extension. Instead of providing a macro BOOST_OUTCOME_CATCH_EXCEPTION_TO_RESULT, I would provide result<T>::set_error_from_exception(std::exception const& e); and outcome<T>::set_error_from_exception(std::exception const& e, std::exception_ptr const & p); that would do more or less what the macro does, except the fallback in the second case would be to store p instead of (EINVAL, e.what()). I'd also use make_error_result and make_value_result instead of make_errored_result and make_valued_result, as the former seem more grammatically correct. II. IMPLEMENTATION ------------------ The implementation, from a cursory look, appears of high quality, which is not surprising given that it was in use for two years. The preprocessed header is not easy to review, but the non-preprocessed form is available and amenable to study. Simple examples using outcome<T> compile and work well, although under VS2017 Clang/C2 windows.h is included, which is unfortunate but fixable. The library does not provide a test/Jamfile, which means that as submitted it will not integrate into Boost's testing infrastructure. Given that a Jamfile that performs the equivalent of withmsvc.bat is trivial to write: import testing ; project : requirements <include>../include/boost/outcome/v1.0 ; run unittests.cpp ; the omission is inexplicable. (Further integration into the Boost test matrix would ideally entail splitting unittests.cpp into separate .cpp tests so that success/failure can be tracked independently.) With the above Jamfile, trying to test the library with Cygwin gcc fails with: D:\tmp2\boost-master\libs\outcome\test>b2 toolset=gcc-cxx14 [...] gcc.compile.c++ ..\..\..\bin.v2\libs\outcome\test\unittests.test\gcc-cxx14-6.3.0\debug\unittests.o In file included from ../include/boost/outcome/outcome.hpp:26:0, from unittests.cpp:34: ../include/boost/outcome/outcome_v1.0.hpp:1446:22: fatal error: execinfo.h: No such file or directory #include <execinfo.h> ^ compilation terminated. I realize that Cygwin is not a supported platform, but this is what I have here and this is what I test with, so I figured I'll give it a try. Trying to test with VS 2017 fails with: D:\tmp2\boost-master\libs\outcome\test>b2 toolset=msvc-14.1 [...] compile-c-c++ ..\..\..\bin.v2\libs\outcome\test\unittests.test\msvc-14.1\debug\threading-multi\unittests.obj unittests.cpp unittests.cpp(780): error C3245: 'boost::outcome::_1_0_std_std_9274c0d4::basic_monad<boost::outcome::_1_0_std_std_9274c0d4::policy::monad_policy<udt>>::is_constructible': use of a variable template requires template argument list d:\tmp2\boost-master\libs\outcome\include\boost\outcome\outcome_v1.0.hpp(4271): note: see declaration of 'boost::outcome::_1_0_std_std_9274c0d4::basic_monad<boost::outcome::_1_0_std_std_9274c0d4::policy::monad_policy<udt>>::is_constructible' unittests.cpp(780): fatal error C1903: unable to recover from previous error(s); stopping compilation I get the same failure with /std:c++latest. Testing with the msvc-14.0 toolset works. The library as submitted contains both a preprocessed, fully self-contained, single header named outcome_v1_0.hpp, which is used by default, and a traditional non-preprocessed form that can be chosen with a macro. For the non-preprocessed form to be functional, a git submodule is incorporated by reference: [submodule "include/boost/outcome/boost-lite"] path = include/boost/outcome/boost-lite url = https://github.com/ned14/boost-lite.git branch = master fetchRecurseSubmodules = true ignore = untracked which in turn incorporates two more submodules: [submodule "include/gsl-lite"] path = include/gsl-lite url = https://github.com/martinmoene/gsl-lite.git branch = master fetchRecurseSubmodules = true ignore = untracked [submodule "pcpp"] path = pcpp url = https://github.com/ned14/pcpp.git branch = master fetchRecurseSubmodules = true ignore = untracked The first of these is not by the library author, and the second is a derived work. It is not possible for a Boost release, say, boost_1_65_0.zip, to incorporate git submodules by reference, so there remains a question of what, exactly, is being submitted for inclusion. My attempts to clarify this with the author have proven unsuccessful. I consider this a critical defect of the submission. It is simply not possible for a reviewer to give an opinion whether the submission ought to be included into Boost releases without it being known what, specifically, is being proposed to be included. There is no problem with the library containing implementation details under its directory, but we need to know what these details are. What the library actually needs as dependencies from boost-lite is not much and it's perfectly possible to extract the necessary parts and prepare a self-contained submission. This should have been done prior to the review. The files in the library contain the following: ``` Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License in the accompanying file Licence.txt or at http://www.apache.org/licenses/LICENSE-2.0 Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. Distributed under the Boost Software License, Version 1.0. (See accompanying file Licence.txt or copy at http://www.boost.org/LICENSE_1_0.txt) ``` This intends to dual license the library, but as written, it doesn't. What it does is apply the terms of BOTH licenses to distribution and use. This is also a critical defect. If the library is included in a Boost release, it should make it perfectly clear that it's licensed under the Boost license, and not under the intersection of Boost and Apache 2.0. There are several ways to do it, one is to clarify the above to specifically say "ASL2 or BSL, at your option" in the appropriate legalese, another is to license the Boost-included Outcome under BSL and the standalone Outcome under whatever. My recommendation would be to just license the lot under BSL and be done with it. Dual licensing creates unnecessary legal complications and may, from a legal point of view, be more burdensome than just using the BSL. Contributors need to be aware that they are licensing their patches under both licenses, it might not be very clear whether they have been aware, and so on. At times, the library duplicates certain functionality present elsewhere in Boost. I do not consider this a defect. It's a legitimate design decision to avoid intra-Boost dependencies, although the author needs to be aware of the downsides - using Boost components is basically taking advantage of free work instead of having to maintain everything oneself. III. DOCUMENTATION ------------------ I found the documentation perfectly adequate and was able to get a very good outline of how the classes were designed, how they were intended to be used, and what problem they intend to solve. It focuses a bit too much on expected<T, E>, which is not in itself a bad thing but is, for me, not exactly what the library is about. Fixing E to std::error_code is a fundamental (and correct) design decision from which these classes derive much of their utility. The Doxygen-generated reference will not win any awards, but it does the job. It however contains too many implementation details that would ideally either not be there at all, or tucked away in their own separate section. The monadic content is also a bit too high for my taste, and I would prefer if a user of result<> and outcome<> does not encounter the word monad at all. IV. VERDICT ----------- Should Outcome be accepted? In short, eventually. This, at present, is a NO. Ideally, what I would like to see is the library resubmitted in a conventional form, consisting of exactly the files that are intended to go into the libs/outcome directory in boost_1_65_0.zip, licensed under the BSL, only containing the files necessary for the non-preprocessed version of the code to compile and work. In addition, the test/ subdirectory would contain a Jamfile that is ready to integrate into the Boost test matrix.

4 14

[build] conditional run of unit-tests?
by Oliver Kowalke 30 May '17

30 May '17

In the unit tests I want conditionally run some unit-tests - depending on property <target-os>- On Windows systems the unit-test should apply/build with property <context- impl>winfib and on POSIX (non-Windows) <context-impl>ucontext. The Jamfile below doesn't work (Windows version build on POSIX) - xould I get a hint how to fix it (couldn't find a reference of rule 'run'). Oliver [ run test_callcc.cpp : : : <context-impl>fcontext [ requires cxx11_auto_declarations cxx11_constexpr cxx11_defaulted_functions cxx11_final cxx11_hdr_thread cxx11_hdr_tuple cxx11_lambdas cxx11_noexcept cxx11_nullptr cxx11_rvalue_references cxx11_template_aliases cxx11_thread_local cxx11_variadic_templates ] : test_callcc_fcontext ] [ run test_callcc.cpp : : : <context-impl>ucontext [ requires cxx11_auto_declarations cxx11_constexpr cxx11_defaulted_functions cxx11_final cxx11_hdr_thread cxx11_hdr_tuple cxx11_lambdas cxx11_noexcept cxx11_nullptr cxx11_rvalue_references cxx11_template_aliases cxx11_thread_local cxx11_variadic_templates ] : test_callcc_ucontext ] [ run test_callcc.cpp : : : <context-impl>winfib [ requires cxx11_auto_declarations cxx11_constexpr cxx11_defaulted_functions cxx11_final cxx11_hdr_thread cxx11_hdr_tuple cxx11_lambdas cxx11_noexcept cxx11_nullptr cxx11_rvalue_references cxx11_template_aliases cxx11_thread_local cxx11_variadic_templates ] : test_callcc_winfib : <target-os>windows ] ;

2 2

Re: [boost] [outcome] Possible extensions/changes to std::experimental::expected
by Vicente J. Botet Escriba 30 May '17

30 May '17

Hi, I believe that this private mail I sent to Niall could concern the Boost community and the Outcome review. While the post concerns the proposed std::experimental::expected, this is applicable as well to Outcome. I've added a more points at the end and some additional information about possible open points for std expected. Le 24/05/2017 à 01:04, Vicente J. Botet Escriba a écrit : > > Hi, > > > After better understanding of the goals of Boost.Outcome I would like > to share two possible extension of std::experimental::expected > > > 1. Implicit conversion when the errors are convertible and the type is > the same. > > I believe this is a missing constructor. > > EXPLICIT expected<T, E>::expected(expected<T, G>); // E is > convertible from G > > I see that the other constructor is also useful > > EXPLICIT expected<T, E>::expected(expected<U, E>); // T is > convertible from U > > and so > > EXPLICIT expected<T, E>::expected(expected<U, G>); // T is > convertible from U and E is convertible from G > > We could as well have an adapt function that transforms expected error explicitly to do error propagation adapt : (E->G) x expected<T,E> -> expected<T,G> (E->G) stands for a function that takes an E and return a G. > 2. variadic expected<Y, E1, ..., En> > > I suggested this already in revision 2 of the Expected standard proposal. > > expected<Y, E1, ..., En> should be something like variant<Y, E1, ..., > En> but with some syntactic sugar. > > > expected<T, E1, ...., En> could be EXPLICIT convertible from > expected<T, G1, ...., Gm> as far as for all Gj we find a unique Ek > that is convertible to. > We can refine the subsumption relation this could take too long to the compiler (as the previous relation has quadratic complexity as Andrzej reported to me). > > I believe this will cover exactly what outcome covers and in addition > it will let open the error types used by the user. > > It acts as a exception specification, which we have abandoned in C++11. > > expected<T, E1, ...., En> f(); > > T f() throw (E1, ...., En); > > > We can consider that this is bad, as we have abandoned it. > > Anyway, I think that expected<T, E1, ...., En> is a good > generalization of the proposed outcome<T> where E1 could be none_t, E2 > could be error_code and E3 is exception_ptr. > > At the end users could use variant<E, E1, ..., En> are return code, so maybe we could to make their life easier. > > In this way we could have > > template <class T> > > using option = expected<T, nullopt_t>; > > Note that this is different to std::optional as the default is T not > nullopt_t :( > I'm not proposing it. Just to say that this could be a possibility for option. > > template <class T> > > using result = expected<T, error_code_extended>; // and possibly none. > > template <class T> > > using outcome = expected<T, none_t, error_code_extended, exception_ptr>; > > > I believe we would need the syntactic sugar as expected<T, > variant<E1,... En>> should have a lot of user code noise. > In addition it will be less efficient than variant<T, E1,... En>. > > E.g. expected<T, variant<E1,... En>> should be convertible from > unexpected_type<G> as soon a one of the Ek is convertible from G. > > I believe variant<E1,... En> is not convertible from variant<G1,... > Gm> under the conditions described above, but maybe this should be the > case. > Andrzej told to me that this is already the case for optional<T>, but I've not found yet on which paper this was added? > > Of course, the never empty warranties could be supported only if the > types Ek allow it and we need to change the standard to ensure it for > exception_ptr. > > > I will not have time to implement a POC for 2, but I believe I could > do it for 1 before Toronto. Anyway I believe it will be worth > discussing these 2 extensions. > > > Best, > > Vicente > > > P.S. EXPLICIT above has the sense given in the standard. > 3. uninitialized default constructed expected<T,E> I know this could be conflictual, but there are a some advantages to doing it. We don't spend time initializing with something we will assign later. This state corresponds to the moved-from state. Initializing it by default to T() is not better initializing to E(). Is expected<Date, error_code> default constructible even if Date is not? Currently it is not the case. There are of course liabilities. We have now the possibility for uninitialized variables, but static analysis tools will help here. 4. About comparisons Outcome doesn't implement comparisons between Outcomes. He pretend that we don't need them. In addition the mix of comparisons and implicit conversion give us some surprising cases as described in Andrzej blog. [1] A gotcha with Optional - https://akrzemi1.wordpress.com/2014/12/02/a-gotcha-with-optional We have that std::future is not comparable (as exception_ptr is not comparable). Instead of comparison we could specialize std::less<> if we want to use them in ordered containers. What others think? 5. About implicit conversion from T The implicit conversion from T to expected<T,E> is a consequence of wanting that expected<T,E> should behave like a T. But this is not right. A expected<T,E> is not a T. We have in some way that expected<T,E> is explicitly constructible from E bu the use of make_unexpected(e). If we had only and explicit constructor from T the code will be much more uniform. Either you build it with a modified make_expected or with make_unexpected. In my opinion the implicit conversion from T to expected<T> is a mistake. 5. About the exception to throw std::experimental::expected throw bad_expected_access<E>. I adopted the design of optional and bad_optional_access We could have as well a bad_expected_access_base class as Outcome has. My question is why don't throw directly E? Some are requesting a way to get a specific exception from E, but either there is one exception that works for all and we can set it using some kind of trait or we need to add a trait parameter to expected to the the mapping :( Do we really want this to be configurable? 6. Implicit conversion from E outcome::expected<T,E> outcome::expected<T,E> is implicitly convertible from E when there is no risk of ambiguity. std::expected<T,E> is implicitly convertible from unexpected<E>. What people thing we should have? In Boost? in the standard? Vicente

5 21

[build] [config] check_target_builds and feature.subfeature composition
by Edward Diener 30 May '17

30 May '17

The Boost library config uses the check_target_builds rule to provide build time feature testing. Other Boost libraries use config checks in their own build and test jamfiles. I have found that this works reliably when a particular toolset is used in a normal manner. However when using the feature.subfeature and feature.compose rules to extend the toolset-version designation with a subfeature, such that the toolset designation now becomes toolset-version-subfeature, the use of the check_target_builds rule does not work as expected. In particular the individual check_target_builds rule results are put into a subdirectory based on the toolset-version and not based on the toolset-version-subfeature when toolset subfeature composition is used. I also suspect that the cached value reflects setup. This causes wrong results in tests where the toolset designation uses subfeatures, and a library's jamfile is using config and/or predef checks. As an example My toolset designation looks like: using gcc : 6.3 : some_path ; feature.subfeature toolset gcc : cxxstd : c03 c11 c14 c17 : optional composite propagated ; feature.compose <toolset-gcc:cxxstd>c03 : <cxxflags>-std=c++03 ; feature.compose <toolset-gcc:cxxstd>c11 : <cxxflags>-std=c++11 ; feature.compose <toolset-gcc:cxxstd>c14 : <cxxflags>-std=c++14 ; feature.compose <toolset-gcc:cxxstd>c17 : <cxxflags>-std=c++1z ; A library's test looks like: import ../../config/checks/config : requires ; run some_test.cpp : : : [ requires cxx11_hdr_forward_list ] ; If I run some some_test with toolset=gcc-6.3-c03, the config result for checking if forward list is supported is put into a subdirectory reflecting the toolset-version while the test result will be put into a subdirectory reflecting the toolset-version-subfeature. Subsequently if I run some_test with toolset=gcc-6.3-c11 the config result is taken from the subdirectory reflect the toolset-version even though whether the forward list is supported or not depends very much on the subfeature I am designating as part of the toolset. This naturally causes incorrect results for the test. Is this a known problem ? Is it a problem with Boost Build or is it a problem with how config uses the check_target_builds rule ? I have run into this problem extensively in my testing once I started to use the feature.subfeature ability of Boost Build.

5 9