[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]

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