From a6b5e0036128acfb0950f567cc83d351a5218fd6 Mon Sep 17 00:00:00 2001
From: Akira Hatanaka <ahatanaka@apple.com>
Date: Sat, 28 Jul 2018 04:06:13 +0000
Subject: [PATCH] [Sema][ObjC] Warn when a method declared in a protocol takes
 a non-escaping parameter but the implementation's method takes an escaping
 parameter.

rdar://problem/39548196

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

llvm-svn: 338189
---
 .../clang/Basic/DiagnosticSemaKinds.td        |  2 +
 clang/lib/Sema/SemaDeclObjC.cpp               | 48 ++++++++++++++++---
 clang/test/SemaObjCXX/noescape.mm             | 39 +++++++++++++++
 3 files changed, 82 insertions(+), 7 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b2ad27328a19f..8ad9049227eac 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1733,6 +1733,8 @@ def warn_overriding_method_missing_noescape : Warning<
   "__attribute__((noescape))">, InGroup<MissingNoEscape>;
 def note_overridden_marked_noescape : Note<
   "parameter of overridden method is annotated with __attribute__((noescape))">;
+def note_cat_conform_to_noescape_prot : Note<
+  "%select{category|class extension}0 conforms to protocol %1 which defines method %2">;
 
 def err_covariant_return_inaccessible_base : Error<
   "invalid covariant return for virtual function: %1 is a "
diff --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp
index e1b033ea82820..ab0ff0c4c9923 100644
--- a/clang/lib/Sema/SemaDeclObjC.cpp
+++ b/clang/lib/Sema/SemaDeclObjC.cpp
@@ -109,6 +109,30 @@ bool Sema::checkInitMethod(ObjCMethodDecl *method,
   return true;
 }
 
+/// Issue a warning if the parameter of the overridden method is non-escaping
+/// but the parameter of the overriding method is not.
+static bool diagnoseNoescape(const ParmVarDecl *NewD, const ParmVarDecl *OldD,
+                             Sema &S) {
+  if (OldD->hasAttr<NoEscapeAttr>() && !NewD->hasAttr<NoEscapeAttr>()) {
+    S.Diag(NewD->getLocation(), diag::warn_overriding_method_missing_noescape);
+    S.Diag(OldD->getLocation(), diag::note_overridden_marked_noescape);
+    return false;
+  }
+
+  return true;
+}
+
+/// Produce additional diagnostics if a category conforms to a protocol that
+/// defines a method taking a non-escaping parameter.
+static void diagnoseNoescape(const ParmVarDecl *NewD, const ParmVarDecl *OldD,
+                             const ObjCCategoryDecl *CD,
+                             const ObjCProtocolDecl *PD, Sema &S) {
+  if (!diagnoseNoescape(NewD, OldD, S))
+    S.Diag(CD->getLocation(), diag::note_cat_conform_to_noescape_prot)
+        << CD->IsClassExtension() << PD
+        << cast<ObjCMethodDecl>(NewD->getDeclContext());
+}
+
 void Sema::CheckObjCMethodOverride(ObjCMethodDecl *NewMethod, 
                                    const ObjCMethodDecl *Overridden) {
   if (Overridden->hasRelatedResultType() && 
@@ -192,13 +216,7 @@ void Sema::CheckObjCMethodOverride(ObjCMethodDecl *NewMethod,
       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);
   }
 }
 
@@ -4643,6 +4661,22 @@ Decl *Sema::ActOnMethodDeclaration(
             << 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, C, P, *this);
+          }
     }
   } else {
     cast<DeclContext>(ClassDecl)->addDecl(ObjCMethod);
diff --git a/clang/test/SemaObjCXX/noescape.mm b/clang/test/SemaObjCXX/noescape.mm
index 6c5d9897aaf03..9432a3a48a0b2 100644
--- a/clang/test/SemaObjCXX/noescape.mm
+++ b/clang/test/SemaObjCXX/noescape.mm
@@ -88,3 +88,42 @@ void test0() {
 
   S5<&noescapeFunc2> ne1;
 }
+
+@protocol NoescapeProt
+-(void) m0:(int*)__attribute__((noescape)) p; // expected-note 2 {{parameter of overridden method is annotated with __attribute__((noescape))}}
++(void) m1:(int*)__attribute__((noescape)) p;
+-(void) m1:(int*) p;
+@end
+
+__attribute__((objc_root_class))
+@interface C3
+-(void) m0:(int*) p;
++(void) m1:(int*)__attribute__((noescape)) p;
+-(void) m1:(int*) p;
+@end
+
+@interface C3 () <NoescapeProt> // expected-note {{class extension conforms to protocol 'NoescapeProt' which defines method 'm0:'}}
+@end
+
+@implementation C3
+-(void) m0:(int*) p { // expected-warning {{parameter of overriding method should be annotated with __attribute__((noescape))}}
+}
++(void) m1:(int*)__attribute__((noescape)) p {
+}
+-(void) m1:(int*) p {
+}
+@end
+
+__attribute__((objc_root_class))
+@interface C4 <NoescapeProt>
+-(void) m0:(int*) p; // expected-warning {{parameter of overriding method should be annotated with __attribute__((noescape))}}
+@end
+
+@implementation C4
+-(void) m0:(int*) p {
+}
++(void) m1:(int*)__attribute__((noescape)) p {
+}
+-(void) m1:(int*) p {
+}
+@end