ahatanak updated this revision to Diff 157398.
ahatanak added a comment.
Add a test case for an interface conforming to a non-escaping protocol.
In https://reviews.llvm.org/D49119#1164285, @vsapsai wrote:
> Also I had a few ideas for tests when the warning isn't required and it is
> absent. But I'm not sure they are actually valuable. If you are interested,
> we can discuss it in more details.
Could you elaborate on what kind of tests you have in mind?
> Another feedback is an idea for potential improvement: have a note pointing
> to the place where protocol conformity is declared. Currently the warning
> looks like
>
> code_review.m:16:19: warning: parameter of overriding method should be
> annotated with __attribute__((noescape))
> [-Wmissing-noescape]
> -(void) m0:(int*) p { // expected-warning {{parameter of overriding method
> should be annotated with __attri...
> ^
> code_review.m:2:44: note: parameter of overridden method is annotated with
> __attribute__((noescape))
> -(void) m0:(int*)__attribute__((noescape)) p; // expected-note {{parameter
> of overridden method is annotate...
> ^
>
>
> and we can see the method both in implementation and in protocol. But in some
> cases it might be unclear where exactly that protocol was added to your
> class. I'm not sure this change is sufficiently useful, it's more for
> discussion.
I think it's possible to add a note that helps the user find where the category
conforming to the non-escaping protocol is declared.
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,30 @@
S5<&noescapeFunc2> ne1;
}
+
+@protocol NoescapeProt
+-(void) m0:(int*)__attribute__((noescape)) p; // expected-note 2 {{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
+
+__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 {
+}
+@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);
}
}
@@ -4643,6 +4647,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,30 @@
S5<&noescapeFunc2> ne1;
}
+
+@protocol NoescapeProt
+-(void) m0:(int*)__attribute__((noescape)) p; // expected-note 2 {{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
+
+__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 {
+}
+@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);
}
}
@@ -4643,6 +4647,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
