[Objective-C] Fix "weak-unavailable" warning with -fobjc-weak

Summary: clang should produce the same errors Objective-C classes that cannot be assigned to weak pointers under both -fobjc-arc and -fobjc-weak. Check for ObjCWeak along with ObjCAutoRefCount when analyzing pointer conversions. Add an -fobjc-weak pass to the existing arc-unavailable-for-weakref test cases to verify the behavior is the same.

Reviewers: rsmith, doug.gregor, rjmccall

Reviewed By: rjmccall

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D31006

llvm-svn: 299014

clang/include/clang/Basic/LangOptions.h

@@ -170,6 +170,11 @@ public:
   /// \brief Is this a libc/libm function that is no longer recognized as a
   /// builtin because a -fno-builtin-* option has been specified?
   bool isNoBuiltinFunc(StringRef Name) const;
+
+  /// \brief True if any ObjC types may have non-trivial lifetime qualifiers.
+  bool allowsNonTrivialObjCLifetimeQualifiers() const {
+    return ObjCAutoRefCount || ObjCWeak;
+  }
 };
 
 /// \brief Floating point control options

clang/include/clang/Sema/Sema.h

@@ -9346,14 +9346,14 @@ public:
   enum ARCConversionResult { ACR_okay, ACR_unbridged, ACR_error };
 
   /// \brief Checks for invalid conversions and casts between
-  /// retainable pointers and other pointer kinds.
+  /// retainable pointers and other pointer kinds for ARC and Weak.
-  ARCConversionResult CheckObjCARCConversion(SourceRange castRange,
+  ARCConversionResult CheckObjCConversion(SourceRange castRange,
-                                             QualType castType, Expr *&op,
+                                          QualType castType, Expr *&op,
-                                             CheckedConversionKind CCK,
+                                          CheckedConversionKind CCK,
-                                             bool Diagnose = true,
+                                          bool Diagnose = true,
-                                             bool DiagnoseCFAudited = false,
+                                          bool DiagnoseCFAudited = false,
-                                             BinaryOperatorKind Opc = BO_PtrMemD
+                                          BinaryOperatorKind Opc = BO_PtrMemD
-                                             );
+                                          );
 
   Expr *stripARCUnbridgedCast(Expr *e);
   void diagnoseARCUnbridgedCast(Expr *e);

clang/lib/Sema/SemaCast.cpp

@@ -120,12 +120,12 @@ namespace {
       Self.CheckCastAlign(SrcExpr.get(), DestType, OpRange);
     }
 
-    void checkObjCARCConversion(Sema::CheckedConversionKind CCK) {
+    void checkObjCConversion(Sema::CheckedConversionKind CCK) {
-      assert(Self.getLangOpts().ObjCAutoRefCount);
+      assert(Self.getLangOpts().allowsNonTrivialObjCLifetimeQualifiers());
 
       Expr *src = SrcExpr.get();
-      if (Self.CheckObjCARCConversion(OpRange, DestType, src, CCK) ==
+      if (Self.CheckObjCConversion(OpRange, DestType, src, CCK) ==
-            Sema::ACR_unbridged)
+          Sema::ACR_unbridged)
         IsARCUnbridgedCast = true;
       SrcExpr = src;
     }
@@ -872,7 +872,7 @@ void CastOperation::CheckReinterpretCast() {
     SrcExpr = ExprError();
   } else if (tcr == TC_Success) {
     if (Self.getLangOpts().ObjCAutoRefCount)
-      checkObjCARCConversion(Sema::CCK_OtherCast);
+      checkObjCConversion(Sema::CCK_OtherCast);
     DiagnoseReinterpretUpDownCast(Self, SrcExpr.get(), DestType, OpRange);
   }
 }
@@ -936,7 +936,7 @@ void CastOperation::CheckStaticCast() {
     if (Kind == CK_BitCast)
       checkCastAlign();
     if (Self.getLangOpts().ObjCAutoRefCount)
-      checkObjCARCConversion(Sema::CCK_OtherCast);
+      checkObjCConversion(Sema::CCK_OtherCast);
   } else if (Kind == CK_BitCast) {
     checkCastAlign();
   }
@@ -2272,8 +2272,9 @@ void CastOperation::CheckCXXCStyleCast(bool FunctionalStyle,
     }
   }
 
