[clang-tidy] Fix misc-unused-parameters on params with attrs

[emaxx-google]

Don't suggest to comment-out the parameter name if the parameter has an attribute that's spelled after the parameter name.

This prevents the parameter's attributes from being wrongly applied to the parameter's type.

This fixes #122191.

[llvmbot]

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clang-tidy

Author: Maksim Ivanov (emaxx-google)

Changes

Don't suggest to comment-out the parameter name if the parameter has an attribute that's spelled after the parameter name.

This prevents the parameter's attributes from being wrongly applied to the parameter's type (#122191).

---

Full diff: https://github.com/llvm/llvm-project/pull/122286.diff

2 Files Affected:

• (modified) clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp (+16)
• (modified) clang-tools-extra/test/clang-tidy/checkers/misc/unused-parameters.cpp (+5)

diff --git a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
index c2d9286312dc4a..e7dfcd9859f64d 100644
--- a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -26,6 +26,17 @@ bool isOverrideMethod(const FunctionDecl *Function) {
     return MD->size_overridden_methods() > 0 || MD->hasAttr<OverrideAttr>();
   return false;
 }
+
+bool hasAttrAfterParam(const clang::SourceManager *SourceManager,
+                       const ParmVarDecl *Param) {
+  for (const auto *Attr : Param->attrs()) {
+    if (SourceManager->isBeforeInTranslationUnit(Param->getLocation(),
+                                                 Attr->getLocation())) {
+      return true;
+    }
+  }
+  return false;
+}
 } // namespace
 
 void UnusedParametersCheck::registerMatchers(MatchFinder *Finder) {
@@ -189,6 +200,11 @@ void UnusedParametersCheck::check(const MatchFinder::MatchResult &Result) {
     if (Param->isUsed() || Param->isReferenced() || !Param->getDeclName() ||
         Param->hasAttr<UnusedAttr>())
       continue;
+    if (hasAttrAfterParam(Result.SourceManager, Param)) {
+      // Due to how grammar works, attributes would be wrongly applied to the
+      // type if we remove the preceding parameter name.
+      continue;
+    }
 
     // In non-strict mode ignore function definitions with empty bodies
     // (constructor initializer counts for non-empty body).
diff --git a/clang-tools-extra/test/clang-tidy/checkers/misc/unused-parameters.cpp b/clang-tools-extra/test/clang-tidy/checkers/misc/unused-parameters.cpp
index a3fcf30f273ef7..86b00ee56c3ce0 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/unused-parameters.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/unused-parameters.cpp
@@ -33,6 +33,10 @@ void f(void (*fn)()) {;}
 // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: parameter 'fn' is unused [misc-unused-parameters]
 // CHECK-FIXES: {{^}}void f(void (* /*fn*/)()) {;}{{$}}
 
+int *k([[clang::lifetimebound]] int *i) {;}
+// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: parameter 'i' is unused [misc-unused-parameters]
+// CHECK-FIXES: {{^}}int *k({{\[\[clang::lifetimebound\]\]}} int * /*i*/) {;}{{$}}
+
 // Unchanged cases
 // ===============
 void f(int i); // Don't remove stuff in declarations
@@ -41,6 +45,7 @@ void h(int i[]);
 void s(int i[1]);
 void u(void (*fn)());
 void w(int i) { (void)i; } // Don't remove used parameters
+int *x(int *i [[clang::lifetimebound]]) { return nullptr; } // Don't reanchor attribute to the type.
 
 bool useLambda(int (*fn)(int));
 static bool static_var = useLambda([] (int a) { return a; });

[emaxx-google]

@usx95

[usx95]

Thanks. LGTM.