[PATCH] D82611: [SemaObjC] Add a warning for @selector expressions that potentially refer to objc_direct methods

Erik Pilkington via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 6 12:54:30 PDT 2020


erik.pilkington added a comment.

In D82611#2125868 <https://reviews.llvm.org/D82611#2125868>, @MadCoder wrote:

> So the risk with that one is that if someone had say the `-didSomething` direct selector and that some UIKit/Apple SDK API now adds this as a thing that you use with CFNotification center for example, where you _need_ dynamism, then the uses of the API would warn all the time when the internal to the client project `-didSomething` is in scope.
>
> So we ideally need a way to silence this warning, so at the very least this needs to be a new `-W` flag (on by default (?)) so that callers that have an issue can use a `_Pragma` at the call site to ignore the error for when it's legal.


The `DiagGroup<"potentially-direct-selector">` creates a command line flag, so the pragma thing works. I couldn't think of a decent way of suppressing this without pragmas, but ISTM that this will have a low enough false-positive rate that it won't be a big problem.

> I would suggest something like `-Wstrict-direct-dispatch` or something.

I kinda prefer `-Wpotentially-direct-selector`, since that seems to more closely correspond to what the compiler is complaining about. WDYT?



================
Comment at: clang/test/SemaObjC/potentially-direct-selector.m:84
+  (void)@selector(inBaseCat);
+  (void)@selector(inBaseCatImpl);
+  (void)@selector(inDerived);
----------------
MadCoder wrote:
> I think this might be too weak, if we add a warning then maybe what we want is a tri-state:
> 
> `-Wstrict-direct-dispatch=none|self|all`
> 
> because I can see cases where the heuristic here would be too weak
Sure - new patch adds an off-by-default "strict" variant, since I think that's a bit more common in clang then the `-Wx=y` approach.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82611/new/

https://reviews.llvm.org/D82611





More information about the cfe-commits mailing list