[cfe-commits] r55767 - in /cfe/trunk: include/clang/AST/ASTContext.h include/clang/Basic/DiagnosticKinds.def lib/AST/ASTContext.cpp lib/Sema/Sema.h lib/Sema/SemaExpr.cpp

[steve naroff]

On Sep 4, 2008, at 3:14 PM, Eli Friedman wrote:

>> +  // If two types are identical, they are are compatible
>> +  if (lcanon == rcanon)
>> +    return true;
>> +  if (isa<FunctionType>(lcanon) && isa<FunctionType>(rcanon)) {
>> +    const FunctionType *lbase = cast<FunctionType>(lcanon);
>> +    const FunctionType *rbase = cast<FunctionType>(rcanon);
>> +
>> +    // First check the return types.
>> +    if (!typesAreBlockCompatible(lbase->getResultType(),rbase- 
>> >getResultType()))
>> +      return false;
>> +
>> +    // Return types matched, now check the argument types.
>> +    const FunctionTypeProto *lproto =  
>> dyn_cast<FunctionTypeProto>(lbase);
>> +    const FunctionTypeProto *rproto =  
>> dyn_cast<FunctionTypeProto>(rbase);
>> +
>> +    if (lproto && rproto) { // two C99 style function prototypes
>> +      unsigned lproto_nargs = lproto->getNumArgs();
>> +      unsigned rproto_nargs = rproto->getNumArgs();
>> +
>> +      if (lproto_nargs != rproto_nargs)
>> +        return false;
>> +
>> +      if (lproto->isVariadic() || rproto->isVariadic())
>> +        return false;
>> +
>> +      // The use of ellipsis agree...now check the argument types.
>> +      for (unsigned i = 0; i < lproto_nargs; i++)
>> +        if (!typesAreBlockCompatible(lproto->getArgType(i),
>> +                                     rproto->getArgType(i)))
>> +          return false;
>> +      return true;
>> +    }
>> +    return (!lproto && !rproto); // two K&R style function decls  
>> match.
>> +  }
>> +  return false;
>> +}
>
> It looks like you could delete all of the stuff after the "if (lcanon
> == rcanon) return true;" without affecting how the code works, unless
> I'm mistaken... for two types have identical return types and argument
> lists, they'd have to be identical anyways.
>

The code above enables us to diagnose the following as an error (which  
isn't considered an error in C):

   int (^emptyArgBlock)();
   int (^intArgBlock)(int) = emptyArgBlock;

I just did a commit which adds a large comment that explains this. For  
blocks, we decide to be more like C++ (and assume no argument list  
actually means no arguments, not any arguments). Make sense?

The comment also has a FIXME to fold the above semantics into  
mergeTypes. Do you agree?

> You need to modify ASTContext::mergeTypes to handle block pointer
> types; otherwise, I think it'll explode with an assertion.  Adding a
> "case Type::BlockPointer: return false;" should be sufficient, unless
> I'm misreading typesAreBlockCompatible.
>

I didn't do this and I'm not seeing any assertions. When I replace  
typesAreBlockCompatible with typesAreCompatible, I imagine I will have  
to add the clause you suggest.

Once I get all my test cases to pass, I will make sure I catch the  
other issues you mentioned...

snaroff