ahatanak created this revision.
ahatanak added reviewers: arphaman, vsapsai, dcoughlin.
ahatanak added a project: clang.
Herald added a subscriber: dexonsmith.

This patch makes Sema issue a warning when there is an extension that conforms 
to a protocol that declares a non-escaping method and the corresponding method 
in the implementation isn't non-escaping.

rdar://problem/39548196


Repository:
  rC Clang

https://reviews.llvm.org/D49119

Files:
  lib/Sema/SemaDeclObjC.cpp
  test/SemaObjCXX/noescape.mm


Index: test/SemaObjCXX/noescape.mm
===================================================================
--- test/SemaObjCXX/noescape.mm
+++ test/SemaObjCXX/noescape.mm
@@ -88,3 +88,20 @@
 
   S5<&noescapeFunc2> ne1;
 }
+
+@protocol NoescapeProt
+-(void) m0:(int*)__attribute__((noescape)) p; // expected-note {{parameter of 
overridden method is annotated with __attribute__((noescape))}}
+@end
+
+__attribute__((objc_root_class))
+@interface C3
+-(void) m0:(int*) p;
+@end
+
+@interface C3 () <NoescapeProt>
+@end
+
+@implementation C3
+-(void) m0:(int*) p { // expected-warning {{parameter of overriding method 
should be annotated with __attribute__((noescape))}}
+}
+@end
Index: lib/Sema/SemaDeclObjC.cpp
===================================================================
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -109,6 +109,16 @@
   return true;
 }
 
+static void diagnoseNoescape(const ParmVarDecl *NewMD, const ParmVarDecl 
*OldMD,
+                             Sema &S) {
+  // A parameter of the overriding method should be annotated with noescape
+  // if the corresponding parameter of the overridden method is annotated.
+  if (OldMD->hasAttr<NoEscapeAttr>() && !NewMD->hasAttr<NoEscapeAttr>()) {
+    S.Diag(NewMD->getLocation(), 
diag::warn_overriding_method_missing_noescape);
+    S.Diag(OldMD->getLocation(), diag::note_overridden_marked_noescape);
+  }
+}
+
 void Sema::CheckObjCMethodOverride(ObjCMethodDecl *NewMethod, 
                                    const ObjCMethodDecl *Overridden) {
   if (Overridden->hasRelatedResultType() && 
@@ -192,13 +202,7 @@
       Diag(oldDecl->getLocation(), diag::note_previous_decl) << "parameter";
     }
 
-    // A parameter of the overriding method should be annotated with noescape
-    // if the corresponding parameter of the overridden method is annotated.
-    if (oldDecl->hasAttr<NoEscapeAttr>() && !newDecl->hasAttr<NoEscapeAttr>()) 
{
-      Diag(newDecl->getLocation(),
-           diag::warn_overriding_method_missing_noescape);
-      Diag(oldDecl->getLocation(), diag::note_overridden_marked_noescape);
-    }
+    diagnoseNoescape(newDecl, oldDecl, *this);
   }
 }
 
@@ -4663,6 +4667,22 @@
             << ObjCMethod->getDeclName();
         }
       }
+
+      // Warn if a method declared in a protocol to which a category or
+      // extension conforms is non-escaping and the implementation's method is
+      // escaping.
+      for (auto *C : IDecl->visible_categories())
+        for (auto &P : C->protocols())
+          if (auto *IMD = P->lookupMethod(ObjCMethod->getSelector(),
+                                          ObjCMethod->isInstanceMethod())) {
+            assert(ObjCMethod->parameters().size() ==
+                       IMD->parameters().size() &&
+                   "Methods have different number of parameters");
+            auto OI = IMD->param_begin(), OE = IMD->param_end();
+            auto NI = ObjCMethod->param_begin();
+            for (; OI != OE; ++OI, ++NI)
+              diagnoseNoescape(*NI, *OI, *this);
+          }
     }
   } else {
     cast<DeclContext>(ClassDecl)->addDecl(ObjCMethod);


Index: test/SemaObjCXX/noescape.mm
===================================================================
--- test/SemaObjCXX/noescape.mm
+++ test/SemaObjCXX/noescape.mm
@@ -88,3 +88,20 @@
 
   S5<&noescapeFunc2> ne1;
 }
+
+@protocol NoescapeProt
+-(void) m0:(int*)__attribute__((noescape)) p; // expected-note {{parameter of overridden method is annotated with __attribute__((noescape))}}
+@end
+
+__attribute__((objc_root_class))
+@interface C3
+-(void) m0:(int*) p;
+@end
+
+@interface C3 () <NoescapeProt>
+@end
+
+@implementation C3
+-(void) m0:(int*) p { // expected-warning {{parameter of overriding method should be annotated with __attribute__((noescape))}}
+}
+@end
Index: lib/Sema/SemaDeclObjC.cpp
===================================================================
--- lib/Sema/SemaDeclObjC.cpp
+++ lib/Sema/SemaDeclObjC.cpp
@@ -109,6 +109,16 @@
   return true;
 }
 
+static void diagnoseNoescape(const ParmVarDecl *NewMD, const ParmVarDecl *OldMD,
+                             Sema &S) {
+  // A parameter of the overriding method should be annotated with noescape
+  // if the corresponding parameter of the overridden method is annotated.
+  if (OldMD->hasAttr<NoEscapeAttr>() && !NewMD->hasAttr<NoEscapeAttr>()) {
+    S.Diag(NewMD->getLocation(), diag::warn_overriding_method_missing_noescape);
+    S.Diag(OldMD->getLocation(), diag::note_overridden_marked_noescape);
+  }
+}
+
 void Sema::CheckObjCMethodOverride(ObjCMethodDecl *NewMethod, 
                                    const ObjCMethodDecl *Overridden) {
   if (Overridden->hasRelatedResultType() && 
@@ -192,13 +202,7 @@
       Diag(oldDecl->getLocation(), diag::note_previous_decl) << "parameter";
     }
 
-    // A parameter of the overriding method should be annotated with noescape
-    // if the corresponding parameter of the overridden method is annotated.
-    if (oldDecl->hasAttr<NoEscapeAttr>() && !newDecl->hasAttr<NoEscapeAttr>()) {
-      Diag(newDecl->getLocation(),
-           diag::warn_overriding_method_missing_noescape);
-      Diag(oldDecl->getLocation(), diag::note_overridden_marked_noescape);
-    }
+    diagnoseNoescape(newDecl, oldDecl, *this);
   }
 }
 
@@ -4663,6 +4667,22 @@
             << ObjCMethod->getDeclName();
         }
       }
+
+      // Warn if a method declared in a protocol to which a category or
+      // extension conforms is non-escaping and the implementation's method is
+      // escaping.
+      for (auto *C : IDecl->visible_categories())
+        for (auto &P : C->protocols())
+          if (auto *IMD = P->lookupMethod(ObjCMethod->getSelector(),
+                                          ObjCMethod->isInstanceMethod())) {
+            assert(ObjCMethod->parameters().size() ==
+                       IMD->parameters().size() &&
+                   "Methods have different number of parameters");
+            auto OI = IMD->param_begin(), OE = IMD->param_end();
+            auto NI = ObjCMethod->param_begin();
+            for (; OI != OE; ++OI, ++NI)
+              diagnoseNoescape(*NI, *OI, *this);
+          }
     }
   } else {
     cast<DeclContext>(ClassDecl)->addDecl(ObjCMethod);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to