PR: Remove safe_bool idiom from boost.tribool

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
23 messages Options
12
Reply | Threaded
Open this post in threaded view
|

PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
Removed usage of "safe_bool" idiom from tribool.

This was initially motivated by the fact that usage of this idiom with
all current and past gcc compilers with constexpr does not work.  First
it reports reference to the address of a local member function as not
being constexpr.  This may or may not be correct according to the
standard.  FWIW clang is OK with it.  Making the member function
constexpr fixes this error.  But still there is a problem.  in GCC, the
operator to implicitly covert the instance to a void * does not seem to
work.  Invocation of this operator in the context of an argument to an
if(... or other conditional doesn't result in behavior similar to bool.
Not even a static cast to a bool seems to work.  Again CLANG is OK with
all this.  This has been reported as an issue to the GCC team.

One idea which has been tried is to use C++11 "explicit" as a substitute
from the safe_bool idiom.  Unfortunately this breaks legacy code.

bool f(x) {
        return tribool(true); // fails
}

So this is not a great solution either.

Attempts to resolve this were inconclusive.  This motivated me to think
about why the safe_bool idiom was being used here at all.  According to
articles on safe_bool articles this main reason is to support the common
practice of using a not null value in a conditional:

Testable t;

if(t) ...

without accidentally permitting things like

if(t >> 1)...

The above would occur if Testable included an operator bool () const
member function.

Basically safe_bool is meant to support the common C/C++ practice of
implicity converting something to bool so we write something like if(t)
... to mean t "is_valid" or "is_not_null" or some such.

a) I don't think this idiom is a good idea and even more so in the
context of modern C++.

Conversion of a Testable instance to a bool instance should result in
something that looks/acts like a C bool - not something else like a void
* which C then maps to a bool.  If we want to use a bool for something
like "is_valid" we'd be much better off just implementing is_valid
directly.  There would be then no confusion, side effects or hand a waving.

b) it's an especially a bad idea for tribool.

The motivating concept behind tribool is that of some sort of "extended"
bool. The naming suggests that it acts like a bool.  But since we've
used he safe_bool idiom, it doesn't any more.  That is we can't use a
tribool anywhere a bool is used. So if we use operator bool we'll get a
tribool which acts like a bool - even when the original usage of bool
was a bad idea according to a) above.  But at least we have the same
behavior for tribool and bool which is a lot more intuitive and less
confusing.

Also, changing to operator bool () will address the current problem with
GCC not supporting a constexpr version of tribool.

Accordingly, I've submitted this PR to change the implementation of
tribool to avoid the safe_bool idiom.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
On 19 May 2018 at 00:17, Robert Ramey via Boost <[hidden email]>
wrote:

> The motivating concept behind tribool is that of some sort of "extended"
> bool. The naming suggests that it acts like a bool.


I would submit that the name is wrong. From the introduction: " The 3-state
boolean library contains a single class, boost::logic::tribool
<https://www.boost.org/doc/libs/1_67_0/doc/html/boost/logic/tribool.html>,
along with support functions and operator overloads that implement 3-state
boolean logic." Boolean logic has 2 states, the latter part of the quote is
a contradiction in terms. Other logic "systems" are applicable:
https://en.wikipedia.org/wiki/Three-valued_logic

degski
--
*"If something cannot go on forever, it will stop" - Herbert Stein*

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
On Fri, May 18, 2018 at 10:55 PM, degski via Boost
<[hidden email]> wrote:
> Other logic "systems" are applicable:
> https://en.wikipedia.org/wiki/Three-valued_logic

If you are proposing that we leave boost::tribool alone for legacy
reasons, and then add boost::trilean which addresses the semantic
deficiencies in tribool in a manner consistent with trilean logic,
then I'm on board. I will make the necessary changes to Beast (which
currently uses tribool).

Regards

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On Sat, May 19, 2018 at 1:17 AM, Robert Ramey via Boost
<[hidden email]> wrote:
>
> One idea which has been tried is to use C++11 "explicit" as a substitute
> from the safe_bool idiom.  Unfortunately this breaks legacy code.
>
> bool f(x) {
>         return tribool(true); // fails
> }
>
> So this is not a great solution either.

