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 PossibleMap; + std::map PossibleMap; /// key: /// value: whether the base method is overridden by some method in the derived /// class. - std::map, bool> OverriddenMap; + std::map, bool> + OverriddenMap; const unsigned EditDistanceThreshold = 1; }; 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(); @@ -32,10 +38,14 @@ static bool checkOverridingFunctionReturnType(const ASTContext *Context, const CXXMethodDecl *BaseMD, const CXXMethodDecl *DerivedMD) { - QualType BaseReturnTy = - BaseMD->getType()->getAs()->getReturnType(); - QualType DerivedReturnTy = - DerivedMD->getType()->getAs()->getReturnType(); + QualType BaseReturnTy = BaseMD->getType() + ->getAs() + ->getReturnType() + .getCanonicalType(); + QualType DerivedReturnTy = DerivedMD->getType() + ->getAs() + ->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,27 +164,21 @@ /// 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; @@ -180,14 +186,13 @@ !isa(BaseMD) && BaseMD->isVirtual() && !BaseMD->isOverloadedOperator() && !isa(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,7 +218,8 @@ Finder->addMatcher( cxxMethodDecl( unless(anyOf(isOverride(), isImplicit(), cxxConstructorDecl(), - cxxDestructorDecl(), cxxConversionDecl()))) + cxxDestructorDecl(), cxxConversionDecl(), isStatic(), + isOverloadedOperator()))) .bind("method"), this); } @@ -222,15 +228,10 @@ const auto *DerivedMD = Result.Nodes.getNodeAs("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/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'