11

The code is following: (Coliru Code)

#include <stdlib.h>
#include <iostream>
#include <iomanip>
#include <boost/logic/tribool.hpp>

struct B
{
  boost::tribool boo;

  void bug ()
  {
    bool tmp = indeterminate (boo);
    std::cout << "tmp = " << std::boolalpha << tmp << "\n";
    if (tmp && (boo = should_not_be_called ()) == false) {;}
  }

  bool should_not_be_called () const
  {
    std::cout << "BUG, wrong call\n";
    abort ();
  }
};

int main ()
{
  B a;
  a.bug ();
}

The output is 

tmp = false
BUG, wrong call
bash: line 7: 14410 Aborted                 (core dumped) ./a.out

I cannot understand why should_not_be_called is called here. The tested compilers was gcc 4.9 and clang 3.6.

UPDATE:

I read the answers and changed the line with "if" to 

if (tmp && (false == (boo = should_not_be_called ()))) {;}

(Coliru)

Now there are plain bool types on both sides of && operator, but I still got the same error. Why?

Nikki Chumakov
  • 1,215
  • 8
  • 18
  • 4
    My local bookmaker is offering odds of 1000:1 that it's a compiler bug. – Paul R Dec 01 '14 at 19:19
  • 4
    Answered here, I think: http://stackoverflow.com/questions/628526/is-short-circuiting-boolean-operators-mandated-in-c-c-and-evaluation-order – indiv Dec 01 '14 at 19:22
  • 1
    Possibly related: http://stackoverflow.com/questions/21993117/boost-tribool-causing-right-to-left-conditional-evaluation-in-c – Paul R Dec 01 '14 at 19:22
  • 1
    @NikkiChumakov No, operator `==` is also overloaded. `bool && (bool == tribool)` will resolve to `bool && tribool`, which will again, call the overloaded operator and not short-circuit – PeterT Dec 01 '14 at 19:37

2 Answers2

15

The compiler is in the right. Let's analyse the types involved in your two ifs, taking into account all the operator overloads that boost::tribool provides:

if (tmp && (boo = should_not_be_called ()) == false) {;}

if (bool && (tribool = bool) == bool) {;} // = is overloaded to return tribool
if (bool && tribool == bool) {;} // == is overloaded to return tribool
if (bool && tribool) {;} // && is overloaded

And the second if:

if (tmp && (false == (boo = should_not_be_called ()))) {;}

if (bool && (bool == (tribool = bool))) {;} // = is overloaded to return tribool
if (bool && (bool == tribool)) {;} // == is overloaded to return tribool
if (bool && tribool) {;} // && is overloaded

In both cases, we end up with an overloaded operator &&. Operator overloads are functions which do not respect special calling behaviour of the built-in operators. That is, user-overloaded &&, || do not short-circuit, and user overloads of , do not guarantee operand evaluation order. All three evaluate all of their operands, in unspecified order, just like any other function call.

This is precisely the reason why it is strongly discouraged to overload &&, ||, or ,, if you want them to mean something like "and", "or" and "sequence."

Excerpt addressing original text of the question:

The compiler is in the right. boost::tribool overloads opeator !, which means the types of the operands of && are bool and boost::tribool. boost::tribool also overloads operator && for these argument types, so this overload gets called.

Angew is no longer proud of SO
  • 167,307
  • 17
  • 350
  • 455
  • Ok, but I got the same error for the line "if (tmp && false == (boo = should_not_be_called ())) {;}". Notice, it is the bool types on both sides of && op, not tribools. – Nikki Chumakov Dec 01 '14 at 19:30
  • 1
    @NikkiChumakov: No, the left side of `&&` is `tmp`, which is a `boost::tribool`. Maybe you meant `if(tmp==true && (boo=should_not_be_called())==false)`? – Mooing Duck Dec 01 '14 at 19:37
  • Ok, I understand now, the result of (false == (boo = ...)) is tribool. The proper conversion to bool should be something like this: "if (tmp && bool(boo = should_not_be_called ())))" – Nikki Chumakov Dec 01 '14 at 19:41
  • 2
    overloading ```,``` is the worst possible thing I could have learned about today. –  Dec 01 '14 at 20:29
  • 2
    The whole thing looks like the standard defect to me, if operator overloading may change the evaluation order and short circuiting. The language should not be designed this way. – Nikki Chumakov Dec 01 '14 at 21:36
  • @NikkiChumakov It's intentional, so not a defect. There are a few (admittedly rare) good uses of it. It's a horrible idea *here*, I agree, but IMO it's not the standard, it's the person who wrote the overloaded operator who's to blame. –  Dec 01 '14 at 21:56
  • @hvd If I got it right, the order of evaluation of operands for overloaded operator&& is unspecified and implementation specific. So I cannot imagine the good use for this unspecified behavior :-) – Nikki Chumakov Dec 01 '14 at 23:17
  • 2
    Could you please revise your answer a little? The OP modified the question to remove `operator !` from the code, so now your answer seems a little confusing to someone who doesn't check the question's history. Also, by "short-circuit" for `,`, do you mean its ordered evaluation? Because it doesn't really "short-circuit" (at least not in the same sense of the word when talking about `&&` and `||`). (Though all in all, good answer!) – Cornstalks Dec 02 '14 at 02:10
  • 1
    @NikkiChumakov You're right that that's exactly what it means, but if there is a use where the user *won't* expect short-circuiting behaviour, the fact that an overloaded operator disables short-circuiting is not a problem. [Boost Spirit makes clever use of an overloaded `operator||` that users will expect not to short-circuit.](http://www.boost.org/doc/libs/1_46_1/libs/spirit/doc/html/spirit/qi/reference/operator/sequential_or.html) –  Dec 02 '14 at 08:19
  • @hvd That's exactly why I said "if you want them to mean..." in the answer. If you're building a DSL and these operators have a different meaning than the built-in ones, it's fine. – Angew is no longer proud of SO Dec 02 '14 at 16:04
  • @Angew Right. My comments were not meant to contradict your answer, I only disagreed with some of the comments on your answer. :) –  Dec 02 '14 at 16:28
4

The logical operators in boost::tribool are overloaded as mentioned HERE so unlike the built in logical operators of c++ the left to right evaluation of logical operators is not applied and there is no short circuiting so the evaluation of operands remains unspecified.

Freelancer
  • 836
  • 1
  • 14
  • 47
  • 8
    your use of the ACM logo as your profile picture is misleading in my opinion since I doubt you are working for or posting on behalf of the ACM. – Gabriel Southern Dec 01 '14 at 19:44
  • 1
    @Gabriel Not to mention the fact that as per [ACM identity standards](http://identitystandards.acm.org/identitystandards.pdf), use of the "ACM Diamond" without the word mark is "subject to approval from ACM" – Angew is no longer proud of SO Dec 02 '14 at 07:40