[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-26 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment. In D61147#1479956 , @rjmccall wrote: > In D61147#1479940 , @erik.pilkington > wrote: > > > In D61147#1479932 , @rjmccall > > wrote: > > >

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. In D61147#1479940 , @erik.pilkington wrote: > In D61147#1479932 , @rjmccall wrote: > > > I do not think the ObjC version of this diagnostic is useful as it's been > > explained here, and

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment. In D61147#1479932 , @rjmccall wrote: > 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.

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. 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.

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. Yes, I guess we can make `-Wunused-parameter ` cover unused parameters of pure @implementation methods too since users can probably just remove them to silence the warning. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61147/new/

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment. In D61147#1479530 , @ahatanak wrote: > In D61147#1479468 , @erik.pilkington > wrote: > > > > Yeah, I tend to think we should just suppress this unconditionally in > > >

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak added a comment. In D61147#1479468 , @erik.pilkington wrote: > > 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

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Sorry, yes, I meant in Objective-C methods, of course, not unconditionally even in C-like code whenever Objective-C is enabled. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61147/new/ https://reviews.llvm.org/D61147

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment. > 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

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment. Yeah, I tend to think we should just suppress this unconditionally in Objective-C. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61147/new/ https://reviews.llvm.org/D61147 ___ cfe-commits

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment. > 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

[PATCH] D61147: [Sema][ObjC] Add a flavor of -Wunused-parameter that doesn't diagnose unused parameters of ObjC methods

2019-04-25 Thread Akira Hatanaka via Phabricator via cfe-commits
ahatanak created this revision. ahatanak added reviewers: erik.pilkington, rjmccall. ahatanak added a project: clang. Herald added subscribers: dexonsmith, jkorous. The new warning `-Wunused-parameter-non-objc-method` works exactly the same as `-Wunused-parameter ` except for unused parameters