Fixed function params comparison. Updated docs and tests.

[congliu]

Summary

"checkParamTypes" may fail if the the type of some parameter is not canonical. Fixed it by comparing canonical types. And added "getCanonicalType()" and "getCanonicalDecl()" on more places to prevent potential fail.

[congliu]

• Fixed bugs introduced by getCanonicalDecl. Has errs log.
• Cleared up logs.
• Added mathcers for static method and overloaded operator.
• Change key type of map from string to pointer to imporve performance.
• Change implementation of checkOverrideByDerivedMethod.

[congliu]

• Fixed a nit in test file.

[alexfh]

Looks good! I'll commit the patch for you.

clang-tools-extra/trunk/clang-tidy/misc/VirtualNearMissCheck.h

private:
   /// derived class.
   ///
   /// Results are memoized in OverriddenMap.
   bool isOverriddenByDerivedClass(const CXXMethodDecl *BaseMD,
                                   const CXXRecordDecl *DerivedRD);
 
   /// 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;
 };
 
 } // namespace misc
 } // namespace tidy
 } // namespace clang
 
 #endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_VIRTUAL_NEAR_MISS_H

clang-tools-extra/trunk/clang-tidy/misc/VirtualNearMissCheck.cpp

 #include "clang/Lex/Lexer.h"
 
 using namespace clang::ast_matchers;
 
 namespace clang {
 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>();
 }
 
 /// Checks whether the return types are covariant, according to
 /// C++[class.virtual]p7.
 ///
 /// Similar with clang::Sema::CheckOverridingFunctionReturnType.
 /// \returns true if the return types of BaseMD and DerivedMD are covariant.
 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;
 
   // Check if return types are identical.
   if (Context->hasSameType(DerivedReturnTy, BaseReturnTy))
     return true;
 
   /// Check if the return types are covariant.
 
   // Both types must be pointers or references to classes.
   if (!(BaseReturnTy->isPointerType() && DerivedReturnTy->isPointerType()) &&
       !(BaseReturnTy->isReferenceType() && DerivedReturnTy->isReferenceType()))
     return false;
 
   /// 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();
   if (DRD == nullptr || BRD == nullptr)
     return false;
 
   if (DRD == BRD)
     return true;
if (!Context->hasSameUnqualifiedType(DTy, BTy)) {
 
     // Check ambiguity.
     if (Paths.isAmbiguous(Context->getCanonicalType(BTy).getUnqualifiedType()))
       return false;
 
     // 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)
         HasPublicAccess = true;
     }
     if (!HasPublicAccess && !IsItself)
       return false;
     // End checking conversion from D to B.

 static bool checkParamTypes(const CXXMethodDecl *BaseMD,
                             const CXXMethodDecl *DerivedMD) {
   unsigned NumParamA = BaseMD->getNumParams();
   unsigned NumParamB = DerivedMD->getNumParams();
   if (NumParamA != NumParamB)
     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;
 }
 
 /// \returns true if derived method can override base method except for the
 /// name.
 static bool checkOverrideWithoutName(const ASTContext *Context,

 }
 
 /// Check whether BaseMD overrides DerivedMD.
 ///
 /// Prerequisite: the class which BaseMD is in should be a base class of that
 /// 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;
-
+  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;
 
   bool IsOverridden = false;
   for (const CXXMethodDecl *DerivedMD : DerivedRD->methods()) {
     if (!isOverrideMethod(DerivedMD))
       continue;

 
 void VirtualNearMissCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
     return;
 
   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()) {
       for (const auto *BaseMD : BaseRD->methods()) {
         if (!isPossibleToBeOverridden(BaseMD))
           continue;
 
         if (isOverriddenByDerivedClass(BaseMD, DerivedRD))

clang-tools-extra/trunk/docs/clang-tidy/checks/misc-virtual-near-miss.rst

 misc-virtual-near-miss
 ======================
 
 Warn if a function is a near miss (ie. the name is very similar and the function signiture is the same) to a virtual function from a base class.
 
 Example:
 
 .. code-block:: c++
 
   struct Base {
     virtual void func();
   };
 
   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?
   };

clang-tools-extra/trunk/test/clang-tidy/misc-virtual-near-miss.cpp

struct Derived : Base {
   void gunk(); // Should not warn: gunk is override.
 
   void fun();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::fun' has {{.*}} 'Base::func'
 
   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 {
 public:
   Mother();
   static void method();
   virtual int method(int argc, const char **argv);
   virtual int method(int argc) const;
public:
 
   virtual Derived &&generat();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::generat' has {{.*}} 'Father::generate'
 
   int decaz(const char str[]);
   // 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'
 };