Skip to content

Commit

Permalink
[clang-tidy] Fix misc-unused-parameters on params with attrs (llvm#12…
Browse files Browse the repository at this point in the history
…2286)

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 llvm#122191.
  • Loading branch information
emaxx-google authored Jan 9, 2025
1 parent 876841b commit 1b897f7
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 0 deletions.
19 changes: 19 additions & 0 deletions clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -26,6 +29,17 @@ bool isOverrideMethod(const FunctionDecl *Function) {
return MD->size_overridden_methods() > 0 || MD->hasAttr<OverrideAttr>();
return false;
}

bool hasAttrAfterParam(const 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) {
Expand Down Expand Up @@ -189,6 +203,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).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,16 @@ 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*/) {;}{{$}}

#define ATTR_BEFORE(x) [[clang::lifetimebound]] x
int* m(ATTR_BEFORE(const int *i)) { return nullptr; }
// CHECK-MESSAGES: :[[@LINE-1]]:31: warning: parameter 'i' is unused [misc-unused-parameters]
// CHECK-FIXES: {{^}}int* m(ATTR_BEFORE(const int * /*i*/)) { return nullptr; }{{$}}
#undef ATTR_BEFORE

// Unchanged cases
// ===============
void f(int i); // Don't remove stuff in declarations
Expand All @@ -42,6 +52,12 @@ void s(int i[1]);
void u(void (*fn)());
void w(int i) { (void)i; } // Don't remove used parameters

// Don't reanchor the attribute to the type:
int *x(int *i [[clang::lifetimebound]]) { return nullptr; }
#define ATTR_AFTER(x) x [[clang::lifetimebound]]
int* y(ATTR_AFTER(const int *i)) { return nullptr; }
#undef ATTR_AFTER

bool useLambda(int (*fn)(int));
static bool static_var = useLambda([] (int a) { return a; });

Expand Down

0 comments on commit 1b897f7

Please sign in to comment.