C++11 explicit conversion operators don't support this use case, so if
it works with the safe_bool idiom then it is either a bug or an
unavoidable unfortunate limitation of the emulation. User's code
should be updated in this case.

> Attempts to resolve this were inconclusive.  This motivated me to think
> about why the safe_bool idiom was being used here at all.  According to
> articles on safe_bool articles this main reason is to support the common
> practice of using a not null value in a conditional:
>
> Testable t;
>
> if(t) ...
>
> without accidentally permitting things like
>
> if(t >> 1)...
>
> The above would occur if Testable included an operator bool () const member
> function.

There are also other accidents that are intended to be prevented by
safe_bool/explicit conversion operators. For example:

  void foo(bool f);

  foo(t);

Some accidents are also prevented by the recent changes to the
language. For example, increment/decrement operations on bool are
deprecated since C++11 and removed since C++17.

> a) I don't think this idiom is a good idea and even more so in the context
> of modern C++.
>
> Conversion of a Testable instance to a bool instance should result in
> something that looks/acts like a C bool - not something else like a void *
> which C then maps to a bool.  If we want to use a bool for something like
> "is_valid" we'd be much better off just implementing is_valid directly.
> There would be then no confusion, side effects or hand a waving.

This is obviously very case-specific, but I find certain types like
smart pointers or optional rather natural to support explicit
conversion to bool. Such an operation has obvious meaning where the
conversion would apply. Having empty() or is_null() instead would just
increase verbosity for no gain.

Note that having a dedicated method is also useful in other contexts.
Like in my foo example above, when you actually want to call that
function with a bool that indicates whether Testable is valid or some
such.

> b) it's an especially a bad idea for tribool.
>
> The motivating concept behind tribool is that of some sort of "extended"
> bool. The naming suggests that it acts like a bool.  But since we've used he
> safe_bool idiom, it doesn't any more.  That is we can't use a tribool
> anywhere a bool is used. So if we use operator bool we'll get a tribool
> which acts like a bool - even when the original usage of bool was a bad idea
> according to a) above.  But at least we have the same behavior for tribool
> and bool which is a lot more intuitive and less confusing.
>
> Also, changing to operator bool () will address the current problem with GCC
> not supporting a constexpr version of tribool.
>
> Accordingly, I've submitted this PR to change the implementation of tribool
> to avoid the safe_bool idiom.

I would say I agree that the conversion operator is not a good idea
with regard to tribool. But I don't quite understand how making the
conversion _easier_ to invoke would make it better. If anything, I
would rather work towards removing the conversion operator entirely
instead of making it implicit. I think this change is a step in the
wrong direction.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 5/18/18 10:55 PM, degski via Boost wrote:

> On 19 May 2018 at 00:17, Robert Ramey via Boost <[hidden email]>
> wrote:
>
>> The motivating concept behind tribool is that of some sort of "extended"
>> bool. The naming suggests that it acts like a bool.
>
>
> I would submit that the name is wrong. From the introduction: " The 3-state
> boolean library contains a single class, boost::logic::tribool
> <https://www.boost.org/doc/libs/1_67_0/doc/html/boost/logic/tribool.html>,
> along with support functions and operator overloads that implement 3-state
> boolean logic." Boolean logic has 2 states, the latter part of the quote is
> a contradiction in terms. Other logic "systems" are applicable:
> https://en.wikipedia.org/wiki/Three-valued_logic
>
> degski
>

Well the word "tribool" may not be defined on the wikipedia page, but it
seems pretty clear to most of what is intended - logic based on true,
false and indeterminant.  I don't think renaming things is going to help
things - although it would create a ton work.

Robert Ramey


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 5/19/18 8:18 AM, Andrey Semashev via Boost wrote:

> On Sat, May 19, 2018 at 1:17 AM, Robert Ramey via Boost
> <[hidden email]> wrote:
>>
>> One idea which has been tried is to use C++11 "explicit" as a substitute
>> from the safe_bool idiom.  Unfortunately this breaks legacy code.
>>
>> bool f(x) {
>>          return tribool(true); // fails
>> }
>>
>> So this is not a great solution either.
>
> C++11 explicit conversion operators don't support this use case, so if
> it works with the safe_bool idiom then it is either a bug or an
> unavoidable unfortunate limitation of the emulation.

