Improve semantic checking for blocks. Radar 6441502

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@71145 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 277395cd7eba7217d28277a588eadef2984b45e2..7ac13d83fc02cf7875cd8f7610da93d1ab48f40c 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1039,6 +1039,8 @@ def ext_typecheck_comparison_of_pointer_integer : Warning<
   "comparison between pointer and integer (%0 and %1)">;
 def ext_typecheck_comparison_of_distinct_pointers : Warning<
   "comparison of distinct pointer types (%0 and %1)">;
+def ext_typecheck_cond_incompatible_operands : Warning<
+  "incompatible operand types (%0 and %1)">;
 def err_typecheck_comparison_of_distinct_pointers : Error<
   "comparison of distinct pointer types (%0 and %1)">;
 def err_typecheck_vector_comparison : Error<

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index d2833bc97c2b77a11d2513606f4d2abf19d4bb86..87bc037c58d88298324f2af0f93e8ae975cd76bf 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -2786,99 +2786,113 @@ QualType Sema::CheckConditionalOperands(Expr *&Cond, Expr *&LHS, Expr *&RHS,
     return RHSTy;
   }
 
+  const PointerType *LHSPT = LHSTy->getAsPointerType();
+  const PointerType *RHSPT = RHSTy->getAsPointerType();
+  const BlockPointerType *LHSBPT = LHSTy->getAsBlockPointerType();
+  const BlockPointerType *RHSBPT = RHSTy->getAsBlockPointerType();
+
   // Handle the case where both operands are pointers before we handle null
   // pointer constants in case both operands are null pointer constants.