-  if (Self.getLangOpts().ObjCAutoRefCount && tcr == TC_Success)
+  if (Self.getLangOpts().allowsNonTrivialObjCLifetimeQualifiers() &&
-    checkObjCARCConversion(CCK);
+      tcr == TC_Success)
+    checkObjCConversion(CCK);
 
   if (tcr != TC_Success && msg != 0) {
     if (SrcExpr.get()->getType() == Self.Context.OverloadTy) {
@@ -2539,12 +2540,13 @@ void CastOperation::CheckCStyleCast() {
   }
 
   // ARC imposes extra restrictions on casts.
-  if (Self.getLangOpts().ObjCAutoRefCount) {
+  if (Self.getLangOpts().allowsNonTrivialObjCLifetimeQualifiers()) {
-    checkObjCARCConversion(Sema::CCK_CStyleCast);
+    checkObjCConversion(Sema::CCK_CStyleCast);
     if (SrcExpr.isInvalid())
       return;
-    
+
-    if (const PointerType *CastPtr = DestType->getAs<PointerType>()) {
+    const PointerType *CastPtr = DestType->getAs<PointerType>();
+    if (Self.getLangOpts().ObjCAutoRefCount && CastPtr) {
       if (const PointerType *ExprPtr = SrcType->getAs<PointerType>()) {
         Qualifiers CastQuals = CastPtr->getPointeeType().getQualifiers();
         Qualifiers ExprQuals = ExprPtr->getPointeeType().getQualifiers();

clang/lib/Sema/SemaExpr.cpp

@@ -7697,7 +7697,7 @@ Sema::CheckAssignmentConstraints(QualType LHSType, ExprResult &RHS,
       Kind = CK_BitCast;
       Sema::AssignConvertType result = 
         checkObjCPointerTypesForAssignment(*this, LHSType, RHSType);
-      if (getLangOpts().ObjCAutoRefCount &&
+      if (getLangOpts().allowsNonTrivialObjCLifetimeQualifiers() &&
           result == Compatible && 
           !CheckObjCARCUnavailableWeakConversion(OrigLHSType, RHSType))
         result = IncompatibleObjCWeakRef;
@@ -7904,7 +7904,7 @@ Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &CallerRHS,
       if (RHS.isInvalid())
         return Incompatible;
       Sema::AssignConvertType result = Compatible;
-      if (getLangOpts().ObjCAutoRefCount &&
+      if (getLangOpts().allowsNonTrivialObjCLifetimeQualifiers() &&
           !CheckObjCARCUnavailableWeakConversion(LHSType, RHSType))
         result = IncompatibleObjCWeakRef;
       return result;
@@ -7981,9 +7981,9 @@ Sema::CheckSingleAssignmentConstraints(QualType LHSType, ExprResult &CallerRHS,
     // 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 &&
+    if (getLangOpts().allowsNonTrivialObjCLifetimeQualifiers() &&
-        CheckObjCARCConversion(SourceRange(), Ty, E, CCK_ImplicitConversion,
+        CheckObjCConversion(SourceRange(), Ty, E, CCK_ImplicitConversion,
-                               Diagnose, DiagnoseCFAudited) != ACR_okay) {
+                            Diagnose, DiagnoseCFAudited) != ACR_okay) {
       if (!Diagnose)
         return Incompatible;
     }
@@ -9609,16 +9609,17 @@ QualType Sema::CheckCompareOperands(ExprResult &LHS, ExprResult &RHS,
       if (LHSIsNull && !RHSIsNull) {
         Expr *E = LHS.get();
         if (getLangOpts().ObjCAutoRefCount)
-          CheckObjCARCConversion(SourceRange(), RHSType, E, CCK_ImplicitConversion);
+          CheckObjCConversion(SourceRange(), RHSType, E,
+                              CCK_ImplicitConversion);
         LHS = ImpCastExprToType(E, RHSType,
                                 RPT ? CK_BitCast :CK_CPointerToObjCPointerCast);
       }
       else {
         Expr *E = RHS.get();
         if (getLangOpts().ObjCAutoRefCount)
-          CheckObjCARCConversion(SourceRange(), LHSType, E,
+          CheckObjCConversion(SourceRange(), LHSType, E, CCK_ImplicitConversion,
-                                 CCK_ImplicitConversion, /*Diagnose=*/true,
+                              /*Diagnose=*/true,
-                                 /*DiagnoseCFAudited=*/false, Opc);
+                              /*DiagnoseCFAudited=*/false, Opc);
         RHS = ImpCastExprToType(E, LHSType,
                                 LPT ? CK_BitCast :CK_CPointerToObjCPointerCast);
       }

clang/lib/Sema/SemaExprCXX.cpp

@@ -3726,10 +3726,9 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
       if (From->getType()->isObjCObjectPointerType() &&
           ToType->isObjCObjectPointerType())
         EmitRelatedResultTypeNote(From);
-    }
+    } else if (getLangOpts().allowsNonTrivialObjCLifetimeQualifiers() &&
-    else if (getLangOpts().ObjCAutoRefCount &&
+               !CheckObjCARCUnavailableWeakConversion(ToType,
-             !CheckObjCARCUnavailableWeakConversion(ToType,
+                                                      From->getType())) {
-                                                    From->getType())) {
       if (Action == AA_Initializing)
         Diag(From->getLocStart(),
              diag::err_arc_weak_unavailable_assign);
@@ -3752,8 +3751,8 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
       (void) PrepareCastToObjCObjectPointer(E);
       From = E.get();
     }
-    if (getLangOpts().ObjCAutoRefCount)
+    if (getLangOpts().allowsNonTrivialObjCLifetimeQualifiers())
-      CheckObjCARCConversion(SourceRange(), ToType, From, CCK);
+      CheckObjCConversion(SourceRange(), ToType, From, CCK);
     From = ImpCastExprToType(From, ToType, Kind, VK_RValue, &BasePath, CCK)
              .get();
     break;

clang/lib/Sema/SemaExprObjC.cpp

@@ -4108,11 +4108,10 @@ Sema::CheckObjCBridgeRelatedConversions(SourceLocation Loc,
 }
 
 Sema::ARCConversionResult
-Sema::CheckObjCARCConversion(SourceRange castRange, QualType castType,
+Sema::CheckObjCConversion(SourceRange castRange, QualType castType,
-                             Expr *&castExpr, CheckedConversionKind CCK,
+                          Expr *&castExpr, CheckedConversionKind CCK,
-                             bool Diagnose,
+                          bool Diagnose, bool DiagnoseCFAudited,
-                             bool DiagnoseCFAudited,
+                          BinaryOperatorKind Opc) {
-                             BinaryOperatorKind Opc) {
   QualType castExprType = castExpr->getType();
 
   // For the purposes of the classification, we assume reference types
@@ -4152,7 +4151,12 @@ Sema::CheckObjCARCConversion(SourceRange castRange, QualType castType,
     }
     return ACR_okay;
   }
-  
+
+  // The life-time qualifier cast check above is all we need for ObjCWeak.
+  // ObjCAutoRefCount has more restrictions on what is legal.
+  if (!getLangOpts().ObjCAutoRefCount)
+    return ACR_okay;
+
   if (isAnyCLike(exprACTC) && isAnyCLike(castACTC)) return ACR_okay;
 
   // Allow all of these types to be cast to integer types (but not

clang/lib/Sema/SemaPseudoObject.cpp

@@ -1126,8 +1126,8 @@ static void CheckKeyForObjCARCConversion(Sema &S, QualType ContainerT,
   if (!Getter)
     return;
   QualType T = Getter->parameters()[0]->getType();
-  S.CheckObjCARCConversion(Key->getSourceRange(), 
+  S.CheckObjCConversion(Key->getSourceRange(), T, Key,
-                         T, Key, Sema::CCK_ImplicitConversion);
+                        Sema::CCK_ImplicitConversion);
 }
 
 bool ObjCSubscriptOpBuilder::findAtIndexGetter() {

clang/test/SemaObjC/arc-unavailable-for-weakref.m

@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -verify -Wno-objc-root-class %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-weak -verify -Wno-objc-root-class %s
 // rdar://9693477
 
 __attribute__((objc_arc_weak_reference_unavailable))

clang/test/SemaObjCXX/arc-unavailable-for-weakref.mm

@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-arc -verify %s
+// RUN: %clang_cc1 -x objective-c++ -triple x86_64-apple-darwin11 -fobjc-runtime-has-weak -fsyntax-only -fobjc-weak -verify %s
 // rdar://9693477
 
 __attribute__((objc_arc_weak_reference_unavailable))