I don't think that one can conclude that because C++11 expicit
conversion operators don't support a use case that it's a bug. The
question is whether conversion to bool from tribool makes some sort of
sense.  I think it does - regardless of what C++11 actually does.

> User's code should be updated in this case.

This code has been in usage for 15 years. One can just start using the
explicit bool conversion and then break tons of code - some of it very
old.  In one case (QT) this change generated 100's of syntax errors in
existing code.

>> Attempts to resolve this were inconclusive.  This motivated me to think
>> about why the safe_bool idiom was being used here at all.  According to
>> articles on safe_bool articles this main reason is to support the common
>> practice of using a not null value in a conditional:
>>
>> Testable t;
>>
>> if(t) ...
>>
>> without accidentally permitting things like
>>
>> if(t >> 1)...
>>
>> The above would occur if Testable included an operator bool () const member
>> function.

> There are also other accidents that are intended to be prevented by
> safe_bool/explicit conversion operators. For example:
>
>    void foo(bool f);
>
>    foo(t);

Right - I get this.  I think this is a good conversion to support.  That
is why my change supports implicit conversion from tribool to bool.

> Some accidents are also prevented by the recent changes to the
> language. For example, increment/decrement operations on bool are
> deprecated since C++11 and removed since C++17.

Hmmm - I did not know that.  Since one of the main motivations of
safe_bool is to inhibit this behavior, and it will shortly be inhibited
by a C++17 in any case, it supports my argument for eliminating
safe_bool idiom from tribool.  Thanks for noticing this.

>> a) I don't think this idiom is a good idea and even more so in the context
>> of modern C++.
>>
>> Conversion of a Testable instance to a bool instance should result in
>> something that looks/acts like a C bool - not something else like a void *
>> which C then maps to a bool.  If we want to use a bool for something like
>> "is_valid" we'd be much better off just implementing is_valid directly.
>> There would be then no confusion, side effects or hand a waving.
>
> This is obviously very case-specific, but I find certain types like
> smart pointers or optional rather natural to support explicit
> conversion to bool. Such an operation has obvious meaning where the
> conversion would apply. Having empty() or is_null() instead would just
> increase verbosity for no gain.

> Note that having a dedicated method is also useful in other contexts.
> Like in my foo example above, when you actually want to call that
> function with a bool that indicates whether Testable is valid or some
> such.

Actually, all these cases are "case specific".  I have views on the
conversion to bool for optional an others, but we're only talking about
tribool here.

>> b) it's an especially a bad idea for tribool.
>>
>> The motivating concept behind tribool is that of some sort of "extended"
>> bool. The naming suggests that it acts like a bool.  But since we've used he
>> safe_bool idiom, it doesn't any more.  That is we can't use a tribool
>> anywhere a bool is used. So if we use operator bool we'll get a tribool
>> which acts like a bool - even when the original usage of bool was a bad idea
>> according to a) above.  But at least we have the same behavior for tribool
>> and bool which is a lot more intuitive and less confusing.
>>
>> Also, changing to operator bool () will address the current problem with GCC
>> not supporting a constexpr version of tribool.
>>
>> Accordingly, I've submitted this PR to change the implementation of tribool
>> to avoid the safe_bool idiom.

and implement a normal operator bool instead!

> I would say I agree that the conversion operator is not a good idea
> with regard to tribool.

Hmmm - I'm proposing adding and ordinary conversion operator to convert
tribool to bool. How can you agree with me while saying it's not a good
idea?

> But I don't quite understand how making the
> conversion _easier_ to invoke would make it better. If anything, I
> would rather work towards removing the conversion operator entirely
> instead of making it implicit. I think this change is a step in the
> wrong direction.

Sure we can disagree.  I've made my argument - I think it's pretty clear.

Robert Ramey



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
On Sat, May 19, 2018 at 7:09 PM, Robert Ramey via Boost
<[hidden email]> wrote:

