Re: [boost] [outcome] Possible extensions/changes to std::experimental::expected
It looks like part of the discussion accidentally went into a private conversation. So, here we are back. 2017-05-25 22:39 GMT+02:00 Vicente J. Botet Escriba < vicente.botet@wanadoo.fr>:
Le 25/05/2017 à 18:40, Andrzej Krzemienski a écrit :
2017-05-25 17:32 GMT+02:00 Vicente J. Botet Escriba < vicente.botet@wanadoo.fr>:
Le 25/05/2017 à 13:18, Andrzej Krzemienski via Boost a écrit :
2017-05-25 9:44 GMT+02:00 Vicente J. Botet Escriba via Boost < boost@lists.boost.org>:
We have now the possibility for uninitialized variables, but static analysis tools will help here.
So, more specifically, I understand that you propose the following:
- Default constructor works: no T or no E is construced (similar what outcome<> does)
I was not aware of this. Do you know that from this reference documentation?
constexpr outcome () noexcept(is_nothrow_default_constructible) Default constructor.
Well, I just assumed that since outcome<> has this empty state, it is exactly for the purpose of giving semantics to the default constructor. But now that you have asked, I looked at the docs. They say nothing about what happens in the default constructor. I tried to look in the code, but was confused with these policies. So, I just run a small test program, and yes: default constructor creates an empty state.
Niall, please could you add an issue in order to document default constructors (if it doesn't already exists).
This is already logged in https://github.com/ned14/boost.outcome/issues/26
Sorrty, I couldn't interpret it as you. But, if it is the case, yes, as outcome does.
- You can assing to and and destroy such an objetc (similar what outcome<> does)
I don't catch what you mean here. Are you referring to the conversion between expected with convertible value type and error types?
No, I am saying that the following should work: outcome<T> o; // default-constructed o = some_outcome<T>(); // this should be safe to execute
This works already with expected even if it doesn't defaults to empty.
Ok, and you expect it to continue to work.
- You will probably need to add an observer function that checks for
this singular state, like `is_singular()`. If not for anything else it would be used for assisting the static analysis tools. (again, similar what outcome<> does)
No. There will be no such observer. This is essential. There is no visible empty state. Or do you consider that chrono::duration should tell you if it was initialized or not? The user know if it is initialized or not, but there is no tool to check it. This could be against safe programming, but we are here in C++ and we should provide first the raw tools and then build on top of them when we want more.
- other observer functions (has_value(), value(), has_error(), error()) cause UB when `is_singular() == true. (this is the only difference from outcome<>)
Not the only one. outcome fixes its error types. I'm proposing an extension to the current expected so that it can take care of the Outcome use cases and needs.
Did I understand your intentions correctly?
Not completly. I don't want an observer that tells me, attention you have not initialized your variable, or you have moved from. This is essential.
Very daring. It would be easier to accept if you could show a tool that is capable of statically analying this kind of "used unintialized" bug for `expected`.
Static analysis to check if uninitialized variables are used (read) works very well already . I don't remember if it si with clang-tidy that I have seen this kind of warnings.
Ok. Now I am thinking, another thing you can do, is to internally store
something like varient
Andrzej Krzemienski wrote:
Instead, rewrite observer functions like this:
bool has_value() const { if (BOOST_UNLIKELY(_is_in_empty_state())) __builtin_unreachable();
Please no. This is horrible. Actually it's doubly horrible; merely horrible would be putting the above in value(). In an observer function... it's just evil.
2017-05-25 23:34 GMT+02:00 Peter Dimov via Boost
Andrzej Krzemienski wrote:
Instead, rewrite observer functions like this:
bool has_value() const { if (BOOST_UNLIKELY(_is_in_empty_state())) __builtin_unreachable();
Please no. This is horrible. Actually it's doubly horrible; merely horrible would be putting the above in value(). In an observer function... it's just evil.
Can you explain? My reasoning is that the above is superior to just leaving members with uninitialized vaues. This is a proposed implementation for the design where decision has been made to only allow to assign to and destroy the empty value. UB sanitizers can detect reachin __builtin_unreachable() and report an error. And what is the alternative? That if I forget to check for the empty state I get some unintended behavior? Regards, &rzej;
Andrzej Krzemienski wrote:
2017-05-25 23:34 GMT+02:00 Peter Dimov via Boost
: Andrzej Krzemienski wrote:
Instead, rewrite observer functions like this:
bool has_value() const { if (BOOST_UNLIKELY(_is_in_empty_state())) __builtin_unreachable();
Please no. This is horrible. Actually it's doubly horrible; merely horrible would be putting the above in value(). In an observer function... it's just evil.
Can you explain?
What is there to explain? This makes calling has_value undefined if empty ("narrow contract"), which makes the user responsible of checking empty() before doing ANYTHING with a result/outcome object. This applies to each and every result/outcome object you receive from somewhere. auto r = function(); if( r.empty() { /* always have to do this*/ } else if( r.has_value() ) { /* otherwise UB here */ } As I said, I don't like undefined behavior for its own sake, and I most certainly don't like putting a "Requires: not empty" contract on every function, including observers. Putting a contract is not even a good fit for value(), because it already checks and throws, to enable auto r = function().value(); where you go from noexcept land to exception land. If UB on empty, goodbye one liner, you still need to save the outcome and check for empty first, before retrieving value().
2017-05-25 23:57 GMT+02:00 Peter Dimov via Boost
Andrzej Krzemienski wrote:
2017-05-25 23:34 GMT+02:00 Peter Dimov via Boost
: Andrzej Krzemienski wrote:
Instead, rewrite observer functions like this:
bool has_value() const { if (BOOST_UNLIKELY(_is_in_empty_state())) __builtin_unreachable();
Please no. This is horrible. Actually it's doubly horrible; merely > horrible would be putting the above in value(). In an observer > function... it's just evil.
Can you explain?
What is there to explain? This makes calling has_value undefined if empty ("narrow contract"), which makes the user responsible of checking empty() before doing ANYTHING with a result/outcome object. This applies to each and every result/outcome object you receive from somewhere.
auto r = function(); if( r.empty() { /* always have to do this*/ } else if( r.has_value() ) { /* otherwise UB here */ }
As I said, I don't like undefined behavior for its own sake, and I most certainly don't like putting a "Requires: not empty" contract on every function, including observers.
Putting a contract is not even a good fit for value(), because it already checks and throws, to enable
auto r = function().value();
where you go from noexcept land to exception land. If UB on empty, goodbye one liner, you still need to save the outcome and check for empty first, before retrieving value().
Yes, it would be wrong if the message was, "whoever gets an `expected` object needs to check for the empty state". The ideal solution would be to put this responsibility on someone who produces the `expected` object. Also, they way I look at this solution is not "when I get this value I have to check ...", but "when I produce this value I have to make sure...". Is there no way to acheive this in the language? Regards, &rzej;
Andrzej Krzemienski wrote:
Also, they way I look at this solution is not "when I get this value I have to check ...", but "when I produce this value I have to make sure...". Is there no way to acheive this in the language?
By not providing a default constructor, I suppose. Otherwise not, there's return value elision so if you do result<T> function() { result<T> r; // do things return r; } nothing is ever called on 'r' on your side if you forget to initialize it. Putting "outcome_errc::not_initialized" inside r made sense, because the principle is that when the returned result has no value, it has to contain the reason for its failure to contain such, and here the reason is that whoever wrote the function left it uninitialized. This looks logical to me, apparently to others not so much. It's not really that much different from returning EINVAL in r because the programmer made some other kind of logic error - instead of forgetting to initialize r, he forgot to initialize some argument to some lower level function he called, so it returned EINVAL and that's what ended up in r.
2017-05-26 0:17 GMT+02:00 Peter Dimov via Boost
Andrzej Krzemienski wrote:
Also, they way I look at this solution is not "when I get this value I
have to check ...", but "when I produce this value I have to make sure...". Is there no way to acheive this in the language?
By not providing a default constructor, I suppose. Otherwise not, there's return value elision so if you do
result<T> function() { result<T> r;
// do things
return r; }
nothing is ever called on 'r' on your side if you forget to initialize it.
True. This is off topic, but maybe the future language should add something to protect against spillint the default-constructed value (at least of some types) to deep into the program. I do not know how. But this problem often occurs and is not limited to `outcome` or `expected`.
Putting "outcome_errc::not_initialized" inside r made sense, because the principle is that when the returned result has no value, it has to contain the reason for its failure to contain such, and here the reason is that whoever wrote the function left it uninitialized. This looks logical to me, apparently to others not so much.
It's not really that much different from returning EINVAL in r because the programmer made some other kind of logic error - instead of forgetting to initialize r, he forgot to initialize some argument to some lower level function he called, so it returned EINVAL and that's what ended up in r.
This will work for result<T>, because it has to store "any error
whatsoever", but will not for expected
2017-05-26 0:17 GMT+02:00 Peter Dimov via Boost
Andrzej Krzemienski wrote:
Also, they way I look at this solution is not "when I get this value I
have to check ...", but "when I produce this value I have to make sure...". Is there no way to acheive this in the language?
By not providing a default constructor, I suppose. Otherwise not, there's return value elision so if you do
result<T> function() { result<T> r;
// do things
return r; }
nothing is ever called on 'r' on your side if you forget to initialize it.
Putting "outcome_errc::not_initialized" inside r made sense, because the principle is that when the returned result has no value, it has to contain the reason for its failure to contain such, and here the reason is that whoever wrote the function left it uninitialized. This looks logical to me, apparently to others not so much.
It's not really that much different from returning EINVAL in r because the programmer made some other kind of logic error - instead of forgetting to initialize r, he forgot to initialize some argument to some lower level function he called, so it returned EINVAL and that's what ended up in r.
This is only one way of looking at it. Another one is that I have function like, `open_file`, I want it to either return the file handle or the information about run-time situation that prevented opening the file. "reading from uninitialized `expected`" is no such situation: it is simply a bug. In this sense I consider Niall's choice better: just throw, even if it means to terminate(). Regards, &rzej;
Andrzej Krzemienski wrote:
This is only one way of looking at it. Another one is that I have function like, `open_file`, I want it to either return the file handle or the information about run-time situation that prevented opening the file. "reading from uninitialized `expected`" is no such situation: it is simply a bug.
As I say in the paragraph immediately before, if your open_file calls open_file_impl, feeding it invalid arguments, this is simply a bug. Yet when open_file_impl returns EINVAL to indicate invalid arguments, and you return that back to the caller, you end up in the exact same situation.
2017-05-26 13:03 GMT+02:00 Peter Dimov via Boost
Andrzej Krzemienski wrote:
This is only one way of looking at it. Another one is that I have function
like, `open_file`, I want it to either return the file handle or the information about run-time situation that prevented opening the file. "reading from uninitialized `expected`" is no such situation: it is simply a bug.
As I say in the paragraph immediately before, if your open_file calls open_file_impl, feeding it invalid arguments, this is simply a bug. Yet when open_file_impl returns EINVAL to indicate invalid arguments, and you return that back to the caller, you end up in the exact same situation.
And I feel that something is wrong with all these artificially wide contracts: std::sqrt() is well defined for any number, including negative ones. Maybe sometimes it is the worse of the two evils, but implementing this in `expected` (I mean this yet annother error code, I guess I must accept expected's wide contracts), wil impose this quesionable choice on everyone. Regards, &rzej;
Peter Dimov wrote:
Andrzej Krzemienski wrote:
Also, they way I look at this solution is not "when I get this value I have to check ...", but "when I produce this value I have to make sure...". Is there no way to acheive this in the language?
By not providing a default constructor, I suppose. Otherwise not, there's return value elision so if you do
result<T> function() { result<T> r;
// do things
return r; }
nothing is ever called on 'r' on your side if you forget to initialize it.
Actually, there is a way, by providing two types, one default-constructible
with a singular empty state, one without. In the code above, you will still
declare the function to return result<>, which can never be empty and has no
default constructor, but declare `r` to be of type result_option, or
optional_result, or optional<result>, or whatever. Then the `return r` line
will convert and check.
(Or you could use `result
2017-05-26 13:12 GMT+02:00 Peter Dimov via Boost
Peter Dimov wrote:
Andrzej Krzemienski wrote:
Also, they way I look at this solution is not "when I get this value I have to check ...", but "when I produce this value I have to make > sure...". Is there no way to acheive this in the language?
By not providing a default constructor, I suppose. Otherwise not, there's return value elision so if you do
result<T> function() { result<T> r;
// do things
return r; }
nothing is ever called on 'r' on your side if you forget to initialize it.
Actually, there is a way, by providing two types, one default-constructible with a singular empty state, one without. In the code above, you will still declare the function to return result<>, which can never be empty and has no default constructor, but declare `r` to be of type result_option, or optional_result, or optional<result>, or whatever. Then the `return r` line will convert and check.
(Or you could use `result
r` and return `r.value()`, which is a funny exercise in ambiguity.)
Nice. On the other hand, maybe the solution to this should come in the form of clever, programmer-assisted, static analysis. Regards, &rzej;
Ok. Now I am thinking, another thing you can do, is to internally store something like varient
but do not expose this Empty in the interface. Instead, rewrite observer functions like this: ``` bool has_value() const { if (BOOST_UNLIKELY(_is_in_empty_state())) __builtin_unreachable();
return _is_in_valued_state; } ```
Good god no. .has_value() is always legal to call irrespective of the current state. I don't mind the above for say .value(), so if trying to retrieve a value and the current state is not a value, that should be unreachable i.e. static analysis triggers if you're doing an obvious stupidity. Niall -- ned Productions Limited Consulting http://www.nedproductions.biz/ http://ie.linkedin.com/in/nialldouglas/
participants (3)
-
Andrzej Krzemienski
-
Niall Douglas
-
Peter Dimov