[cfe-commits] r76911 - /cfe/trunk/lib/Analysis/CFG.cpp

[Ted Kremenek]

On Jul 23, 2009, at 9:33 PM, Eli Friedman wrote:

>>> +    // See if this is a known constant.
>>> +    int KnownVal = TryEvaluateBool(B->getLHS());
>>> +    if (KnownVal != -1 && (B->getOpcode() == BinaryOperator::LOr))
>>> +      KnownVal = !KnownVal;
>>> +
>>>     // Now link the LHSBlock with RHSBlock.
>>>     if (B->getOpcode() == BinaryOperator::LOr) {
>>> -      if (KnownTrue)
>>> +      if (KnownVal == true)
>>>         LHSBlock->addSuccessor(0);
>>>       else
>>>         LHSBlock->addSuccessor(ConfluenceBlock);
>>> -      if (KnownFalse)
>>> +      if (KnownVal == false)
>>>         LHSBlock->addSuccessor(0);
>>
>> Hi Mike,
>>
>> This looks a little suspicious to me.  Doesn't '-1' convert to 'true'
>> when an int is casted to a bool?  This would cause 'KnowVal == true'
>> to evaluate to true even when 'KnownVal' is -1.
>
> (int)x == false is semantically equivalent to (int)x == 0 due to
> integer promotion rules.  That said, it is slightly confusing the way
> it's written.


The problem isn't with 'x == 0', but with 'x == -1'.  Since (I  
believe) both '1' and '-1' convert to true when int is casted to a  
bool, the 'KnownVal == true' would evaluate to true in both cases  
(which isn't the desired behavior).