[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