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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
