Skip to content

Commit

Permalink
[clang-tidy] Fixed misc-unused-parameters omitting parameters square …
Browse files Browse the repository at this point in the history
…brackets

Summary:
Bug: https://bugs.llvm.org/show_bug.cgi?id=34449

**Problem:**

Clang-tidy check misc-unused-parameters comments out parameter name omitting following characters (e.g. square brackets) what results in its complete removal. Compilation errors might occur after clang-tidy fix as well.

**Patch description:**

Changed removal range. The range should end after parameter name, not after whole parameter declarator (which might be followed by e.g. square brackets).

Reviewers: alexfh

Reviewed By: alexfh

Subscribers: JDevlieghere, xazax.hun, cfe-commits

Tags: #clang-tools-extra

Patch by Pawel Maciocha!

Differential Revision: https://reviews.llvm.org/D37846

llvm-svn: 313355
  • Loading branch information
alexfh committed Sep 15, 2017
1 parent d6ce937 commit 7b9f3e2
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 4 deletions.
3 changes: 1 addition & 2 deletions clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,7 @@ void UnusedParametersCheck::warnOnUnusedParameter(
if (Function->isExternallyVisible() ||
!Result.SourceManager->isInMainFile(Function->getLocation()) ||
!Indexer->getOtherRefs(Function).empty() || isOverrideMethod(Function)) {
SourceRange RemovalRange(Param->getLocation(),
Param->DeclaratorDecl::getSourceRange().getEnd());
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.
Expand Down
51 changes: 49 additions & 2 deletions clang-tools-extra/test/clang-tidy/misc-unused-parameters.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,26 @@ void c(int *i) {}
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: parameter 'i' is unused [misc-unused-parameters]
// CHECK-FIXES: {{^}}void c(int * /*i*/) {}{{$}}

void d(int i[]) {}
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused [misc-unused-parameters]
// CHECK-FIXES: {{^}}void d(int /*i*/[]) {}{{$}}

void e(int i[1]) {}
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: parameter 'i' is unused [misc-unused-parameters]
// CHECK-FIXES: {{^}}void e(int /*i*/[1]) {}{{$}}

void f(void (*fn)()) {}
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: parameter 'fn' is unused [misc-unused-parameters]
// CHECK-FIXES: {{^}}void f(void (* /*fn*/)()) {}{{$}}

// Unchanged cases
// ===============
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
void h(int i[]);
void s(int i[1]);
void u(void (*fn)());
void w(int i) { (void)i; } // Don't remove used parameters

bool useLambda(int (*fn)(int));
static bool static_var = useLambda([] (int a) { return a; });
Expand Down Expand Up @@ -59,6 +74,18 @@ static void staticFunctionF(int i) {}
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning
// CHECK-FIXES: {{^}}static void staticFunctionF()

static void staticFunctionG(int i[]);
// CHECK-FIXES: {{^}}static void staticFunctionG();
static void staticFunctionG(int i[]) {}
// CHECK-MESSAGES: :[[@LINE-1]]:33: warning
// CHECK-FIXES: {{^}}static void staticFunctionG()

static void staticFunctionH(void (*fn)());
// CHECK-FIXES: {{^}}static void staticFunctionH();
static void staticFunctionH(void (*fn)()) {}
// CHECK-MESSAGES: :[[@LINE-1]]:36: warning
// CHECK-FIXES: {{^}}static void staticFunctionH()

static void someCallSites() {
staticFunctionA(1);
// CHECK-FIXES: staticFunctionA();
Expand All @@ -74,6 +101,12 @@ static void someCallSites() {
// CHECK-FIXES: staticFunctionF();
staticFunctionF();
// CHECK-FIXES: staticFunctionF();
int t[] = {1};
staticFunctionG(t);
// CHECK-FIXES: staticFunctionG();
void func();
staticFunctionH(&func);
// CHECK-FIXES: staticFunctionH();
}

/*
Expand Down Expand Up @@ -109,6 +142,12 @@ class SomeClass {
static void g(int i = 1) {}
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning
// CHECK-FIXES: static void g(int /*i*/ = 1) {}
static void h(int i[]) {}
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning
// CHECK-FIXES: static void h(int /*i*/[]) {}
static void s(void (*fn)()) {}
// CHECK-MESSAGES: :[[@LINE-1]]:24: warning
// CHECK-FIXES: static void s(void (* /*fn*/)()) {}
};

namespace {
Expand All @@ -125,6 +164,12 @@ class C {
void s(int i = 1) {}
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning
// CHECK-FIXES: void s(int /*i*/ = 1) {}
void u(int i[]) {}
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning
// CHECK-FIXES: void u(int /*i*/[]) {}
void w(void (*fn)()) {}
// CHECK-MESSAGES: :[[@LINE-1]]:17: warning
// CHECK-FIXES: void w(void (* /*fn*/)()) {}
};

void C::f(int i) {}
Expand All @@ -142,7 +187,9 @@ void someMoreCallSites() {
// CHECK-FIXES: c.g();

useFunction(&C::h);
useFunction(&C::s);;
useFunction(&C::s);
useFunction(&C::u);
useFunction(&C::w);
}

class Base {
Expand Down

0 comments on commit 7b9f3e2

Please sign in to comment.