> On 5/19/18 8:18 AM, Andrey Semashev via Boost wrote:
>>
>> On Sat, May 19, 2018 at 1:17 AM, Robert Ramey via Boost
>> <[hidden email]> wrote:
>>>
>>>
>>> One idea which has been tried is to use C++11 "explicit" as a substitute
>>> from the safe_bool idiom.  Unfortunately this breaks legacy code.
>>>
>>> bool f(x) {
>>>          return tribool(true); // fails
>>> }
>>>
>>> So this is not a great solution either.
>>
>>
>> C++11 explicit conversion operators don't support this use case, so if
>> it works with the safe_bool idiom then it is either a bug or an
>> unavoidable unfortunate limitation of the emulation.
>
> I don't think that one can conclude that because C++11 expicit conversion
> operators don't support a use case that it's a bug. The question is whether
> conversion to bool from tribool makes some sort of sense.  I think it does -
> regardless of what C++11 actually does.

Explicit conversion operators are the evolution of the safe_bool
idiom. In other words, it represents what we tried to achieve with
safe_bool (and possibly failed in some aspects).

>> User's code should be updated in this case.
>
> This code has been in usage for 15 years. One can just start using the
> explicit bool conversion and then break tons of code - some of it very old.
> In one case (QT) this change generated 100's of syntax errors in existing
> code.

I'm not saying this is going to be easy, but the change would be for
the better, IMHO. We could offer some transition period.

>> There are also other accidents that are intended to be prevented by
>> safe_bool/explicit conversion operators. For example:
>>
>>    void foo(bool f);
>>
>>    foo(t);
>
> Right - I get this.  I think this is a good conversion to support.  That is
> why my change supports implicit conversion from tribool to bool.

I tried using tribool in my projects multiple times and every time I
dropped it soon after the start in favor of an enum or something
similar. The reason is that I can never readily tell how the
conversion to bool works. The example above illustrates this rather
well - I can never say what foo will receive if t is indeterminate.

>> Some accidents are also prevented by the recent changes to the
>> language. For example, increment/decrement operations on bool are
>> deprecated since C++11 and removed since C++17.
>
> Hmmm - I did not know that.  Since one of the main motivations of safe_bool
> is to inhibit this behavior, and it will shortly be inhibited by a C++17 in
> any case, it supports my argument for eliminating safe_bool idiom from
> tribool.

I don't think it does. The change removes ambiguous or plain wrong
operations on bool, and explicit conversion (and, similarly,
safe_bool) are there for the same reason, only for user's types.

>>> b) it's an especially a bad idea for tribool.
>>>
>>> The motivating concept behind tribool is that of some sort of "extended"
>>> bool. The naming suggests that it acts like a bool.  But since we've used
>>> he
>>> safe_bool idiom, it doesn't any more.  That is we can't use a tribool
>>> anywhere a bool is used. So if we use operator bool we'll get a tribool
>>> which acts like a bool - even when the original usage of bool was a bad
>>> idea
>>> according to a) above.  But at least we have the same behavior for
>>> tribool
>>> and bool which is a lot more intuitive and less confusing.
>>>
>>> Also, changing to operator bool () will address the current problem with
>>> GCC
>>> not supporting a constexpr version of tribool.
>>>
>>> Accordingly, I've submitted this PR to change the implementation of
>>> tribool
>>> to avoid the safe_bool idiom.
>
> and implement a normal operator bool instead!
>
>> I would say I agree that the conversion operator is not a good idea
>> with regard to tribool.
>
> Hmmm - I'm proposing adding and ordinary conversion operator to convert
> tribool to bool. How can you agree with me while saying it's not a good
> idea?

Maybe I misunderstood you, but you said yourself that this is "a bad
idea for tribool". Sorry if I misunderstood.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
As usual, discussions have gotten off on various tangents.

My proposal is to replace the safe_bool idiom used in boost::tribool
with a simple operator bool.

I maintain that this will

a) not break anything already working.
b) make tribool better model tri-valued logic
c) address a failing in gcc which inhibits the current tribool from
being constexpr.
d) So that all in all we will be in a slightly better place.

That's the whole thing.

It's not about

a) whether or not tribool itself is good idea
b) whether tribool is correctly named
c) whether implicit conversions to bool in other cases like optional are
a good idea
d) or anything else other than the above.

My intent to to get this PR accepted so I don't have to clone tribool
into my safe numerics library.  Selfish I know - but there it is.

So far no one has posted any reason why my proposal would not be an
improvement.

