[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

Mike Stump mrs at apple.com
Thu May 7 11:53:05 PDT 2009

On May 6, 2009, at 9:11 PM, Eli Friedman wrote:
> On Wed, May 6, 2009 at 8:14 PM, Mike Stump <mrs at apple.com> wrote:
> This makes the handling for block pointers and function pointers
> inconsistent; is that the intent?

Yes.  block pointers are more akin to object pointers.  The model to  
use is a pointer to an object that itself contains a function pointer.

> (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:

   void (^bp)(int);
   const void *vp = bp;

   f(1 ? vp : bp);

into the compiler, it says the destType is void *.

I know what people _want_ it to do, but it doesn't appear that that  
has been done yet.  When it is, yes, quite possibly is it better to  
not have any qualifiers on it.

>> @@ -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?

>> @@ -3867,14 +3881,20 @@
>>   }
>>   // Handle block pointers.
>>   if (lType->isBlockPointerType() && rType->isIntegerType()) {
>> -    if (!RHSIsNull)
>> +    if (isRelational)
>> +      Diag(Loc, diag::err_typecheck_invalid_operands)
>> +        << lType << rType << lex->getSourceRange() << rex- 
>> >getSourceRange();
>> +    else if (!RHSIsNull)
> Couldn't you just check isRelational in the outer if statement, and
> fall through for the error case?

Ah, yes, that's much better, thanks.  Oh, actually, on second thought,  
this code is way to lax.  I've tightened it up.  Check out r71171.

More information about the cfe-commits mailing list