The warning isn't very useful when the function is an ObjC method as it generates lots of false positives.
- rG6bbe1e557621: [Sema][ObjC] Disable -Wunused-parameter for ObjC methods
rG12c69f04c5a2: [Sema][ObjC] Disable -Wunused-parameter for ObjC methods
rL359864: [Sema][ObjC] Disable -Wunused-parameter for ObjC methods
rG8cd01e69d8ee: [Sema][ObjC] Disable -Wunused-parameter for ObjC methods
rC359864: [Sema][ObjC] Disable -Wunused-parameter for ObjC methods
but don't want to annotate each unused ObjC method parameters with __attribute__((unused)) or insert (void)unused_param into the body of the method to silence the warning since, unlike C/C++ functions, it's not possible to comment out the unused parameters of ObjC methods.
Thats also true of function parameters in C (and Objective-C), so I don't think that argument is very convincing. I think it is true that this is a much less useful diagnostic for methods though, because they're more likely to be overriding something that requires a certain selector. Maybe we should just suppress this warning in general for methods (and even virtual functions in C++)?
Yeah, I tend to think we should just suppress this unconditionally in Objective-C.
IMO this warning still makes sense for normal functions, or methods that are only declared in an @implementation. Adding a fix-it to cast to void in the function/method body would probably go a long way too.
Should we use the new warning to warn just about unused ObjC method parameters and have -Wunused-parameter warn about everything else? If we make -Wunused-parameter unconditionally ignore ObjC methods, a user might complain about it later if -Wunused-parameter didn't diagnose an unused parameter when it should have.
Yeah, that seems fine to me, leave the method diagnostic off-by-default, since I doubt many would actually want it. Do you think its worth emitting -Wunused-parameter in pure-@implementation methods?
I do not think the ObjC version of this diagnostic is useful as it's been explained here, and I would strongly recommend just not including such a warning for the time being.
I would also recommend that you go fix the warning to never fire on virtual C++ methods.
Why? It seems to me like the vast majority of methods only declared in an @implementation are used as local helpers (even if people probably should be using functions for this), where this warning would be useful. Maybe I'm missing something? Still trying to learn more about Objective-C.
What rule are you suggesting for whether a method is "only declared in an @implementation"? Where exactly are you proposing to look for other declarations?
Objective-C does not have a formalized notion of an @implementation-private method, or really even an unambiguous convention for them (underscoring is *library*-private by convention, not class-private). The Objective-C ecosystem includes a lot of informal protocols and conventions based around discovery via naming, and Objective-C programmers do (admittedly rarely) just override methods that they know are there but which they can't see. These things make me very worried about trying to port assumptions from other languages.
Anyway, put that aside for a minute. As I understand it, your current proposal is to make -Wunused-parameter not fire on the majority of Objective-C methods and then add a new -Wunused-objc-parameter warning that's not really formally related to -Wunused-parameter. I think the right way to think about that is that the new warning is an independent feature which can happen at its own pace. You will need to do some investigation in order to implement that feature, so in the short term, you should just not warn about unused parameters in Objective-C methods, and then you can do the investigation for generalizing the warning as time permits.
Concretely, I was just thinking of using something like ObjCMethodDecl::isOverriding, but if that isn't a good enough heuristic for finding local methods then I guess we should just forgo methods entirely.
I tried building clang/llvm with -Wunused-parameter turned on and there are lots of places where parameters are unused other than virtual functions, in particular template functions and non-template functions that are called by other template functions. So disabling the warning on C++ virtual functions is probably not enough in order to make this warning more useful.