Robert Ramey


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 5/19/2018 1:22 PM, Andrey Semashev via Boost wrote:

> On Sat, May 19, 2018 at 7:09 PM, Robert Ramey via Boost
> <[hidden email]> wrote:
>> On 5/19/18 8:18 AM, Andrey Semashev via Boost wrote:
>>>
>>> On Sat, May 19, 2018 at 1:17 AM, Robert Ramey via Boost
>>> <[hidden email]> wrote:
>>>>
>>>>
>>>> One idea which has been tried is to use C++11 "explicit" as a substitute
>>>> from the safe_bool idiom.  Unfortunately this breaks legacy code.
>>>>
>>>> bool f(x) {
>>>>           return tribool(true); // fails
>>>> }
>>>>
>>>> So this is not a great solution either.
>>>
>>>
>>> C++11 explicit conversion operators don't support this use case, so if
>>> it works with the safe_bool idiom then it is either a bug or an
>>> unavoidable unfortunate limitation of the emulation.
>>
>> I don't think that one can conclude that because C++11 expicit conversion
>> operators don't support a use case that it's a bug. The question is whether
>> conversion to bool from tribool makes some sort of sense.  I think it does -
>> regardless of what C++11 actually does.
>
> Explicit conversion operators are the evolution of the safe_bool
> idiom. In other words, it represents what we tried to achieve with
> safe_bool (and possibly failed in some aspects).
>
>>> User's code should be updated in this case.
>>
>> This code has been in usage for 15 years. One can just start using the
>> explicit bool conversion and then break tons of code - some of it very old.
>> In one case (QT) this change generated 100's of syntax errors in existing
>> code.
>
> I'm not saying this is going to be easy, but the change would be for
> the better, IMHO. We could offer some transition period.
>
>>> There are also other accidents that are intended to be prevented by
>>> safe_bool/explicit conversion operators. For example:
>>>
>>>     void foo(bool f);
>>>
>>>     foo(t);
>>
>> Right - I get this.  I think this is a good conversion to support.  That is
>> why my change supports implicit conversion from tribool to bool.
>
> I tried using tribool in my projects multiple times and every time I
> dropped it soon after the start in favor of an enum or something
> similar. The reason is that I can never readily tell how the
> conversion to bool works. The example above illustrates this rather
> well - I can never say what foo will receive if t is indeterminate.

It is documented that the:

"conversion to bool will be true when the value of the tribool is always
true, and false otherwise."

So you always know how the conversion to bool works. Not reading the doc
is not an excuse for not knowing.



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
On Sun, May 20, 2018 at 2:54 AM, Edward Diener via Boost
<[hidden email]> wrote:

> On 5/19/2018 1:22 PM, Andrey Semashev via Boost wrote:
>>
>> I tried using tribool in my projects multiple times and every time I
>> dropped it soon after the start in favor of an enum or something
>> similar. The reason is that I can never readily tell how the
>> conversion to bool works. The example above illustrates this rather
>> well - I can never say what foo will receive if t is indeterminate.
>
> It is documented that the:
>
> "conversion to bool will be true when the value of the tribool is always
> true, and false otherwise."
>
> So you always know how the conversion to bool works. Not reading the doc is
> not an excuse for not knowing.

Some stuff is intuitive and you don't even need to read the docs to
know how it works. Some other stuff is arbitrary and I will never
remember how it works no matter how many times I read the docs. This
tribool conversion to bool is of the latter kind. And frankly, without
having looked at the docs I would have guessed that it converts to
false if the tribool is false and true otherwise because that's how
other types work (e.g. pointers convert to false when null and true
when any other value).

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 20/05/2018 06:34, Robert Ramey wrote:
> As usual, discussions have gotten off on various tangents.
>
> My proposal is to replace the safe_bool idiom used in boost::tribool
> with a simple operator bool.

I'm strongly opposed to downgrading from safe_bool to implicit bool.

Upgrading to explicit bool is ok, but you can only do that on C++11;
it's not safe in older compilers.

