[clang-tools-extra] r214701 - [clang-tidy] Make the named parameter check only warn on declarations if a definition is visible.
Benjamin Kramer
benny.kra at googlemail.com
Mon Aug 4 02:33:58 PDT 2014
Author: d0k
Date: Mon Aug 4 04:33:58 2014
New Revision: 214701
URL: http://llvm.org/viewvc/llvm-project?rev=214701&view=rev
Log:
[clang-tidy] Make the named parameter check only warn on declarations if a definition is visible.
Summary:
This allows us to copy the parameter name from the definition (as a comment)
or insert /*unused*/ in both places.
Reviewers: alexfh, klimek
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D4772
Modified:
clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/google-readability-function.cpp
Modified: clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.cpp?rev=214701&r1=214700&r2=214701&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/google/NamedParameterCheck.cpp Mon Aug 4 04:33:58 2014
@@ -37,6 +37,14 @@ void NamedParameterCheck::check(const Ma
if (Function->isImplicit())
return;
+ // Ignore declarations without a definition if we're not dealing with an
+ // overriden method.
+ const FunctionDecl *Definition = nullptr;
+ if (!Function->isDefined(Definition) &&
+ (!isa<CXXMethodDecl>(Function) ||
+ cast<CXXMethodDecl>(Function)->size_overridden_methods() == 0))
+ return;
+
// TODO: Handle overloads.
// TODO: We could check that all redeclarations use the same name for
// arguments in the same position.
@@ -69,26 +77,35 @@ void NamedParameterCheck::check(const Ma
auto D = diag(FirstParm->getLocation(),
"all parameters should be named in a function");
+ // Fallback to an unused marker.
+ StringRef NewName = "unused";
+
for (auto P : UnnamedParams) {
// If the method is overridden, try to copy the name from the base method
// into the overrider.
- const ParmVarDecl *Parm = P.first->getParamDecl(P.second);
const auto *M = dyn_cast<CXXMethodDecl>(P.first);
if (M && M->size_overridden_methods() > 0) {
const ParmVarDecl *OtherParm =
(*M->begin_overridden_methods())->getParamDecl(P.second);
- std::string Name = OtherParm->getNameAsString();
- if (!Name.empty()) {
- D << FixItHint::CreateInsertion(Parm->getLocation(),
- " /*" + Name + "*/");
- continue;
- }
+ StringRef Name = OtherParm->getName();
+ if (!Name.empty())
+ NewName = Name;
}
- // Otherwise just insert an unused marker. Note that getLocation() points
- // to the place where the name would be, this allows us to also get
- // complex cases like function pointers right.
- D << FixItHint::CreateInsertion(Parm->getLocation(), " /*unused*/");
+ // If the definition has a named parameter use that name.
+ if (Definition) {
+ const ParmVarDecl *DefParm = Definition->getParamDecl(P.second);
+ StringRef Name = DefParm->getName();
+ if (!Name.empty())
+ NewName = Name;
+ }
+
+ // Now insert the comment. Note that getLocation() points to the place
+ // where the name would be, this allows us to also get complex cases like
+ // function pointers right.
+ const ParmVarDecl *Parm = P.first->getParamDecl(P.second);
+ D << FixItHint::CreateInsertion(Parm->getLocation(),
+ " /*" + NewName.str() + "*/");
}
}
}
Modified: clang-tools-extra/trunk/test/clang-tidy/google-readability-function.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/google-readability-function.cpp?rev=214701&r1=214700&r2=214701&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/google-readability-function.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/google-readability-function.cpp Mon Aug 4 04:33:58 2014
@@ -4,57 +4,57 @@
void Method(char *) { /* */ }
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: all parameters should be named in a function
// CHECK-FIXES: void Method(char * /*unused*/) { /* */ }
-void Method2(char *);
+void Method2(char *) {}
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in a function
-// CHECK-FIXES: void Method2(char * /*unused*/);
-void Method3(char *, void *);
+// CHECK-FIXES: void Method2(char * /*unused*/) {}
+void Method3(char *, void *) {}
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in a function
-// CHECK-FIXES: void Method3(char * /*unused*/, void * /*unused*/);
-void Method4(char *, int /*unused*/);
+// CHECK-FIXES: void Method3(char * /*unused*/, void * /*unused*/) {}
+void Method4(char *, int /*unused*/) {}
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: all parameters should be named in a function
-// CHECK-FIXES: void Method4(char * /*unused*/, int /*unused*/);
-void operator delete[](void *) throw();
+// CHECK-FIXES: void Method4(char * /*unused*/, int /*unused*/) {}
+void operator delete[](void *) throw() {}
// CHECK-MESSAGES: :[[@LINE-1]]:30: warning: all parameters should be named in a function
-// CHECK-FIXES: void operator delete[](void * /*unused*/) throw();
-int Method5(int);
+// CHECK-FIXES: void operator delete[](void * /*unused*/) throw() {}
+int Method5(int) { return 0; }
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: all parameters should be named in a function
-// CHECK-FIXES: int Method5(int /*unused*/);
+// CHECK-FIXES: int Method5(int /*unused*/) { return 0; }
void Method6(void (*)(void *)) {}
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: all parameters should be named in a function
// CHECK-FIXES: void Method6(void (* /*unused*/)(void *)) {}
-template <typename T> void Method7(T);
+template <typename T> void Method7(T) {}
// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: all parameters should be named in a function
-// CHECK-FIXES: template <typename T> void Method7(T /*unused*/);
+// CHECK-FIXES: template <typename T> void Method7(T /*unused*/) {}
// Don't warn in macros.
-#define M void MethodM(int);
+#define M void MethodM(int) {}
M
-void operator delete(void *x) throw();
+void operator delete(void *x) throw() {}
void Method7(char * /*x*/) {}
-void Method8(char *x);
+void Method8(char *x) {}
typedef void (*TypeM)(int x);
void operator delete[](void *x) throw();
void operator delete[](void * /*x*/) throw();
struct X {
- X operator++(int);
+ X operator++(int) {}
// CHECK-MESSAGES: :[[@LINE-1]]:19: warning: all parameters should be named in a function
-// CHECK-FIXES: X operator++(int /*unused*/);
- X operator--(int /*unused*/);
+// CHECK-FIXES: X operator++(int /*unused*/) {}
+ X operator--(int /*unused*/) {}
const int &i;
};
void (*Func1)(void *);
void Func2(void (*func)(void *)) {}
-template <void Func(void *)> void Func3();
+template <void Func(void *)> void Func3() {}
template <typename T>
struct Y {
- void foo(T);
+ void foo(T) {}
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: all parameters should be named in a function
-// CHECK-FIXES: void foo(T /*unused*/);
+// CHECK-FIXES: void foo(T /*unused*/) {}
};
Y<int> y;
@@ -70,3 +70,10 @@ struct Derived : public Base {
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: all parameters should be named in a function
// CHECK-FIXES: void foo(int /*argname*/);
};
+
+void FDef(int);
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: all parameters should be named in a function
+// CHECK-FIXES: void FDef(int /*n*/);
+void FDef(int n) {};
+
+void FNoDef(int);
More information about the cfe-commits
mailing list