]> granicus.if.org Git - clang/commitdiff
[Sema] More changes to fix Objective-C fallout from r249995.
authorBob Wilson <bob.wilson@apple.com>
Sat, 13 Feb 2016 01:41:41 +0000 (01:41 +0000)
committerBob Wilson <bob.wilson@apple.com>
Sat, 13 Feb 2016 01:41:41 +0000 (01:41 +0000)
This is a follow-up to PR26085. That was fixed in r257710 but the testcase
there was incomplete. There is a related issue where the overload resolution
for Objective-C incorrectly picks a method that is not valid without a
bridge cast. The call to Sema::CheckSingleAssignmentConstraints that was
added to SemaOverload.cpp's IsStandardConversion() function does not catch
that case and reports that the method is Compatible even when it is not.

The root cause here is that various Objective-C-related functions in Sema
do not consistently return a value to indicate whether there was an error.
This was fine in the past because they would report diagnostics when needed,
but r257710 changed them to suppress reporting diagnostics when checking
during overload resolution.

This patch adds a new ACR_error result to the ARCConversionResult enum and
updates Sema::CheckObjCARCConversion to return that value when there is an
error. Most of the calls to that function do not check the return value,
so adding this new result does not affect them. The one exception is in
SemaCast.cpp where it specifically checks for ACR_unbridged, so that is
also OK. The call in Sema::CheckSingleAssignmentConstraints can then check
for an ACR_okay result and identify assignments as Incompatible. To
preserve the existing behavior, it only changes the return value to
Incompatible when the new Diagnose argument (from r257710) is false.

Similarly, the CheckObjCBridgeRelatedConversions and
ConversionToObjCStringLiteralCheck need to identify when an assignment is
Incompatible. Those functions already return appropriate values but they
need some fixes related to the new Diagnose argument.

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

include/clang/Sema/Sema.h
lib/Sema/SemaExpr.cpp
lib/Sema/SemaExprObjC.cpp
test/SemaObjC/ovl-check.m

index 766bfa1e12f62c6f21ec38209ca4a73a1b8fd9a0..235f083ffa753e532bdc270645b7f109756439b2 100644 (file)
@@ -8654,7 +8654,7 @@ public:
                                         Expr *CastExpr,
                                         SourceLocation RParenLoc);
 
-  enum ARCConversionResult { ACR_okay, ACR_unbridged };
+  enum ARCConversionResult { ACR_okay, ACR_unbridged, ACR_error };
 
   /// \brief Checks for invalid conversions and casts between
   /// retainable pointers and other pointer kinds.
