congliu updated this revision to Diff 47307.
congliu added a comment.
- Disallowed matches for method in template instantiations. Updated test.
http://reviews.llvm.org/D16922
Files:
clang-tidy/misc/VirtualNearMissCheck.cpp
test/clang-tidy/misc-virtual-near-miss.cpp
Index: test/clang-tidy/misc-virtual-near-miss.cpp
===================================================================
--- test/clang-tidy/misc-virtual-near-miss.cpp
+++ test/clang-tidy/misc-virtual-near-miss.cpp
@@ -16,22 +16,54 @@
// overriden by this class.
virtual void funk();
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::funk' has a similar name and the same signature as virtual method 'Base::func'; did you mean to override it? [misc-virtual-near-miss]
+ // CHECK-FIXES: virtual void func();
void func2();
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::func2' has {{.*}} 'Base::func'
+ // CHECK-FIXES: void func();
void func22(); // Should not warn.
void gunk(); // Should not warn: gunk is override.
void fun();
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::fun' has {{.*}} 'Base::func'
+ // CHECK-FIXES: void func();
Derived &operator==(const Base &); // Should not warn: operators are ignored.
virtual NoDefinedClass2 *f1(); // Should not crash: non-defined class return type is ignored.
};
+template <typename T>
+struct TBase {
+ virtual void tfunc(T t);
+};
+
+template <typename T>
+struct TDerived : TBase<T> {
+ virtual void tfunk(T t);
+ // Should not warn for 'TDerived<int>::tfunk' or 'TDerived<double>::tfunk',
+ // because matches in template instantiations are ignored.
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: method 'TDerived::tfunk' has {{.*}} 'TBase::tfunc'
+ // CHECK-FIXES: virtual void tfunc(T t);
+};
+
+TDerived<int> T1;
+TDerived<double> T2;
+
+// Should not fix macro definition
+#define MACRO1 void funcM()
+#define MACRO2(m) void m()
+struct DerivedMacro : Base {
+ MACRO1;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'DerivedMacro::funcM' has {{.*}} 'Base::func'
+
+ MACRO2(func3);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'DerivedMacro::func3' has {{.*}} 'Base::func'
+ // CHECK-FIXES: MACRO2(func);
+};
+
typedef Derived derived_type;
class Father {
@@ -58,32 +90,40 @@
virtual void func2();
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::func2' has {{.*}} 'Father::func'
+ // CHECK-FIXES: virtual void func();
int methoe(int x, char **strs); // Should not warn: parameter types don't match.
int methoe(int x);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::methoe' has {{.*}} 'Mother::method'
+ // CHECK-FIXES: int method(int x);
void methof(int x, const char **strs); // Should not warn: return types don't match.
int methoh(int x, const char **strs);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::methoh' has {{.*}} 'Mother::method'
+ // CHECK-FIXES: int method(int x, const char **strs);
virtual Child *creat(int i);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::creat' has {{.*}} 'Father::create'
+ // CHECK-FIXES: virtual Child *create(int i);
virtual Derived &&generat();
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::generat' has {{.*}} 'Father::generate'
+ // CHECK-FIXES: virtual Derived &&generate();
int decaz(const char str[]);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::decaz' has {{.*}} 'Mother::decay'
+ // CHECK-FIXES: int decay(const char str[]);
operator bool();
derived_type *canonica(derived_type D);
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::canonica' has {{.*}} 'Father::canonical'
+ // CHECK-FIXES: derived_type *canonical(derived_type D);
private:
void funk();
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::funk' has {{.*}} 'Father::func'
+ // CHECK-FIXES: void func();
};
Index: clang-tidy/misc/VirtualNearMissCheck.cpp
===================================================================
--- clang-tidy/misc/VirtualNearMissCheck.cpp
+++ clang-tidy/misc/VirtualNearMissCheck.cpp
@@ -178,6 +178,50 @@
return false;
}
+/// Get declaration given the type of a class (including template class).
+///
+/// This function is copied from the one defined in ASTMatchFinder.cpp to solve
+/// the problem that QualType::getAsCXXRecordDecl does not work for template
+/// class.
+static CXXRecordDecl *getAsCXXRecordDecl(const Type *TypeNode) {
+ if (TypeNode->getAs<DependentNameType>() != nullptr ||
+ TypeNode->getAs<DependentTemplateSpecializationType>() != nullptr ||
+ TypeNode->getAs<TemplateTypeParmType>() != nullptr)
+ // Dependent names and template TypeNode parameters will be matched when the
+ // template is instantiated.
+ return nullptr;
+ TemplateSpecializationType const *TemplateType =
+ TypeNode->getAs<TemplateSpecializationType>();
+ if (!TemplateType)
+ return TypeNode->getAsCXXRecordDecl();
+
+ if (TemplateType->getTemplateName().isDependent())
+ // Dependent template specializations will be matched when the template is
+ // instantiated.
+ return nullptr;
+ // For template specialization types which are specializing a template
+ // declaration which is an explicit or partial specialization of another
+ // template declaration, getAsCXXRecordDecl() returns the corresponding
+ // ClassTemplateSpecializationDecl.
+ //
+ // For template specialization types which are specializing a template
+ // declaration which is neither an explicit nor partial specialization of
+ // another template declaration, getAsCXXRecordDecl() returns NULL and we get
+ // the CXXRecordDecl of the templated declaration.
+ CXXRecordDecl *SpecializationDecl = TemplateType->getAsCXXRecordDecl();
+ if (SpecializationDecl)
+ return SpecializationDecl;
+
+ NamedDecl *Templated =
+ TemplateType->getTemplateName().getAsTemplateDecl()->getTemplatedDecl();
+ if (CXXRecordDecl *TemplatedRecord = dyn_cast<CXXRecordDecl>(Templated))
+ return TemplatedRecord;
+
+ TypeAliasDecl *AliasDecl = dyn_cast<TypeAliasDecl>(Templated);
+ assert(AliasDecl);
+ return getAsCXXRecordDecl(AliasDecl->getUnderlyingType().getTypePtr());
+}
+
bool VirtualNearMissCheck::isPossibleToBeOverridden(
const CXXMethodDecl *BaseMD) {
auto Iter = PossibleMap.find(BaseMD);
@@ -187,7 +231,8 @@
bool IsPossible = !BaseMD->isImplicit() && !isa<CXXConstructorDecl>(BaseMD) &&
!isa<CXXDestructorDecl>(BaseMD) && BaseMD->isVirtual() &&
!BaseMD->isOverloadedOperator() &&
- !isa<CXXConversionDecl>(BaseMD);
+ !isa<CXXConversionDecl>(BaseMD) &&
+ !BaseMD->isTemplateInstantiation();
PossibleMap[BaseMD] = IsPossible;
return IsPossible;
}
@@ -221,7 +266,8 @@
cxxMethodDecl(
unless(anyOf(isOverride(), isImplicit(), cxxConstructorDecl(),
cxxDestructorDecl(), cxxConversionDecl(), isStatic(),
- isOverloadedOperator())))
+ isOverloadedOperator(),
+ clang::ast_matchers::isTemplateInstantiation())))
.bind("method"),
this);
}
@@ -236,7 +282,8 @@
assert(DerivedRD);
for (const auto &BaseSpec : DerivedRD->bases()) {
- if (const auto *BaseRD = BaseSpec.getType()->getAsCXXRecordDecl()) {
+ if (const auto *BaseRD =
+ getAsCXXRecordDecl(BaseSpec.getType().getTypePtr())) {
for (const auto *BaseMD : BaseRD->methods()) {
if (!isPossibleToBeOverridden(BaseMD))
continue;
@@ -249,11 +296,14 @@
if (EditDistance > 0 && EditDistance <= EditDistanceThreshold) {
if (checkOverrideWithoutName(Context, BaseMD, DerivedMD)) {
// A "virtual near miss" is found.
+ auto Range = CharSourceRange::getTokenRange(
+ SourceRange(DerivedMD->getLocation()));
diag(DerivedMD->getLocStart(),
"method '%0' has a similar name and the same signature as "
"virtual method '%1'; did you mean to override it?")
<< DerivedMD->getQualifiedNameAsString()
- << BaseMD->getQualifiedNameAsString();
+ << BaseMD->getQualifiedNameAsString()
+ << FixItHint::CreateReplacement(Range, BaseMD->getName());
}
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
