PriMee created this revision. Herald added a subscriber: JDevlieghere. Bug: https://bugs.llvm.org/show_bug.cgi?id=34450
**Problem:** Clang-tidy check misc-unused-parameters omits parameter default value what results in its complete removal. Compilation errors might occur after clang-tidy fix. **Patch description:** Changed removal range. The range should end after parameter declarator, not after whole parameter declaration (which might contain a default value). https://reviews.llvm.org/D37566 Files: clang-tidy/misc/UnusedParametersCheck.cpp test/clang-tidy/misc-unused-parameters.cpp Index: test/clang-tidy/misc-unused-parameters.cpp =================================================================== --- test/clang-tidy/misc-unused-parameters.cpp +++ test/clang-tidy/misc-unused-parameters.cpp @@ -14,15 +14,16 @@ void b(int i = 1) {} // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused [misc-unused-parameters] -// CHECK-FIXES: {{^}}void b(int /*i*/) {}{{$}} +// CHECK-FIXES: {{^}}void b(int /*i*/ = 1) {}{{$}} void c(int *i) {} // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: parameter 'i' is unused [misc-unused-parameters] // CHECK-FIXES: {{^}}void c(int * /*i*/) {}{{$}} // Unchanged cases // =============== -void g(int i); // Don't remove stuff in declarations +void f(int i); // Don't remove stuff in declarations +void g(int i = 1); void h(int i) { (void)i; } // Don't remove used parameters bool useLambda(int (*fn)(int)); @@ -52,6 +53,11 @@ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning // CHECK-FIXES: {{^}}static void staticFunctionE() +static void staticFunctionF(int i = 4); +// CHECK-FIXES: {{^}}static void staticFunctionF(); +static void staticFunctionF(int i) {} +// CHECK-MESSAGES: :[[@LINE-1]]:33: warning +// CHECK-FIXES: {{^}}static void staticFunctionF() static void someCallSites() { staticFunctionA(1); @@ -62,7 +68,10 @@ // CHECK-FIXES: staticFunctionC(2); staticFunctionD(1, 2, 3); // CHECK-FIXES: staticFunctionD(1, 3); - staticFunctionE(); + staticFunctionE(1); +// CHECK-FIXES: staticFunctionE(); + staticFunctionF(1); +// CHECK-FIXES: staticFunctionF(); } /* @@ -95,6 +104,9 @@ static void f(int i) {} // CHECK-MESSAGES: :[[@LINE-1]]:21: warning // CHECK-FIXES: static void f(int /*i*/) {} + static void g(int i = 1) {} +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning +// CHECK-FIXES: static void g(int /*i*/ = 1) {} }; namespace { @@ -108,6 +120,9 @@ void h(int i) {} // CHECK-MESSAGES: :[[@LINE-1]]:14: warning // CHECK-FIXES: void h(int /*i*/) {} + void s(int i = 1) {} +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning +// CHECK-FIXES: void s(int /*i*/ = 1) {} }; void C::f(int i) {} @@ -125,6 +140,7 @@ // CHECK-FIXES: c.g(); useFunction(&C::h); + useFunction(&C::s); } class Base { Index: clang-tidy/misc/UnusedParametersCheck.cpp =================================================================== --- clang-tidy/misc/UnusedParametersCheck.cpp +++ clang-tidy/misc/UnusedParametersCheck.cpp @@ -138,7 +138,8 @@ if (Function->isExternallyVisible() || !Result.SourceManager->isInMainFile(Function->getLocation()) || !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) { - SourceRange RemovalRange(Param->getLocation(), Param->getLocEnd()); + SourceRange RemovalRange(Param->getLocation(), + Param->DeclaratorDecl::getSourceRange().getEnd()); // 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.
Index: test/clang-tidy/misc-unused-parameters.cpp =================================================================== --- test/clang-tidy/misc-unused-parameters.cpp +++ test/clang-tidy/misc-unused-parameters.cpp @@ -14,15 +14,16 @@ void b(int i = 1) {} // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused [misc-unused-parameters] -// CHECK-FIXES: {{^}}void b(int /*i*/) {}{{$}} +// CHECK-FIXES: {{^}}void b(int /*i*/ = 1) {}{{$}} void c(int *i) {} // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: parameter 'i' is unused [misc-unused-parameters] // CHECK-FIXES: {{^}}void c(int * /*i*/) {}{{$}} // Unchanged cases // =============== -void g(int i); // Don't remove stuff in declarations +void f(int i); // Don't remove stuff in declarations +void g(int i = 1); void h(int i) { (void)i; } // Don't remove used parameters bool useLambda(int (*fn)(int)); @@ -52,6 +53,11 @@ // CHECK-MESSAGES: :[[@LINE-1]]:33: warning // CHECK-FIXES: {{^}}static void staticFunctionE() +static void staticFunctionF(int i = 4); +// CHECK-FIXES: {{^}}static void staticFunctionF(); +static void staticFunctionF(int i) {} +// CHECK-MESSAGES: :[[@LINE-1]]:33: warning +// CHECK-FIXES: {{^}}static void staticFunctionF() static void someCallSites() { staticFunctionA(1); @@ -62,7 +68,10 @@ // CHECK-FIXES: staticFunctionC(2); staticFunctionD(1, 2, 3); // CHECK-FIXES: staticFunctionD(1, 3); - staticFunctionE(); + staticFunctionE(1); +// CHECK-FIXES: staticFunctionE(); + staticFunctionF(1); +// CHECK-FIXES: staticFunctionF(); } /* @@ -95,6 +104,9 @@ static void f(int i) {} // CHECK-MESSAGES: :[[@LINE-1]]:21: warning // CHECK-FIXES: static void f(int /*i*/) {} + static void g(int i = 1) {} +// CHECK-MESSAGES: :[[@LINE-1]]:21: warning +// CHECK-FIXES: static void g(int /*i*/ = 1) {} }; namespace { @@ -108,6 +120,9 @@ void h(int i) {} // CHECK-MESSAGES: :[[@LINE-1]]:14: warning // CHECK-FIXES: void h(int /*i*/) {} + void s(int i = 1) {} +// CHECK-MESSAGES: :[[@LINE-1]]:14: warning +// CHECK-FIXES: void s(int /*i*/ = 1) {} }; void C::f(int i) {} @@ -125,6 +140,7 @@ // CHECK-FIXES: c.g(); useFunction(&C::h); + useFunction(&C::s); } class Base { Index: clang-tidy/misc/UnusedParametersCheck.cpp =================================================================== --- clang-tidy/misc/UnusedParametersCheck.cpp +++ clang-tidy/misc/UnusedParametersCheck.cpp @@ -138,7 +138,8 @@ if (Function->isExternallyVisible() || !Result.SourceManager->isInMainFile(Function->getLocation()) || !Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) { - SourceRange RemovalRange(Param->getLocation(), Param->getLocEnd()); + SourceRange RemovalRange(Param->getLocation(), + Param->DeclaratorDecl::getSourceRange().getEnd()); // 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.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits