[cfe-commits] r71145 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp test/Sema/block-misc.c test/SemaObjC/conditional-expr-2.m

[Eli Friedman]

On Thu, May 7, 2009 at 11:53 AM, Mike Stump <mrs at apple.com> wrote:
>> (It's fine if it isn't, but I'd like confirmation.)  In any case, I don't
>> think you want to be
>> creating a block pointer with a qualified function type.
>
> The compiler claims that isn't what happens.  Thoughts?  When I run:

Oh, oops, nevermind.  I got myself confused about the returned type.

>>> @@ -3671,7 +3680,7 @@
>>>  QualType lType = lex->getType();
>>>  QualType rType = rex->getType();
>>>
>>> -  if (!lType->isFloatingType()) {
>>> +  if (!lType->isFloatingType() && !(lType->isBlockPointerType() &&
>>> isRelational)) {
>>
>> This looks strange... if the issue is that we don't want to warn in
>> cases of errors, the code should be reorganized in a more general way.
>
> I think when if conditionals have 1, 2 or 3 parts, I think they are fairly
> simple, though, I can read and keep track of conditionals that span 20+
> parts, thanks to gcc.  I do agree, there is an upper bound we should not
> cross, and I'd claim, 20 is far to large.  Where do people think the limit
> should be?
>
> Would a bool just above be better, or a predicate function that given the
> type and isRelational decides wether we should possibly warn?  I kinda like
> the function, if we split it out.  Or, is it the ! and the &&s that you find
> confusing.  Possibly you'd find !(a || (b && c)) to be more understandable?

The issue isn't that the condition is unreadable, but that we'd have
to add a bunch more conditions to catch all the cases.  For example,
we don't want to warn about "a<a" when a is complex or a struct, or
for '1i=="asdf"', etc.  Hence the suggestion to reorganize the code so
that if we print any error, we skip the warning.

-Eli