r226870 - Make the ?: precedence warning handle pointers to the left of ?

[Hans Wennborg]

On Thu, Jan 22, 2015 at 2:43 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Thu, Jan 22, 2015 at 2:11 PM, Hans Wennborg <hans at hanshq.net> wrote:
>>
>> Author: hans
>> Date: Thu Jan 22 16:11:56 2015
>> New Revision: 226870
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=226870&view=rev
>> Log:
>> Make the ?: precedence warning handle pointers to the left of ?
>>
>> Previously, Clang would fail to warn on:
>>
>>   int n = x + foo ? 1 : 2;
>>
>> when foo is a pointer.
>>
>> Modified:
>>     cfe/trunk/lib/Sema/SemaExpr.cpp
>>     cfe/trunk/test/Sema/parentheses.c
>>
>> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=226870&r1=226869&r2=226870&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Jan 22 16:11:56 2015
>> @@ -6129,6 +6129,8 @@ static bool ExprLooksBoolean(Expr *E) {
>>      return IsLogicOp(OP->getOpcode());
>>    if (UnaryOperator *OP = dyn_cast<UnaryOperator>(E))
>>      return OP->getOpcode() == UO_LNot;
>> +  if (E->getType()->isPointerType())
>
>
> Could we generalize this a bit further, somehow? (I haven't looked at the
> code in question, but it sounds like this should use some more general tool
> of "try to apply contextual conversion to bool" so that it matches the
> actual semantic situation we're interested in here)

It's tricky, because we don't really want to match the actual
semantics, we want to figure out if the intention was to use 'foo' as
a conditional expression. That's what 'ExprLooksBoolean' does, and
it's erring on the side of caution.

For example, we don't want to warn if 'foo' is int, even if that could
be used as a conditional expression. But we do want to warn if 'foo'
is 'a==b', even in C where the type of that expression is int.

Having said that, I now realised I might have made the warning a
little too broad.. we want to warn here:

  int x = x + ptr ? 1 : 0;

because 'x + ptr' seems like a pretty unlikely condition expression.

But we don't want to warn here:

  int y = ptr1 - ptr2 ? 1 : 0;

I'll think about that.