index cbbe79accf8a9b88f760fb280365da837c45c1c4..bde7cebafe3740d25fa34a5ec297ca6d74c0ca20 100644 (file)
@@ -7566,13 +7566,24 @@ Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &CallerRHS,
   if (result != Incompatible && RHS.get()->getType() != LHSType) {
     QualType Ty = LHSType.getNonLValueExprType(Context);
     Expr *E = RHS.get();
-    if (getLangOpts().ObjCAutoRefCount)
-      CheckObjCARCConversion(SourceRange(), Ty, E, CCK_ImplicitConversion,
-                             Diagnose, DiagnoseCFAudited);
+
+    // Check for various Objective-C errors. If we are not reporting
+    // diagnostics and just checking for errors, e.g., during overload
+    // resolution, return Incompatible to indicate the failure.
+    if (getLangOpts().ObjCAutoRefCount &&
+        CheckObjCARCConversion(SourceRange(), Ty, E, CCK_ImplicitConversion,
+                               Diagnose, DiagnoseCFAudited) != ACR_okay) {
+      if (!Diagnose)
+        return Incompatible;
+    }
     if (getLangOpts().ObjC1 &&
         (CheckObjCBridgeRelatedConversions(E->getLocStart(), LHSType,
                                            E->getType(), E, Diagnose) ||
          ConversionToObjCStringLiteralCheck(LHSType, E, Diagnose))) {
+      if (!Diagnose)
+        return Incompatible;
+      // Replace the expression with a corrected version and continue so we
+      // can find further errors.
       RHS = E;
       return Compatible;
     }
@@ -12035,10 +12046,11 @@ bool Sema::ConversionToObjCStringLiteralCheck(QualType DstType, Expr *&Exp,
   StringLiteral *SL = dyn_cast<StringLiteral>(SrcExpr);
   if (!SL || !SL->isAscii())
     return false;
-  if (Diagnose)
+  if (Diagnose) {
     Diag(SL->getLocStart(), diag::err_missing_atsign_prefix)
       << FixItHint::CreateInsertion(SL->getLocStart(), "@");
-  Exp = BuildObjCStringLiteral(SL->getLocStart(), SL).get();
+    Exp = BuildObjCStringLiteral(SL->getLocStart(), SL).get();
+  }
   return true;
 }
 
index b7e9ca87a279804d07a068b9e5e5b877d243fa73..68f9235b074f49d2d778d7f8922b8bfef14c9730 100644 (file)
@@ -3474,6 +3474,8 @@ diagnoseObjCARCConversion(Sema &S, SourceRange castRange,
     return;
 
   QualType castExprType = castExpr->getType();
+  // Defer emitting a diagnostic for bridge-related casts; that will be
+  // handled by CheckObjCBridgeRelatedConversions.
   TypedefNameDecl *TDNDecl = nullptr;
   if ((castACTC == ACTC_coreFoundation &&  exprACTC == ACTC_retainable &&
        ObjCBridgeRelatedAttrFromType(castType, TDNDecl)) ||
@@ -3916,16 +3918,16 @@ Sema::CheckObjCBridgeRelatedConversions(SourceLocation Loc,
           << FixItHint::CreateInsertion(SrcExprEndLoc, "]");
         Diag(RelatedClass->getLocStart(), diag::note_declared_at);
         Diag(TDNDecl->getLocStart(), diag::note_declared_at);
-      }
       
-      QualType receiverType = Context.getObjCInterfaceType(RelatedClass);
-      // Argument.
-      Expr *args[] = { SrcExpr };
-      ExprResult msg = BuildClassMessageImplicit(receiverType, false,
+        QualType receiverType = Context.getObjCInterfaceType(RelatedClass);
+        // Argument.
+        Expr *args[] = { SrcExpr };
+        ExprResult msg = BuildClassMessageImplicit(receiverType, false,
                                       ClassMethod->getLocation(),
                                       ClassMethod->getSelector(), ClassMethod,
                                       MultiExprArg(args, 1));
-      SrcExpr = msg.get();
+        SrcExpr = msg.get();
+      }
       return true;
     }
   }
@@ -3959,14 +3961,14 @@ Sema::CheckObjCBridgeRelatedConversions(SourceLocation Loc,
         }
         Diag(RelatedClass->getLocStart(), diag::note_declared_at);
         Diag(TDNDecl->getLocStart(), diag::note_declared_at);
-      }
       
-      ExprResult msg =
-        BuildInstanceMessageImplicit(SrcExpr, SrcType,
-                                     InstanceMethod->getLocation(),
-                                     InstanceMethod->getSelector(),
-                                     InstanceMethod, None);
-      SrcExpr = msg.get();
+        ExprResult msg =
+          BuildInstanceMessageImplicit(SrcExpr, SrcType,
+                                       InstanceMethod->getLocation(),
+                                       InstanceMethod->getSelector(),
+                                       InstanceMethod, None);
+        SrcExpr = msg.get();
+      }
       return true;
     }
   }
