This revision was automatically updated to reflect the committed changes.
Closed by commit rL259197: Fixed function params comparison. Updated docs and
tests. (authored by alexfh).
Changed prior to commit:
http://reviews.llvm.org/D16587?vs=46360&id=46382#toc
Repository:
rL LLVM
http://reviews.llvm.org/D16587
Files:
clang-tools-extra/trunk/clang-tidy/misc/VirtualNearMissCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/VirtualNearMissCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-virtual-near-miss.rst
clang-tools-extra/trunk/test/clang-tidy/misc-virtual-near-miss.cpp
Index: clang-tools-extra/trunk/clang-tidy/misc/VirtualNearMissCheck.cpp
===================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/VirtualNearMissCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/VirtualNearMissCheck.cpp
@@ -19,6 +19,12 @@
namespace tidy {
namespace misc {
+AST_MATCHER(CXXMethodDecl, isStatic) { return Node.isStatic(); }
+
+AST_MATCHER(CXXMethodDecl, isOverloadedOperator) {
+ return Node.isOverloadedOperator();
+}
+
/// Finds out if the given method overrides some method.
static bool isOverrideMethod(const CXXMethodDecl *MD) {
return MD->size_overridden_methods() > 0 || MD->hasAttr<OverrideAttr>();
@@ -32,10 +38,14 @@
static bool checkOverridingFunctionReturnType(const ASTContext *Context,
const CXXMethodDecl *BaseMD,
const CXXMethodDecl *DerivedMD) {
- QualType BaseReturnTy =
- BaseMD->getType()->getAs<FunctionType>()->getReturnType();
- QualType DerivedReturnTy =
- DerivedMD->getType()->getAs<FunctionType>()->getReturnType();
+ QualType BaseReturnTy = BaseMD->getType()
+ ->getAs<FunctionType>()
+ ->getReturnType()
+ .getCanonicalType();
+ QualType DerivedReturnTy = DerivedMD->getType()
+ ->getAs<FunctionType>()
+ ->getReturnType()
+ .getCanonicalType();
if (DerivedReturnTy->isDependentType() || BaseReturnTy->isDependentType())
return false;
@@ -54,8 +64,8 @@
/// BTy is the class type in return type of BaseMD. For example,
/// B* Base::md()
/// While BRD is the declaration of B.
- QualType DTy = DerivedReturnTy->getPointeeType();
- QualType BTy = BaseReturnTy->getPointeeType();
+ QualType DTy = DerivedReturnTy->getPointeeType().getCanonicalType();
+ QualType BTy = BaseReturnTy->getPointeeType().getCanonicalType();
const CXXRecordDecl *DRD = DTy->getAsCXXRecordDecl();
const CXXRecordDecl *BRD = BTy->getAsCXXRecordDecl();
@@ -81,7 +91,8 @@
// Check accessibility.
// FIXME: We currently only support checking if B is accessible base class
// of D, or D is the same class which DerivedMD is in.
- bool IsItself = DRD == DerivedMD->getParent();
+ bool IsItself =
+ DRD->getCanonicalDecl() == DerivedMD->getParent()->getCanonicalDecl();
bool HasPublicAccess = false;
for (const auto &Path : Paths) {
if (Path.Access == AS_public)
@@ -121,8 +132,9 @@
return false;
for (unsigned I = 0; I < NumParamA; I++) {
- if (getDecayedType(BaseMD->getParamDecl(I)->getType()) !=
- getDecayedType(DerivedMD->getParamDecl(I)->getType()))
+ if (getDecayedType(BaseMD->getParamDecl(I)->getType().getCanonicalType()) !=
+ getDecayedType(
+ DerivedMD->getParamDecl(I)->getType().getCanonicalType()))
return false;
}
return true;
@@ -152,42 +164,35 @@
/// DerivedMD is in.
static bool checkOverrideByDerivedMethod(const CXXMethodDecl *BaseMD,
const CXXMethodDecl *DerivedMD) {
- if (BaseMD->getNameAsString() != DerivedMD->getNameAsString())
- return false;
-
- if (!checkParamTypes(BaseMD, DerivedMD))
- return false;
-
- return true;
-}
+ for (CXXMethodDecl::method_iterator I = DerivedMD->begin_overridden_methods(),
+ E = DerivedMD->end_overridden_methods();
+ I != E; ++I) {
+ const CXXMethodDecl *OverriddenMD = *I;
+ if (BaseMD->getCanonicalDecl() == OverriddenMD->getCanonicalDecl()) {
+ return true;
+ }
+ }
-/// Generate unique ID for given MethodDecl.
-///
-/// The Id is used as key for 'PossibleMap'.
-/// Typical Id: "Base::func void (void)"
-static std::string generateMethodId(const CXXMethodDecl *MD) {
- return MD->getQualifiedNameAsString() + " " + MD->getType().getAsString();
+ return false;
}
bool VirtualNearMissCheck::isPossibleToBeOverridden(
const CXXMethodDecl *BaseMD) {
- std::string Id = generateMethodId(BaseMD);
- auto Iter = PossibleMap.find(Id);
+ auto Iter = PossibleMap.find(BaseMD);
if (Iter != PossibleMap.end())
return Iter->second;
bool IsPossible = !BaseMD->isImplicit() && !isa<CXXConstructorDecl>(BaseMD) &&
!isa<CXXDestructorDecl>(BaseMD) && BaseMD->isVirtual() &&
!BaseMD->isOverloadedOperator() &&
!isa<CXXConversionDecl>(BaseMD);
- PossibleMap[Id] = IsPossible;
+ PossibleMap[BaseMD] = IsPossible;
return IsPossible;
}
bool VirtualNearMissCheck::isOverriddenByDerivedClass(
const CXXMethodDecl *BaseMD, const CXXRecordDecl *DerivedRD) {
- auto Key = std::make_pair(generateMethodId(BaseMD),
- DerivedRD->getQualifiedNameAsString());
+ auto Key = std::make_pair(BaseMD, DerivedRD);
auto Iter = OverriddenMap.find(Key);
if (Iter != OverriddenMap.end())
return Iter->second;
@@ -213,24 +218,20 @@
Finder->addMatcher(
cxxMethodDecl(
unless(anyOf(isOverride(), isImplicit(), cxxConstructorDecl(),
- cxxDestructorDecl(), cxxConversionDecl())))
+ cxxDestructorDecl(), cxxConversionDecl(), isStatic(),
+ isOverloadedOperator())))
.bind("method"),
this);
}
void VirtualNearMissCheck::check(const MatchFinder::MatchResult &Result) {
const auto *DerivedMD = Result.Nodes.getNodeAs<CXXMethodDecl>("method");
assert(DerivedMD);
- if (DerivedMD->isStatic())
- return;
-
- if (DerivedMD->isOverloadedOperator())
- return;
-
const ASTContext *Context = Result.Context;
- const auto *DerivedRD = DerivedMD->getParent();
+ const auto *DerivedRD = DerivedMD->getParent()->getDefinition();
+ assert(DerivedRD);
for (const auto &BaseSpec : DerivedRD->bases()) {
if (const auto *BaseRD = BaseSpec.getType()->getAsCXXRecordDecl()) {
Index: clang-tools-extra/trunk/clang-tidy/misc/VirtualNearMissCheck.h
===================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/VirtualNearMissCheck.h
+++ clang-tools-extra/trunk/clang-tidy/misc/VirtualNearMissCheck.h
@@ -48,12 +48,13 @@
/// key: the unique ID of a method;
/// value: whether the method is possible to be overridden.
- std::map<std::string, bool> PossibleMap;
+ std::map<const CXXMethodDecl *, bool> PossibleMap;
/// key: <unique ID of base method, name of derived class>
/// value: whether the base method is overridden by some method in the derived
/// class.
- std::map<std::pair<std::string, std::string>, bool> OverriddenMap;
+ std::map<std::pair<const CXXMethodDecl *, const CXXRecordDecl *>, bool>
+ OverriddenMap;
const unsigned EditDistanceThreshold = 1;
};
Index: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-virtual-near-miss.rst
===================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-virtual-near-miss.rst
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-virtual-near-miss.rst
@@ -13,5 +13,5 @@
struct Derived : Base {
virtual funk();
- // warning: Do you want to override 'func'?
+ // warning: 'Derived::funk' has a similar name and the same signature as virtual method 'Base::func'; did you mean to override it?
};
Index: clang-tools-extra/trunk/test/clang-tidy/misc-virtual-near-miss.cpp
===================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-virtual-near-miss.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-virtual-near-miss.cpp
@@ -26,12 +26,15 @@
Derived &operator==(const Base &); // Should not warn: operators are ignored.
};
+typedef Derived derived_type;
+
class Father {
public:
Father();
virtual void func();
virtual Father *create(int i);
virtual Base &&generate();
+ virtual Base *canonical(Derived D);
};
class Mother {
@@ -70,6 +73,10 @@
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::decaz' has {{.*}} 'Mother::decay'
operator bool();
+
+ derived_type *canonica(derived_type D);
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::canonica' has {{.*}} 'Father::canonical'
+
private:
void funk();
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::funk' has {{.*}} 'Father::func'
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