I would be ok with #if logic that uses safe_bool on older compilers and
explicit bool on C++11 and up.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
On 5/20/18 5:34 PM, Gavin Lambert via Boost wrote:
> On 20/05/2018 06:34, Robert Ramey wrote:
>> As usual, discussions have gotten off on various tangents.
>>
>> My proposal is to replace the safe_bool idiom used in boost::tribool
>> with a simple operator bool.
>
> I'm strongly opposed to downgrading from safe_bool to implicit bool.

I made my argument why this is good idea.  What's wrong with my argument?

>
> Upgrading to explicit bool is ok, but you can only do that on C++11;
> it's not safe in older compilers.

This would/could break lot's of current code. Basically any code which
looks like the following:

bool f() {
        tribool tb ...
        return tb;
}

which to me looks like a perfectly reasonable thing to do.  But using
explicit would create a compile time error.
>
> I would be ok with #if logic that uses safe_bool on older compilers and
> explicit bool on C++11 and up.

This to me seems the worst.  It would mean that the semantics of tribool
vary depending on which standard the compiler is adhering to.  It would
be impossible to discern the behavior of the code just by looking at it.
You'd have to know which compile time switches are being used.

Robert Ramey


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
On 22/05/2018 02:53, Robert Ramey wrote:
>> I'm strongly opposed to downgrading from safe_bool to implicit bool.
>
> I made my argument why this is good idea.  What's wrong with my argument?

Perhaps I missed something, but your argument seemed to be "safe bool
doesn't work in newer compilers" (so fine, use explicit bool instead,
since newer compilers do support that), and "explicit bool doesn't let
me write bad code" (so don't do that).

> This would/could break lot's of current code. Basically any code which
> looks like the following:
>
> bool f() {
>      tribool tb ...
>      return tb;
> }
>
> which to me looks like a perfectly reasonable thing to do.  But using
> explicit would create a compile time error.

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.

This is exactly the reason why safe_bool was created in the first place,
and why explicit bool was corrected in C++11 to match.

If you *want* that code to compile, then "return bool(tb);" (or use
static_cast if you prefer) instead.  And this will work with both
safe_bool and explicit bool.

>> I would be ok with #if logic that uses safe_bool on older compilers
>> and explicit bool on C++11 and up.
>
> This to me seems the worst.  It would mean that the semantics of tribool
> vary depending on which standard the compiler is adhering to.  It would
> be impossible to discern the behavior of the code just by looking at it.
> You'd have to know which compile time switches are being used.

The semantics of explicit bool are the same as the semantics of
safe_bool, just better as it has proper compiler support instead of
taking advantage of a hack to work around the previous defect in
operator bool.


Personally, the thing I would most prefer is to get rid of any bool
conversion at all and require code to explicitly test tribools for
==true/==false/boost::indeterminate, as people will usually disagree on
whether indeterminate should be truthy or falsy when forced to pick one.
  But I am not advocating that as I think the ship has long sailed.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
Robert Ramey wrote:

> > I'm strongly opposed to downgrading from safe_bool to implicit bool.
>
> I made my argument why this is good idea.  What's wrong with my argument?

Defining a conversion to bool enables unwanted operations such as

    tribool b1, b2;

    b1 < b2;
    b1 + b2;
    b1 + 5;
    b1 * 2;

That's the whole point of "safe bool", to avoid those.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 5/21/18 4:49 PM, Gavin Lambert via Boost wrote:

> On 22/05/2018 02:53, Robert Ramey wrote:
>>> I'm strongly opposed to downgrading from safe_bool to implicit bool.
>>
>> I made my argument why this is good idea.  What's wrong with my argument?
>
> Perhaps I missed something, but your argument seemed to be "safe bool
> doesn't work in newer compilers" (so fine, use explicit bool instead,
> since newer compilers do support that), and "explicit bool doesn't let
> me write bad code" (so don't do that).
>
>> This would/could break lot's of current code. Basically any code which
>> looks like the following:
>>
>> bool f() {
>>      tribool tb ...
>>      return tb;
>> }
>>
>> which to me looks like a perfectly reasonable thing to do.  But using
>> explicit would create a compile time error.

I disagree.
>
> 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.

> This is exactly the reason why safe_bool was created in the first place,

safe_bool never corrected this and won't in C++11.

> and why explicit bool was corrected in C++11 to match.

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.

> If you *want* that code to compile, then "return bool(tb);"

Hmmm - I don't think this actually depends upon implicit conversion of
tribool to bool.

>(or use static_cast if you prefer) instead.

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.

   And this will work with both

> safe_bool and explicit bool.
>
>>> I would be ok with #if logic that uses safe_bool on older compilers
>>> and explicit bool on C++11 and up.
>>
>> This to me seems the worst.  It would mean that the semantics of
>> tribool vary depending on which standard the compiler is adhering to.  
>> It would be impossible to discern the behavior of the code just by
>> looking at it. You'd have to know which compile time switches are
>> being used.
>
> The semantics of explicit bool are the same as the semantics of
> safe_bool,

No they are not. And that's the exact problem. You can review my example
above or check out this short video

https://www.youtube.com/watch?v=7uIyl_tDMkM

> just better as it has proper compiler support instead of
> taking advantage of a hack to work around the previous defect in
> operator bool.

but it's not the same thing.

> Personally, the thing I would most prefer is to get rid of any bool
> conversion at all and require code to explicitly test tribools for
> ==true/==false/boost::indeterminate,

OK - but neither of us were around 15 years ago when it was reviewed.
Unilateral changing it at this point would change the whole design and
intention and break existing code. I don't see how that could possibly
be acceptable.

Of course if you want to make and propose an alternative library named
trilleam or something like that you're more than welcome.  But we can't
trapse all over boost mucking up things wily nilly.

The change I propose:

a) won't change the current semantics of tribool as it relates to any
currently working programs.
b) will address a current problem with tribool and gcc
c) will "recover" the trapping behavior of safe_bool with C++17

It's really a no-brainer.

> as people will usually disagree on
> whether indeterminate should be truthy or falsy when forced to pick one.
>   But I am not advocating that as I think the ship has long sailed.


Of course if you want to make and propose an alternative library named
trilleam or something like that you're more than welcome.  But we can't
trapse all over boost mucking up things wily nilly.

Robert Ramey



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
On 22/05/2018 13:25, Robert Ramey wrote:
>>> which to me looks like a perfectly reasonable thing to do.  But using
>>> explicit would create a compile time error.
>
> I disagree.

I am not sure what you are disagreeing with here, unless you intended to
disagree with yourself?

>> 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.

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.

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.

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.

> 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.  (You can use the same keywords pre-C++11
but it will not have the correct effect.)

(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.)

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

> 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.  I can't believe this is even an argument.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 5/21/18 6:02 PM, Peter Dimov via Boost wrote:

> Robert Ramey wrote:
>
>> > I'm strongly opposed to downgrading from safe_bool to implicit bool.
>>
>> I made my argument why this is good idea.  What's wrong with my argument?
>
> Defining a conversion to bool enables unwanted operations such as
>
>     tribool b1, b2;
>
>     b1 < b2;
>     b1 + b2;
>     b1 + 5;
>     b1 * 2;
>
> That's the whole point of "safe bool", to avoid those.

Right.

But - Since the have been prohibited by the current safe_bool, there are
none in current code. So the change won't break anything.

Finally.  I believe that these are compile time errors in C++17.  So
safe_bool isn't needed to prevent them anymore anyway.

Robert Ramey

>
> _______________________________________________
> Unsubscribe & other changes:
> http://lists.boost.org/mailman/listinfo.cgi/boost
>



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
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






_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
On 22/05/2018 17:23, Robert Ramey wrote:

> On 5/21/18 7:05 PM, Gavin Lambert wrote:
>> 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.

The intended purpose of safe_bool implicitly converting to bool was for
use in conditional expression contexts.

That it permitted other narrowing conversions is not intended and is
unfortunate.

> 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.

std::optional was added after C++11 and as such uses explicit bool.  It
does not have implicit conversions.

Older versions of boost::optional did use safe_bool, but have since been
updated to use explicit bool where compiler-supported (via macro
BOOST_EXPLICIT_OPERATOR_BOOL_NOEXCEPT).

So both of these support my argument, not yours.

(Perhaps the better PR would be to make tribool use that macro, defined
in boost/core/explicit_operator_bool.hpp)

> 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.

I'm referring to the contextual bool conversion mentioned later on.  It
is indeed a 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.

True, I misspoke; I was thinking of explicit constructors, which don't
apply here as it's the wrong conversion direction.

> 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.

Implicit widening conversions can still be a good idea.  Implicit
narrowing conversions never are.  And again optional does not do what
you claim.

>> 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.

Anything that is explicitly convertible to bool, yes.  That doesn't
invalidate my point.

> 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.

This is incorrect, as Peter already pointed out.

Additionally, you should consider the effect on future code -- it's not
really a great argument to claim that a safety railing is no longer
necessary because nobody has fallen off the cliff while the safety
railing was there.

> 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.

This is the only option that makes sense to me.  If that breaks user
code, it is a net positive effect because that code was already broken
-- and it is trivial to correct it where actually intended.

This is no different than when boost::optional switched to explicit
bool, to use your own example correctly.

> 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.

As far as I am aware, this is not correct; bool is still implicitly
convertible to int in all versions of C++.
(And https://en.cppreference.com/w/cpp/language/implicit_conversion agrees.)

I welcome proof otherwise; although that still would not change my
opinion as it doesn't help those using pre-C++17 compilers.


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
Reply | Threaded
Open this post in threaded view
|

Re: PR: Remove safe_bool idiom from boost.tribool

Boost - Dev mailing list
In reply to this post by Boost - Dev mailing list
On 05/22/18 07:47, Robert Ramey via Boost wrote:

> On 5/21/18 6:02 PM, Peter Dimov via Boost wrote:
>> Robert Ramey wrote:
>>
>>> > I'm strongly opposed to downgrading from safe_bool to implicit bool.
>>>
>>> I made my argument why this is good idea.  What's wrong with my
>>> argument?
>>
>> Defining a conversion to bool enables unwanted operations such as
>>
>>     tribool b1, b2;
>>
>>     b1 < b2;
>>     b1 + b2;
>>     b1 + 5;
>>     b1 * 2;
>>
>> That's the whole point of "safe bool", to avoid those.
>
> Right.
>
> But - Since the have been prohibited by the current safe_bool, there are
> none in current code. So the change won't break anything.

It won't break the current code, but it will open avenue for bugs like
that in the future code.

> Finally.  I believe that these are compile time errors in C++17.

No, they are not.

struct tribool
{
     int value;

     EXPLICIT operator bool() const { return value != 0; }
};

int main()
{
     tribool b1, b2;
     b1 < b2;
     b1 + b2;
     b1 + 5;
     b1 * 2;
}

$ g++ -std=c++17 -DEXPLICIT= -o tribool_conv tribool_conv.cpp

$ g++ -std=c++17 -DEXPLICIT=explicit -o tribool_conv tribool_conv.cpp
tribool_conv.cpp: In function ‘int main()’:
tribool_conv.cpp:11:8: error: no match for ‘operator<’ (operand types
are ‘tribool’ and ‘tribool’)
      b1 < b2;
      ~~~^~~~
tribool_conv.cpp:11:8: note: candidate: operator<(int, int) <built-in>
tribool_conv.cpp:11:8: note:   no known conversion for argument 2 from
‘tribool’ to ‘int’
tribool_conv.cpp:12:8: error: no match for ‘operator+’ (operand types
are ‘tribool’ and ‘tribool’)
      b1 + b2;
      ~~~^~~~
tribool_conv.cpp:12:8: note: candidate: operator+(int, int) <built-in>
tribool_conv.cpp:12:8: note:   no known conversion for argument 2 from
‘tribool’ to ‘int’
tribool_conv.cpp:13:8: error: no match for ‘operator+’ (operand types
are ‘tribool’ and ‘int’)
      b1 + 5;
      ~~~^~~
tribool_conv.cpp:13:8: note: candidate: operator+(int, int) <built-in>
tribool_conv.cpp:13:8: note:   no known conversion for argument 1 from
‘tribool’ to ‘int’
tribool_conv.cpp:14:8: error: no match for ‘operator*’ (operand types
are ‘tribool’ and ‘int’)
      b1 * 2;
      ~~~^~~
tribool_conv.cpp:14:8: note: candidate: operator*(int, int) <built-in>
tribool_conv.cpp:14:8: note:   no known conversion for argument 1 from
‘tribool’ to ‘int’


_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
12