Re: [boost] [err] RFC

Subject: Re: [boost] [err] RFC
From: Domagoj Šarić (dsaritz_at_[hidden])
Date: 2015-11-26 12:18:07

"On Fri, 20 Nov 2015 03:36:54 +0530, Gavin Lambert <gavinl_at_[hidden]>
wrote:

> On 20/11/2015 04:58, Domagoj Šarić wrote:
>
>
>>> The more problematic case is if the combiner was not expecting
>>> failure, and so someone used the same expression with a combiner that
>>> accepted T.   So the compiler calls all three calc methods
>>> (constructing fallible_result<T>s along the way), then gets around to
>>> converting the first one back to T, which throws.  This is ok, but
>>> then the other two are destroyed; and if at least one of these throws
>>> too, then you're dead.
>>
>> As explained before this cannot happen as even the implicit conversion
>> operators work only on rvalues...
>
> The result of a function call that returns either a bare T or a T&& is  
> an rvalue.
>
> Your asserts will prevent this particular usage, but that's the only  
> thing that does.  And asserts don't fire until runtime, so if it's an  
> infrequently exercised path (without a unit test) this may go unnoticed  
> for quite a while.  Especially if people are in the habit of testing  
> release builds (which is not that uncommon).

Sorry, I misunderstood you...thought you were talking about converting to
Ts _inside_ the combiner function...
Hm..right I missed that this is actually a shady area in the case of
multiple function parameters...probably because I just assumed that, even
though the order of computation of individual parameter values is
unspecified, each value would be fully computed before the compiler moves
onto the next one...Trying to see what the standard actually says (n3797
draft @ 1.9.15):
"When calling a function (whether or not the function is inline), every
value computation and side effect associated with any argument expression,
or with the postfix expression designating the called function, is
sequenced before execution of every expression or statement in the body of
the called function. [Note: Value computations and side effects associated
with different argument expressions are unsequenced. —end note]"[1]
-> this end note might be interpreted as implying the negative, i.e. that
value computations and side effects associated with _same_ argument
expressions are _not_ unsequenced ... IOW that operations/instructions
producing the value of parameter1 may not be interleaved with those
producing the value of parameter2 (although the order whether parameter1
or parameter2 is produced first is left unspecified).
I may very well be completely mistaken in this amateur 'exegesis' but,
even if I am wrong and 'complete/non-interleaved' parameter values
computation is not
guaranteed fallible_result<> can still be made to work with the desired or  
'good enough' [2]
semantics by allowing multiple fallible_results to exist (removing the  
related asserts)
and inserting an if (!std::uncaught_exception()) check before throwing  
(Scot Meyers'
std::uncaught_exception() related gotw does not apply here).
I can then improve the debugging logic to assert that at least one of the
multiple fallible_results was inspected before leaving the current scope
thereby still catching the contrived case of unexamined/untouched/unused
local fallible_result variables 'hidden&forgotten' in autos. I say
contrived because this is still better than what you currently have with
'regular' return codes, i.e. you don't get even an assertion for
unexamined results. You do get compiler specific warnings about unused
variables but you'd get those just as well for fallible results [3].

[1]
Just earlier than that paragraph we also read:
"If a side effect on a scalar object is unsequenced relative to either
another side effect on the same scalar object or a value computation using
the value of the same scalar object, the behavior is undefined."
...which specifically talks about scalars w/o explaining (AFAICT) the  
difference WRT other types of objects -> FWIW/if this can be interpreted  
in the 'inverse meaning' (i.e. to mean that the described UB does not  
apply to classes i.e. fallible_results)...

[2]
Sure this means that construction of other parameters can proceed even if  
a previous one has already failed but this is actually not that different  
 from what you have to expect anyway precisely because of the undefined  
order in which parameters are constructed...

[3]
Compilers usually omit this warning for types with nontrivial destructors  
(e.g. guard objects) but some compilers offer a function attribute that  
will issue a warning if the function's result is unexamined in all cases.  
In any case/as said before, all this goes away when/if && destructors are  
added to the language (all wrong usage is detected at compile time)...

> Yes, within the function that gets called the && reference parameter is  
> an lvalue, not an rvalue, since it has a name.  But all it takes to make  
> it an rvalue again is a call to std::move.
>
> And this does not seem like unreasonable behaviour in itself, if that  
> only occurs once (perhaps to construct the result_or_error within the  
> function).

I'm not sure what you are getting at here...

> Even this is illegal (and really verbose) using your current rules:
>
>      typedef fallible_result<foo_t, error_t> fallible_foo_t;
>      typedef result_or_error<foo_t, error_t> foo_error_t;
>
>      fallible_foo_t calcA();
>      fallible_foo_t calcB();
>      fallible_foo_t combiner(foo_error_t result1, foo_error_t result2);
>
>      {
>         ...
>         foo_error_t r = combiner(calcA(), calcB());
>         ...
>      }
>
> It doesn't seem like it should be, but it is.  It's even still illegal  
> if you write calcA().as_result_or_error() explicitly in the parameters.

True but that's a mere omission in the debugging code on my side and
easily fixable - I just have to change fallible_result to consider itself
inactive, i.e. decrease its debugging refcount, once/as soon as it is
'inspected' as opposed to waiting until it is destroyed - or simply
replace this assert (as it was already shown to be too simple by your
previous/above more generic example with multiple parameters - IOW the
solution presented for that problem would also fix this one).

As for verbosity, remember to compare this to the 'competition', i.e.
classic error codes, std::error_code and local try-catch blocks...I've
primarily tried to eliminate any extra verbosity in the 'EH mode/style':
that's why fallible_result operators * and -> return T - I could change
the * operator (or add operator ()) to return result_or_error (eliminating  
the need for the
lengthy 'as_result_or_error' which OTOH can also be renamed to something
shorter)...

> The only way it works is to explicitly separate out the calc calls,  
> either as:
>
>          foo_error_t a = calcA();
>          foo_error_t b = calcB();
>
> or as:
>
>          auto a = calcA().as_result_or_error();
>          auto b = calcB().as_result_or_error();
>
> (and woe betide you if you accidentally use "auto" in the first case)

What woe? You'd get a compiler error if you 'used auto in the first case'
and tried to send a and b to 'combiner' (or pretty much do anything else
with them)...

-- 
"What Huxley teaches is that in the age of advanced technology, spiritual
devastation is more likely to come from an enemy with a smiling face than  
 from one whose countenance exudes suspicion and hate."
Neil Postman
---
Ova e-pošta je provjerena na viruse Avast protuvirusnim programom.
https://www.avast.com/antivirus