@@ -3990,9 +3992,9 @@ Sema::CheckObjCARCConversion(SourceRange castRange, QualType castType,
   ARCConversionTypeClass exprACTC = classifyTypeForARCConversion(castExprType);
   ARCConversionTypeClass castACTC = classifyTypeForARCConversion(effCastType);
   if (exprACTC == castACTC) {
-    // check for viablity and report error if casting an rvalue to a
+    // Check for viability and report error if casting an rvalue to a
     // life-time qualifier.
-    if (Diagnose && castACTC == ACTC_retainable &&
+    if (castACTC == ACTC_retainable &&
         (CCK == CCK_CStyleCast || CCK == CCK_OtherCast) &&
         castType != castExprType) {
       const Type *DT = castType.getTypePtr();
@@ -4008,10 +4010,12 @@ Sema::CheckObjCARCConversion(SourceRange castRange, QualType castType,
         QDT = AT->desugar();
       if (QDT != castType &&
           QDT.getObjCLifetime() !=  Qualifiers::OCL_None) {
-        SourceLocation loc =
-          (castRange.isValid() ? castRange.getBegin() 
-                              : castExpr->getExprLoc());
-        Diag(loc, diag::err_arc_nolifetime_behavior);
+        if (Diagnose) {
+          SourceLocation loc = (castRange.isValid() ? castRange.getBegin() 
+                                                    : castExpr->getExprLoc());
+          Diag(loc, diag::err_arc_nolifetime_behavior);
+        }
+        return ACR_error;
       }
     }
     return ACR_okay;
@@ -4059,24 +4063,26 @@ Sema::CheckObjCARCConversion(SourceRange castRange, QualType castType,
       CCK != CCK_ImplicitConversion)
     return ACR_unbridged;
 
-  // Do not issue bridge cast" diagnostic when implicit casting a cstring
-  // to 'NSString *'. Let caller issue a normal mismatched diagnostic with
-  // suitable fix-it.
+  // Issue a diagnostic about a missing @-sign when implicit casting a cstring
+  // to 'NSString *', instead of falling through to report a "bridge cast"
+  // diagnostic.
   if (castACTC == ACTC_retainable && exprACTC == ACTC_none &&
       ConversionToObjCStringLiteralCheck(castType, castExpr, Diagnose))
-    return ACR_okay;
+    return ACR_error;
   
   // Do not issue "bridge cast" diagnostic when implicit casting
   // a retainable object to a CF type parameter belonging to an audited
   // CF API function. Let caller issue a normal type mismatched diagnostic
   // instead.
-  if (Diagnose &&
-      (!DiagnoseCFAudited || exprACTC != ACTC_retainable ||
-        castACTC != ACTC_coreFoundation))
-    if (!(exprACTC == ACTC_voidPtr && castACTC == ACTC_retainable &&
-          (Opc == BO_NE || Opc == BO_EQ)))
+  if ((!DiagnoseCFAudited || exprACTC != ACTC_retainable ||
+       castACTC != ACTC_coreFoundation) &&
+      !(exprACTC == ACTC_voidPtr && castACTC == ACTC_retainable &&
+        (Opc == BO_NE || Opc == BO_EQ))) {
+    if (Diagnose)
       diagnoseObjCARCConversion(*this, castRange, castType, castACTC, castExpr,
                                 castExpr, exprACTC, CCK);
+    return ACR_error;
+  }
   return ACR_okay;
 }
 
index 7fc8a64049116c2967c5ee763313e735eac85347..cc2f094ad2ead9ce0a28577e0a7174abadb98b54 100644 (file)
@@ -4,20 +4,24 @@
 // in overload resolution in ObjC.
 
 struct Type1 { int a; };
+typedef const __attribute__((objc_bridge(id))) void * CFTypeRef;
 @interface Iface1 @end
 
 @interface NeverCalled
 - (void) test:(struct Type1 *)arg;
+- (void) test2:(CFTypeRef)arg;
 @end
 
 @interface TakesIface1
 - (void) test:(Iface1 *)arg;
+- (void) test2:(Iface1 *)arg;
 @end
 
 // PR26085, rdar://problem/24111333
 void testTakesIface1(id x, Iface1 *arg) {
   // This should resolve silently to `TakesIface1`.
   [x test:arg];
+  [x test2:arg];
 }
 
 @class NSString;
@@ -36,17 +40,16 @@ void testTakesNSString(id x) {
   [x testStr:"someStringLiteral"];
 }
 
-typedef const void *CFTypeRef;
 id CreateSomething();
 
-@interface NeverCalledv3
-- (void) testCFTypeRef:(struct Type1 *)arg;
-@end
-
 @interface TakesCFTypeRef
 - (void) testCFTypeRef:(CFTypeRef)arg;
 @end
 
+@interface NeverCalledv3
+- (void) testCFTypeRef:(struct Type1 *)arg;
+@end
+
 // Not called out explicitly by PR26085, but related.
 void testTakesCFTypeRef(id x) {
   // Overload resolution should occur silently, select the CFTypeRef overload,