[PATCH] D63088: [clang-tidy] misc-unused-parameters: don't comment out parameter name for C code
Roman Lebedev via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 14 15:36:45 PDT 2019
lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.
Seems obviously correct, with the nit.
================
Comment at: clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp:141-156
+ // Cannot remove parameter for non-local functions.
if (Function->isExternallyVisible() ||
!Result.SourceManager->isInMainFile(Function->getLocation()) ||
!Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
- SourceRange RemovalRange(Param->getLocation());
- // Note: We always add a space before the '/*' to not accidentally create a
- // '*/*' for pointer types, which doesn't start a comment. clang-format will
- // clean this up afterwards.
- MyDiag << FixItHint::CreateReplacement(
- RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
+
+ // Comment out parameter name.
+ if (Result.Context->getLangOpts().CPlusPlus) {
----------------
I'd recommend to instead do less confusing
```
// Cannot remove parameter for non-local functions.
if (Function->isExternallyVisible() ||
!Result.SourceManager->isInMainFile(Function->getLocation()) ||
!Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
// It is illegal to omit parameter name here in C code, so early-out.
if (!Result.Context->getLangOpts().CPlusPlus)
return;
SourceRange RemovalRange(Param->getLocation());
// Note: We always add a space before the '/*' to not accidentally create
// a '*/*' for pointer types, which doesn't start a comment. clang-format
// will clean this up afterwards.
MyDiag << FixItHint::CreateReplacement(
RemovalRange, (Twine(" /*") + Param->getName() + "*/").str());
return;
}
```
================
Comment at: clang-tools-extra/test/clang-tidy/misc-unused-parameters.c:1-7
// RUN: %check_clang_tidy %s misc-unused-parameters %t -- -- -xc
// Basic removal
// =============
void a(int i) {;}
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused [misc-unused-parameters]
-// CHECK-FIXES: {{^}}void a(int /*i*/) {;}{{$}}
----------------
This screams a separate bug to me.
Does `%check_clang_tidy` not check that sources, after applying fix-it's, still parse?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63088/new/
https://reviews.llvm.org/D63088
More information about the cfe-commits
mailing list