[cfe-commits] r125445 - in /cfe/trunk: include/clang/Sema/Overload.h include/clang/Sema/Sema.h lib/Sema/SemaExprCXX.cpp lib/Sema/SemaOverload.cpp test/CodeGenObjCXX/blocks.mm test/SemaCXX/block-call.cpp test/SemaObjCXX/blocks.mm

[Douglas Gregor]

On Feb 12, 2011, at 11:07 AM, Fariborz Jahanian wrote:

> Author: fjahanian
> Date: Sat Feb 12 13:07:46 2011
> New Revision: 125445
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=125445&view=rev
> Log:
> Implement objective-c++'s block pointer type matching involving
> types which are contravariance in argument types and covariance
> in return types. // rdar://8979379.

Very nice! A few comments inline.

> Added:
>    cfe/trunk/test/CodeGenObjCXX/blocks.mm
>    cfe/trunk/test/SemaCXX/block-call.cpp
> Modified:
>    cfe/trunk/include/clang/Sema/Overload.h
>    cfe/trunk/include/clang/Sema/Sema.h
>    cfe/trunk/lib/Sema/SemaExprCXX.cpp
>    cfe/trunk/lib/Sema/SemaOverload.cpp
>    cfe/trunk/test/SemaObjCXX/blocks.mm
> 
> Modified: cfe/trunk/include/clang/Sema/Overload.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Overload.h?rev=125445&r1=125444&r2=125445&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Overload.h (original)
> +++ cfe/trunk/include/clang/Sema/Overload.h Sat Feb 12 13:07:46 2011
> @@ -75,6 +75,7 @@
>     ICK_Vector_Conversion,     ///< Vector conversions
>     ICK_Vector_Splat,          ///< A vector splat from an arithmetic type
>     ICK_Complex_Real,          ///< Complex-real conversions (C99 6.3.1.7)
> +    ICK_Block_Pointer_Conversion,    ///< Block Pointer conversions 
>     ICK_Num_Conversion_Kinds   ///< The number of conversion kinds
>   };
> 
> 
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=125445&r1=125444&r2=125445&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Sat Feb 12 13:07:46 2011
> @@ -1019,6 +1019,8 @@
>                            QualType& ConvertedType, bool &IncompatibleObjC);
>   bool isObjCPointerConversion(QualType FromType, QualType ToType,
>                                QualType& ConvertedType, bool &IncompatibleObjC);
> +  bool IsBlockPointerConversion(QualType FromType, QualType ToType,
> +                                QualType& ConvertedType);
>   bool FunctionArgTypesAreEqual(const FunctionProtoType *OldType, 
>                                 const FunctionProtoType *NewType);
> 
> 
> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=125445&r1=125444&r2=125445&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Sat Feb 12 13:07:46 2011
> @@ -2199,7 +2199,12 @@
>       }
>     }
>     break;
> -
> +  
> +  case ICK_Block_Pointer_Conversion: {
> +      ImpCastExprToType(From, ToType.getUnqualifiedType(), CK_BitCast, VK_RValue);
> +      break;
> +    }
> +      
>   case ICK_Lvalue_To_Rvalue:
>   case ICK_Array_To_Pointer:
>   case ICK_Function_To_Pointer:
> 
> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=125445&r1=125444&r2=125445&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Sat Feb 12 13:07:46 2011
> @@ -1166,6 +1166,8 @@
>     // Floating-integral conversions (C++ 4.9).
>     SCS.Second = ICK_Floating_Integral;
>     FromType = ToType.getUnqualifiedType();
> +  } else if (S.IsBlockPointerConversion(FromType, ToType, FromType)) {
> +               SCS.Second = ICK_Block_Pointer_Conversion;
>   } else if (S.IsPointerConversion(From, FromType, ToType, InOverloadResolution,
>                                    FromType, IncompatibleObjC)) {
>     // Pointer conversions (C++ 4.10).
> @@ -1782,6 +1784,92 @@
>   return false;
> }
> 
> +bool Sema::IsBlockPointerConversion(QualType FromType, QualType ToType,
> +                                    QualType& ConvertedType) {
> +  QualType ToPointeeType;
> +  if (const BlockPointerType *ToBlockPtr =
> +        ToType->getAs<BlockPointerType>())
> +    ToPointeeType = ToBlockPtr->getPointeeType();
> +  else
> +    return false;
> +  
> +  QualType FromPointeeType;
> +  if (const BlockPointerType *FromBlockPtr =
> +      FromType->getAs<BlockPointerType>())
> +    FromPointeeType = FromBlockPtr->getPointeeType();
> +  else
> +    return false;
> +  // We have pointer to blocks, check whether the only
> +  // differences in the argument and result types are in Objective-C
> +  // pointer conversions. If so, we permit the conversion.
> +  
> +  const FunctionProtoType *FromFunctionType
> +    = FromPointeeType->getAs<FunctionProtoType>();
> +  const FunctionProtoType *ToFunctionType
> +    = ToPointeeType->getAs<FunctionProtoType>();
> +  
> +  if (FromFunctionType && ToFunctionType) {
> +    if (Context.getCanonicalType(FromPointeeType)
> +          == Context.getCanonicalType(ToPointeeType))
> +      return true;
> +    
> +    // Perform the quick checks that will tell us whether these
> +    // function types are obviously different.
> +    if (FromFunctionType->getNumArgs() != ToFunctionType->getNumArgs() ||
> +        FromFunctionType->isVariadic() != ToFunctionType->isVariadic() ||
> +        FromFunctionType->getTypeQuals() != ToFunctionType->getTypeQuals())
> +      return false;

The getTypeQuals() check isn't necessary, since block pointers can't have qualifiers on the function type itself.

> +    bool IncompatibleObjC = false;
> +    if (Context.getCanonicalType(FromFunctionType->getResultType())
> +          == Context.getCanonicalType(ToFunctionType->getResultType())) {
> +      // Okay, the types match exactly. Nothing to do.
> +    } else {
> +      QualType RHS = FromFunctionType->getResultType();
> +      QualType CanRHS = Context.getCanonicalType(RHS);
> +      QualType LHS = ToFunctionType->getResultType();
> +      QualType CanLHS = Context.getCanonicalType(LHS);
> +      if (!CanRHS->isRecordType() &&
> +          !CanRHS.hasQualifiers() && CanLHS.hasQualifiers())
> +        CanLHS = CanLHS.getUnqualifiedType();

FWIW, you don't actually have to operate on the canonical types here; hasQualifiers() and getUnqualifiedType() do the right thing on non-canonical types, too.

Also, due to attribute overloadable, we can actually end up here in C code... so the !CanRHS->isRecordType() check should be (!getLangOptions().CPlusPlus || !CanRHS->isRecordType()).

> +      if (Context.getCanonicalType(CanRHS)
> +          == Context.getCanonicalType(CanLHS)) {
> +        // OK exact match.

Context.hasSameType() is often easier to read (and works on non-canonical types).

> +    // Check argument types.
> +    for (unsigned ArgIdx = 0, NumArgs = FromFunctionType->getNumArgs();
> +         ArgIdx != NumArgs; ++ArgIdx) {
> +      IncompatibleObjC = false;
> +      QualType FromArgType = FromFunctionType->getArgType(ArgIdx);
> +      QualType ToArgType = ToFunctionType->getArgType(ArgIdx);
> +      if (Context.getCanonicalType(FromArgType)
> +            == Context.getCanonicalType(ToArgType)) {
> +        // Okay, the types match exactly. Nothing to do.

Same comment about Context.hasSameType() here.

  - Doug