[clang-tools-extra] [clang-tidy] Fix misc-unused-parameters on params with attrs (PR #122286)

Maksim Ivanov via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 9 07:20:16 PST 2025


https://github.com/emaxx-google updated https://github.com/llvm/llvm-project/pull/122286

>From 3faba8c3df1901d9fb58bf0d087a2a832ac9f04a Mon Sep 17 00:00:00 2001
From: Maksim Ivanov <emaxx at google.com>
Date: Thu, 9 Jan 2025 14:59:35 +0000
Subject: [PATCH 1/2] [clang-tidy] Fix misc-unused-parameters on params with
 attrs

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.
---
 .../clang-tidy/misc/UnusedParametersCheck.cpp    | 16 ++++++++++++++++
 .../checkers/misc/unused-parameters.cpp          |  5 +++++
 2 files changed, 21 insertions(+)

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; });

>From e143d37705b748cd846a0f84b2dc34b9c1a8c0b0 Mon Sep 17 00:00:00 2001
From: Maksim Ivanov <emaxx at google.com>
Date: Thu, 9 Jan 2025 15:20:05 +0000
Subject: [PATCH 2/2] includes, namespace

---
 clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
index e7dfcd9859f64d..d792b5c52191f7 100644
--- a/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
@@ -9,8 +9,11 @@
 #include "UnusedParametersCheck.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTLambda.h"
+#include "clang/AST/Attr.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/ADT/STLExtras.h"
 #include <unordered_map>
@@ -27,7 +30,7 @@ bool isOverrideMethod(const FunctionDecl *Function) {
   return false;
 }
 
-bool hasAttrAfterParam(const clang::SourceManager *SourceManager,
+bool hasAttrAfterParam(const SourceManager *SourceManager,
                        const ParmVarDecl *Param) {
   for (const auto *Attr : Param->attrs()) {
     if (SourceManager->isBeforeInTranslationUnit(Param->getLocation(),



More information about the cfe-commits mailing list