-  if (const PointerType *LHSPT = LHSTy->getAsPointerType()) { // C99 6.5.15p3,6
-    if (const PointerType *RHSPT = RHSTy->getAsPointerType()) {
-      // get the "pointed to" types
-      QualType lhptee = LHSPT->getPointeeType();
-      QualType rhptee = RHSPT->getPointeeType();
-
-      // ignore qualifiers on void (C99 6.5.15p3, clause 6)
-      if (lhptee->isVoidType() &&
-          rhptee->isIncompleteOrObjectType()) {
-        // Figure out necessary qualifiers (C99 6.5.15p6)
-        QualType destPointee=lhptee.getQualifiedType(rhptee.getCVRQualifiers());
-        QualType destType = Context.getPointerType(destPointee);
-        ImpCastExprToType(LHS, destType); // add qualifiers if necessary
-        ImpCastExprToType(RHS, destType); // promote to void*
-        return destType;
-      }
-      if (rhptee->isVoidType() && lhptee->isIncompleteOrObjectType()) {
-        QualType destPointee=rhptee.getQualifiedType(lhptee.getCVRQualifiers());
-        QualType destType = Context.getPointerType(destPointee);
-        ImpCastExprToType(LHS, destType); // add qualifiers if necessary
-        ImpCastExprToType(RHS, destType); // promote to void*
-        return destType;
-      }
+  if ((LHSPT || LHSBPT) && (RHSPT || RHSBPT)) { // C99 6.5.15p3,6
+    // get the "pointed to" types
+    QualType lhptee = LHSPT ? LHSPT->getPointeeType() : LHSBPT->getPointeeType();
+    QualType rhptee = RHSPT ? RHSPT->getPointeeType() : RHSBPT->getPointeeType();
+
+    // ignore qualifiers on void (C99 6.5.15p3, clause 6)
+    if (lhptee->isVoidType()
+        && (RHSBPT || rhptee->isIncompleteOrObjectType())) {
+      // Figure out necessary qualifiers (C99 6.5.15p6)
+      QualType destPointee=lhptee.getQualifiedType(rhptee.getCVRQualifiers());
+      QualType destType = Context.getPointerType(destPointee);
+      ImpCastExprToType(LHS, destType); // add qualifiers if necessary
+      ImpCastExprToType(RHS, destType); // promote to void*
+      return destType;
+    }
+    if (rhptee->isVoidType()
+        && (LHSBPT || lhptee->isIncompleteOrObjectType())) {
+      QualType destPointee=rhptee.getQualifiedType(lhptee.getCVRQualifiers());
+      QualType destType = Context.getPointerType(destPointee);
+      ImpCastExprToType(LHS, destType); // add qualifiers if necessary
+      ImpCastExprToType(RHS, destType); // promote to void*
+      return destType;
+    }
 
-      if (Context.getCanonicalType(LHSTy) == Context.getCanonicalType(RHSTy)) {
-        // Two identical pointer types are always compatible.
-        return LHSTy;
-      }
+    bool sameKind = (LHSPT && RHSPT) || (LHSBPT && RHSBPT);
+    if (sameKind
+        && Context.getCanonicalType(LHSTy) == Context.getCanonicalType(RHSTy)) {
+      // Two identical pointer types are always compatible.
+      return LHSTy;
+    }
 
-      QualType compositeType = LHSTy;
-
-      // If either type is an Objective-C object type then check
-      // compatibility according to Objective-C.
-      if (Context.isObjCObjectPointerType(LHSTy) ||
-          Context.isObjCObjectPointerType(RHSTy)) {
-        // If both operands are interfaces and either operand can be
-        // assigned to the other, use that type as the composite
-        // type. This allows
-        //   xxx ? (A*) a : (B*) b
-        // where B is a subclass of A.
-        //
-        // Additionally, as for assignment, if either type is 'id'
-        // allow silent coercion. Finally, if the types are
-        // incompatible then make sure to use 'id' as the composite
-        // type so the result is acceptable for sending messages to.
-
-        // FIXME: Consider unifying with 'areComparableObjCPointerTypes'.
-        // It could return the composite type.
-        const ObjCInterfaceType* LHSIface = lhptee->getAsObjCInterfaceType();
-        const ObjCInterfaceType* RHSIface = rhptee->getAsObjCInterfaceType();
-        if (LHSIface && RHSIface &&
-            Context.canAssignObjCInterfaces(LHSIface, RHSIface)) {
-          compositeType = LHSTy;
-        } else if (LHSIface && RHSIface &&
-                   Context.canAssignObjCInterfaces(RHSIface, LHSIface)) {
-          compositeType = RHSTy;
-        } else if (Context.isObjCIdStructType(lhptee) ||
-                   Context.isObjCIdStructType(rhptee)) {
-          compositeType = Context.getObjCIdType();
-        } else {
-          Diag(QuestionLoc, diag::ext_typecheck_comparison_of_distinct_pointers)
-               << LHSTy << RHSTy
-               << LHS->getSourceRange() << RHS->getSourceRange();
-          QualType incompatTy = Context.getObjCIdType();
-          ImpCastExprToType(LHS, incompatTy);
-          ImpCastExprToType(RHS, incompatTy);
-          return incompatTy;
-        }
-      } else if (!Context.typesAreCompatible(lhptee.getUnqualifiedType(),
-                                             rhptee.getUnqualifiedType())) {
-        Diag(QuestionLoc, diag::warn_typecheck_cond_incompatible_pointers)
-          << LHSTy << RHSTy << LHS->getSourceRange() << RHS->getSourceRange();
-        // In this situation, we assume void* type. No especially good
-        // reason, but this is what gcc does, and we do have to pick
-        // to get a consistent AST.
-        QualType incompatTy = Context.getPointerType(Context.VoidTy);
+    QualType compositeType = LHSTy;
+
+    // If either type is an Objective-C object type then check
+    // compatibility according to Objective-C.
+    if (Context.isObjCObjectPointerType(LHSTy) ||
+        Context.isObjCObjectPointerType(RHSTy)) {
+      // If both operands are interfaces and either operand can be
+      // assigned to the other, use that type as the composite
+      // type. This allows
+      //   xxx ? (A*) a : (B*) b
+      // where B is a subclass of A.
+      //
+      // Additionally, as for assignment, if either type is 'id'
+      // allow silent coercion. Finally, if the types are
+      // incompatible then make sure to use 'id' as the composite
+      // type so the result is acceptable for sending messages to.
+
+      // FIXME: Consider unifying with 'areComparableObjCPointerTypes'.
+      // It could return the composite type.
+      const ObjCInterfaceType* LHSIface = lhptee->getAsObjCInterfaceType();
+      const ObjCInterfaceType* RHSIface = rhptee->getAsObjCInterfaceType();
+      if (LHSIface && RHSIface &&
+          Context.canAssignObjCInterfaces(LHSIface, RHSIface)) {
+        compositeType = LHSTy;
+      } else if (LHSIface && RHSIface &&
+                 Context.canAssignObjCInterfaces(RHSIface, LHSIface)) {
+        compositeType = RHSTy;
+      } else if (Context.isObjCIdStructType(lhptee) ||
+                 Context.isObjCIdStructType(rhptee)) {
+        compositeType = Context.getObjCIdType();
+      } else if (LHSBPT || RHSBPT) {
+        if (!sameKind
+            || !Context.typesAreBlockCompatible(lhptee.getUnqualifiedType(),
+                                                rhptee.getUnqualifiedType()))
+          Diag(QuestionLoc, diag::err_typecheck_cond_incompatible_operands)
+            << LHSTy << RHSTy << LHS->getSourceRange() << RHS->getSourceRange();
+        return QualType();
+      } else {
+        Diag(QuestionLoc, diag::ext_typecheck_cond_incompatible_operands)
+          << LHSTy << RHSTy
+          << LHS->getSourceRange() << RHS->getSourceRange();
+        QualType incompatTy = Context.getObjCIdType();
         ImpCastExprToType(LHS, incompatTy);
         ImpCastExprToType(RHS, incompatTy);
         return incompatTy;
       }
-      // The pointer types are compatible.
-      // C99 6.5.15p6: If both operands are pointers to compatible types *or* to
-      // differently qualified versions of compatible types, the result type is
-      // a pointer to an appropriately qualified version of the *composite*
-      // type.
-      // FIXME: Need to calculate the composite type.
-      // FIXME: Need to add qualifiers
-      ImpCastExprToType(LHS, compositeType);
-      ImpCastExprToType(RHS, compositeType);
-      return compositeType;
+    } else if (!sameKind
+               || !Context.typesAreCompatible(lhptee.getUnqualifiedType(),
+                                              rhptee.getUnqualifiedType())) {
+      Diag(QuestionLoc, diag::warn_typecheck_cond_incompatible_pointers)
+        << LHSTy << RHSTy << LHS->getSourceRange() << RHS->getSourceRange();
+      // In this situation, we assume void* type. No especially good
+      // reason, but this is what gcc does, and we do have to pick
+      // to get a consistent AST.
+      QualType incompatTy = Context.getPointerType(Context.VoidTy);
+      ImpCastExprToType(LHS, incompatTy);
+      ImpCastExprToType(RHS, incompatTy);
+      return incompatTy;
     }
+    // The pointer types are compatible.
+    // C99 6.5.15p6: If both operands are pointers to compatible types *or* to
+    // differently qualified versions of compatible types, the result type is
+    // a pointer to an appropriately qualified version of the *composite*
+    // type.
+    // FIXME: Need to calculate the composite type.
+    // FIXME: Need to add qualifiers
+    ImpCastExprToType(LHS, compositeType);
+    ImpCastExprToType(RHS, compositeType);
+    return compositeType;
   }
 
   // GCC compatibility: soften pointer/integer mismatch.
@@ -2895,11 +2909,6 @@ QualType Sema::CheckConditionalOperands(Expr *&Cond, Expr *&LHS, Expr *&RHS,
     return LHSTy;
   }
 
-  // Selection between block pointer types is ok as long as they are the same.
-  if (LHSTy->isBlockPointerType() && RHSTy->isBlockPointerType() &&
-      Context.getCanonicalType(LHSTy) == Context.getCanonicalType(RHSTy))
-    return LHSTy;
-
   // Need to handle "id<xx>" explicitly. Unlike "id", whose canonical type
   // evaluates to "struct objc_object *" (and is handled above when comparing
   // id with statically typed objects).
@@ -3671,7 +3680,7 @@ QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
   QualType lType = lex->getType();
   QualType rType = rex->getType();
 
-  if (!lType->isFloatingType()) {
+  if (!lType->isFloatingType() && !(lType->isBlockPointerType() && isRelational)) {
     // For non-floating point types, check for self-comparisons of the form
     // x == x, x != x, x < x, etc.  These always evaluate to a constant, and
     // often indicate logic errors in the program.
@@ -3795,7 +3804,7 @@ QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
     return ResultTy;
   }
   // Handle block pointer types.
-  if (lType->isBlockPointerType() && rType->isBlockPointerType()) {
+  if (!isRelational && lType->isBlockPointerType() && rType->isBlockPointerType()) {
     QualType lpointee = lType->getAsBlockPointerType()->getPointeeType();
     QualType rpointee = rType->getAsBlockPointerType()->getPointeeType();
 
@@ -3808,11 +3817,16 @@ QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
     return ResultTy;
   }
   // Allow block pointers to be compared with null pointer constants.
-  if ((lType->isBlockPointerType() && rType->isPointerType()) ||
-      (lType->isPointerType() && rType->isBlockPointerType())) {
+  if (!isRelational
+      && ((lType->isBlockPointerType() && rType->isPointerType())
+          || (lType->isPointerType() && rType->isBlockPointerType()))) {
     if (!LHSIsNull && !RHSIsNull) {
-      Diag(Loc, diag::err_typecheck_comparison_of_distinct_blocks)
-        << lType << rType << lex->getSourceRange() << rex->getSourceRange();
+      if (!((rType->isPointerType() && rType->getAsPointerType()
+             ->getPointeeType()->isVoidType())
+            || (lType->isPointerType() && lType->getAsPointerType()
+                ->getPointeeType()->isVoidType())))
+        Diag(Loc, diag::err_typecheck_comparison_of_distinct_blocks)
+          << lType << rType << lex->getSourceRange() << rex->getSourceRange();
     }
     ImpCastExprToType(rex, lType); // promote the pointer to pointer
     return ResultTy;
@@ -3867,14 +3881,20 @@ QualType Sema::CheckCompareOperands(Expr *&lex, Expr *&rex, SourceLocation Loc,
   }
   // 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)
       Diag(Loc, diag::ext_typecheck_comparison_of_pointer_integer)
         << lType << rType << lex->getSourceRange() << rex->getSourceRange();
     ImpCastExprToType(rex, lType); // promote the integer to pointer
     return ResultTy;
   }
   if (lType->isIntegerType() && rType->isBlockPointerType()) {
-    if (!LHSIsNull)
+    if (isRelational)
+      Diag(Loc, diag::err_typecheck_invalid_operands)
+        << lType << rType << lex->getSourceRange() << rex->getSourceRange();
+    else if (!LHSIsNull)
       Diag(Loc, diag::ext_typecheck_comparison_of_pointer_integer)
         << lType << rType << lex->getSourceRange() << rex->getSourceRange();
     ImpCastExprToType(lex, rType); // promote the integer to pointer

diff --git a/test/Sema/block-misc.c b/test/Sema/block-misc.c
index 6786f4d31e78d74141cbe738b435c75262b8c19e..9eda46420a8729dd097d7fe46b6ce543a5749850 100644 (file)
--- a/test/Sema/block-misc.c
+++ b/test/Sema/block-misc.c
@@ -156,3 +156,19 @@ void test16(__block int i) { // expected-error {{__block attribute not allowed,
   __block int a[size]; // expected-error {{__block attribute not allowed on declaration with a variably modified type}}
   __block int (*ap)[size]; // expected-error {{__block attribute not allowed on declaration with a variably modified type}}
 }
+
+void test17() {
+  void (^bp)(int);
+  void (*rp)(int);
+  void (^bp1)();
+  void *vp = bp;
+
+  f(1 ? bp : vp);
+  f(1 ? vp : bp);
+  f(1 ? bp : bp1); // expected-error {{incompatible operand types ('void (^)(int)' and 'void (^)()')}}
+  (void)(bp > rp); // expected-error {{invalid operands}}
+  (void)(bp > 0); // expected-error {{invalid operands}}
+  (void)(bp > bp); // expected-error {{invalid operands}}
+  (void)(bp > vp); // expected-error {{invalid operands}}
+  f(1 ? bp : rp); // expected-error {{incompatible operand types ('void (^)(int)' and 'void (*)(int)')}}
+}

diff --git a/test/SemaObjC/conditional-expr-2.m b/test/SemaObjC/conditional-expr-2.m
index 831f151cbe265e00d1e9435e817c62fb4bef9d63..08758488c540c99c91119cd943606a504dac5c38 100644 (file)
--- a/test/SemaObjC/conditional-expr-2.m
+++ b/test/SemaObjC/conditional-expr-2.m
@@ -8,7 +8,7 @@
 void f0(int cond, A *a, B *b) {
   // Ensure that we can still send a message to result of incompatible
   // conditional expression.
-  [ (cond ? a : b) test ]; // expected-warning{{comparison of distinct pointer types ('A *' and 'B *')}} expected-warning {{method '-test' not found}}
+  [ (cond ? a : b) test ]; // expected-warning{{incompatible operand types ('A *' and 'B *')}} expected-warning {{method '-test' not found}}
 }
 
 @interface NSKey @end
@@ -22,8 +22,8 @@ void foo (int i, NSKey *NSKeyValueCoding_NullValue, UpdatesList *nukedUpdatesLis
   NSKey *key;
   KeySub *keysub;
 
-  obj = i ? NSKeyValueCoding_NullValue : nukedUpdatesList; // expected-warning{{comparison of distinct pointer types ('NSKey *' and 'UpdatesList *')}}
-  key = i ? NSKeyValueCoding_NullValue : nukedUpdatesList; // expected-warning{{comparison of distinct pointer types ('NSKey *' and 'UpdatesList *')}}
+  obj = i ? NSKeyValueCoding_NullValue : nukedUpdatesList; // expected-warning{{incompatible operand types ('NSKey *' and 'UpdatesList *')}}
+  key = i ? NSKeyValueCoding_NullValue : nukedUpdatesList; // expected-warning{{incompatible operand types ('NSKey *' and 'UpdatesList *')}}
   key = i ? NSKeyValueCoding_NullValue : keysub;
   keysub = i ? NSKeyValueCoding_NullValue : keysub; // expected-warning{{incompatible pointer types assigning 'NSKey *', expected 'KeySub *'}}
 }