[PATCH] D49119: [Sema][ObjC] Issue a warning when a method declared in a protocol is non-escaping but the corresponding method in the implementation is escaping.
Volodymyr Sapsai via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 16 14:57:37 PDT 2018
vsapsai added a comment.
Overall looks good to me. Maybe add a test when a protocol is declared for an interface, not for a category. Something like
__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
Looks like there is no such test already and it seems different enough to be worth adding. The idea for this test was triggered by code `for (auto *C : IDecl->visible_categories())` and the goal is to cover a case when protocol is not for a category.
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.
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.
Repository:
rC Clang
https://reviews.llvm.org/D49119
More information about the cfe-commits
mailing list