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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits