r293194 - [Sema][ObjC] Make sure -Wblock-capture-autoreleasing issues a warning

[Akira Hatanaka via cfe-commits]

Author: ahatanak
Date: Thu Jan 26 12:13:06 2017
New Revision: 293194

URL: http://llvm.org/viewvc/llvm-project?rev=293194&view=rev
Log:
[Sema][ObjC] Make sure -Wblock-capture-autoreleasing issues a warning
even in the presence of nullability qualifiers.

This commit fixes bugs in r285031 where -Wblock-capture-autoreleasing
wouldn't issue warnings when the function parameters were annotated
with nullability qualifiers. Specifically, look through the sugar and
see if there is an AttributedType of kind attr_objc_ownership to
determine whether __autoreleasing was explicitly specified or implicitly
added by the compiler.

rdar://problem/30193488

Modified:
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/SemaObjC/arc.m

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=293194&r1=293193&r2=293194&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Jan 26 12:13:06 2017
@@ -13603,11 +13603,28 @@ static bool captureInBlock(BlockScopeInf
   }
 
   // Warn about implicitly autoreleasing indirect parameters captured by blocks.
-  if (auto *PT = dyn_cast<PointerType>(CaptureType)) {
+  if (const auto *PT = CaptureType->getAs<PointerType>()) {
+    // This function finds out whether there is an AttributedType of kind
+    // attr_objc_ownership in Ty. The existence of AttributedType of kind
+    // attr_objc_ownership implies __autoreleasing was explicitly specified
+    // rather than being added implicitly by the compiler.
+    auto IsObjCOwnershipAttributedType = [](QualType Ty) {
+      while (const auto *AttrTy = Ty->getAs<AttributedType>()) {
+        if (AttrTy->getAttrKind() == AttributedType::attr_objc_ownership)
+          return true;
+
+        // Peel off AttributedTypes that are not of kind objc_ownership.
+        Ty = AttrTy->getModifiedType();
+      }
+
+      return false;
+    };
+
     QualType PointeeTy = PT->getPointeeType();
-    if (isa<ObjCObjectPointerType>(PointeeTy.getCanonicalType()) &&
+
+    if (PointeeTy->getAs<ObjCObjectPointerType>() &&
         PointeeTy.getObjCLifetime() == Qualifiers::OCL_Autoreleasing &&
-        !isa<AttributedType>(PointeeTy)) {
+        !IsObjCOwnershipAttributedType(PointeeTy)) {
       if (BuildAndDiagnose) {
         SourceLocation VarLoc = Var->getLocation();
         S.Diag(Loc, diag::warn_block_capture_autoreleasing);

Modified: cfe/trunk/test/SemaObjC/arc.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc.m?rev=293194&r1=293193&r2=293194&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/arc.m (original)
+++ cfe/trunk/test/SemaObjC/arc.m Thu Jan 26 12:13:06 2017
@@ -809,9 +809,30 @@ int garf() {
   TKAssertEqual(object, (id)nil);
 }
 
-void block_capture_autoreleasing(A * __autoreleasing *a, A **b) { // expected-note {{declare the parameter __autoreleasing explicitly to suppress this warning}} expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}}
+void block_capture_autoreleasing(A * __autoreleasing *a,
+                                 A **b, // expected-note {{declare the parameter __autoreleasing explicitly to suppress this warning}} expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}}
+                                 A * _Nullable *c, // expected-note {{declare the parameter __autoreleasing explicitly to suppress this warning}} expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}}
+                                 A * _Nullable __autoreleasing *d,
+                                 A ** _Nullable e, // expected-note {{declare the parameter __autoreleasing explicitly to suppress this warning}} expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}}
+                                 A * __autoreleasing * _Nullable f,
+                                 id __autoreleasing *g,
+                                 id *h, // expected-note {{declare the parameter __autoreleasing explicitly to suppress this warning}} expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}}
+                                 id _Nullable *i, // expected-note {{declare the parameter __autoreleasing explicitly to suppress this warning}} expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}}
+                                 id _Nullable __autoreleasing *j,
+                                 id * _Nullable k, // expected-note {{declare the parameter __autoreleasing explicitly to suppress this warning}} expected-note {{declare the parameter __strong or capture a __block __strong variable to keep values alive across autorelease pools}}
+                                 id __autoreleasing * _Nullable l) {
   ^{
     (void)*a;
     (void)*b; // expected-warning {{block captures an autoreleasing out-parameter, which may result in use-after-free bugs}}
+    (void)*c; // expected-warning {{block captures an autoreleasing out-parameter, which may result in use-after-free bugs}}
+    (void)*d;
+    (void)*e; // expected-warning {{block captures an autoreleasing out-parameter, which may result in use-after-free bugs}}
+    (void)*f;
+    (void)*g;
+    (void)*h; // expected-warning {{block captures an autoreleasing out-parameter, which may result in use-after-free bugs}}
+    (void)*i; // expected-warning {{block captures an autoreleasing out-parameter, which may result in use-after-free bugs}}
+    (void)*j;
+    (void)*k; // expected-warning {{block captures an autoreleasing out-parameter, which may result in use-after-free bugs}}
+    (void)*l;
   }();
 }