Re: [boost] PR: Remove safe_bool idiom from boost.tribool

Subject: Re: [boost] PR: Remove safe_bool idiom from boost.tribool
From: Robert Ramey (ramey_at_[hidden])
Date: 2018-05-22 05:23:50

On 5/21/18 7:05 PM, Gavin Lambert via Boost wrote:
> On 22/05/2018 13:25, Robert Ramey wrote:

>>> No, that's a perfect example of terrible code that should absolutely 
>>> generate a compiler error.  A narrowing conversion (from a source 
>>> type that can express more values than the destination type) should 
>>> absolutely *never* under any circumstances occur implicitly.
>>
>> I think that tribool is/was designed to promote this and in this case 
>> I find it useful and natural.
> 
> No, it wasn't.  The somewhat-implicit bool conversion (safe_bool) was 
> expressly for use within if statements, as demonstrated in the 
> documentation 
> (https://www.boost.org/doc/libs/1_67_0/doc/html/tribool/tutorial.html). 
> Nowhere in this documentation will you find implicitly returning a 
> tribool as a bool.

I doesn't have to since void * (safe_bool) can be converted implicitly 
as bool. And there are many, many cases where this is done.  You might 
not think this is a great idea, but it's a fact.  These programs will 
fail to compile if you change safe_bool to explicit.

> Note that explicit bool is also designed to handle the case where used 
> in a conditional context in an implicit fashion, exactly the same as 
> safe_bool.

Maybe, but it doesn't replicate the behavior of the current safe_bool. 
That is my point.

> Explicit bool also avoids the unintended side effects of implicit bool 
> conversions, such as promotion to integer (as pointed out by Peter) and 
> unintended narrowing when returning or passing parameters.

whatever the intention of safe_bool or explicit, they don't produce the 
same behavior.

> Implicitly returning a tribool as a bool should be forbidden for exactly 
> the same reasons as returning an int64_t as an int32_t -- you are losing 
> information, and unless you have explicitly indicated that this was 
> intended (presumably because you know things that the compiler does 
> not), it should be assumed to be a bug, perhaps as a result of a recent 
> change of types or just an oversight.

Now you're re-hashing the design of tri-bool.  It works the way it has 
for many, many years.  Its consistent with other components such as 
std::optional which do the same thing and what many people are happy 
with.  In this case there is a much better case for it than for 
optional.  Not that it matters. It's not a great idea to silently change 
the operation of a component which has been in usage for 15 years.  If 
this really bugs you, you can just make your own component - just call 
it trilliam, trit or something else.

>> I don't think explicit was created to fix safe_bool.  In general I 
>> support usage of explicit.  But in this case I don't think it's a good 
>> match.
> 
> The explicit keyword itself was not.  But "explicit operator bool" 
> itself was, as part of C++11. 

actually "explicit" can be applied to conversion operators of all types 
- intrinsic as well as user defined.  I'm sure there was no intent to 
think in terms of "explicit bool" as some special situation.

> (You can use the same keywords pre-C++11 
> but it will not have the correct effect.)

Hmmm - I was not  aware that "explicit" could be applied to conversion 
operators before C++11.

> (Specifically, there's a loophole that allows implicit use of explicit 
> bool conversions when in a conditional context such as an if statement 
> where only a bool expression is valid.)

LOL - Right.  That's the root of the whole discussion.  At some point 
long ago everyone thought these implicit conversions were a good idea.
I presume they still do as we have optional which include implicit 
conversions to bool and I'm sure there are some more.

> Thus "explicit" by itself disables all the unintended implicit usages of 
> the conversion, and C++11 then enables the one intended implicit use.

Hmmm - I thought that C++11 enables implicit conversion of any integer 
or pointer to a bool for purposes of if, when etc.

>> Right.  But then I'm imposing your view point on many users programs 
>> which already exist.  You're basically changing the intention of 
>> tribool.  You may well have a good argument that the very idea of 
>> tribool is a bad idea, that it should never have been made the way it 
>> was and that no one should use it, but that ship sailed long, long 
>> ago. It's not relevant here.
> 
> No, you're the one trying to change the intent.
> 
> Implicit bool conversions are a behavioural downgrade and will only 
> serve to introduce bugs.

maybe for components like optional, but I don't see it happening for 
tribool. continuing to permit tribool implicitly convert to bool - as it 
currently - won't introduce any more bugs than it does currently - if 
there are any.

I can't believe this is even an argument.

LOL - neither can I.

FWIW - my original suggestion was to conditionally specify "explicit" 
and eliminate the safe_bool from from the tribool component. This would 
solve my current problem and so I was happy when this PR was created.

But then we discovered that other effects on current user code.  Also it 
became apparent that using explicit wasn't equivalent to the safe_bool 
idiom.  So making this change would silently change the semantics of a 
program depending on whether or not it was compiled with the switch 
std=c++11 or not.  I hope it's obvious that this would be a very bad 
idea.  So I considered all this and now I recommend we drop the 
safe_bool idiom and include an implicit conversion to bool for tribool. 
This wouldn't have this problem and makes things no worse than they are 
now.  To summarize there are for options:

a) leave things as they are.  This includes a compile time error for 
constexpr gcc tribool code.  And also includes implicit conversion to a 
bool return value.

b) move to explicit when supported. - which eliminates the compile error 
but makes behavior dependent to the C++ standard used to compile.  It 
will also break a lot of current user code.

c) implement inplicit conversion to bool.  This breaks no current code 
and resolves the current constexpr issue.  It does permit t + u where t 
and u are results of bool conversions.  This will be fixed automatically 
for C++17.

d) Separately, there is the option to create a "more correct" version of 
tribool under a different name.

in my view c) is the most practical option given the current situation. 
d) is an option for someone who has the time available and want's to 
create, document and submit to boost.  I personally don't have the time 
to do this.

So, I would like the person responsible for maintaining tribool to 
accept this PR.

Robert Ramey