[clang-tidy] Handle decayed types and other improvements in VirtualNearMiss check.

Handle decayed types, ignore qualifiers and accessibility when considering a
method as a possible overload.

Differential Revision: http://reviews.llvm.org/D16179 

llvm-svn: 258562

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

@@ -45,31 +45,20 @@ static bool checkOverridingFunctionReturnType(const ASTContext *Context,
     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 BTy, DTy;
-  const CXXRecordDecl *BRD, *DRD;
+  QualType DTy = DerivedReturnTy->getPointeeType();
+  QualType BTy = BaseReturnTy->getPointeeType();
 
-  // Both types must be pointers or references to classes.
-  if (const auto *DerivedPT = DerivedReturnTy->getAs<PointerType>()) {
-    if (const auto *BasePT = BaseReturnTy->getAs<PointerType>()) {
-      DTy = DerivedPT->getPointeeType();
-      BTy = BasePT->getPointeeType();
-    }
-  } else if (const auto *DerivedRT = DerivedReturnTy->getAs<ReferenceType>()) {
-    if (const auto *BaseRT = BaseReturnTy->getAs<ReferenceType>()) {
-      DTy = DerivedRT->getPointeeType();
-      BTy = BaseRT->getPointeeType();
-    }
-  }
-
-  // The return types aren't either both pointers or references to a class type.
-  if (DTy.isNull())
-    return false;
-
-  DRD = DTy->getAsCXXRecordDecl();
-  BRD = BTy->getAsCXXRecordDecl();
+  const CXXRecordDecl *DRD = DTy->getAsCXXRecordDecl();
+  const CXXRecordDecl *BRD = BTy->getAsCXXRecordDecl();
   if (DRD == nullptr || BRD == nullptr)
     return false;
 
@@ -116,6 +105,13 @@ static bool checkOverridingFunctionReturnType(const ASTContext *Context,
   return true;
 }
 
+/// \returns decayed type for arrays and functions.
+static QualType getDecayedType(QualType Type) {
+  if (const auto *Decayed = Type->getAs<DecayedType>())
+    return Decayed->getDecayedType();
+  return Type;
+}
+
 /// \returns true if the param types are the same.
 static bool checkParamTypes(const CXXMethodDecl *BaseMD,
                             const CXXMethodDecl *DerivedMD) {
@@ -125,8 +121,8 @@ static bool checkParamTypes(const CXXMethodDecl *BaseMD,
     return false;
 
   for (unsigned I = 0; I < NumParamA; I++) {
-    if (BaseMD->getParamDecl(I)->getType() !=
-        DerivedMD->getParamDecl(I)->getType())
+    if (getDecayedType(BaseMD->getParamDecl(I)->getType()) !=
+        getDecayedType(DerivedMD->getParamDecl(I)->getType()))
       return false;
   }
   return true;
@@ -137,15 +133,9 @@ static bool checkParamTypes(const CXXMethodDecl *BaseMD,
 static bool checkOverrideWithoutName(const ASTContext *Context,
                                      const CXXMethodDecl *BaseMD,
                                      const CXXMethodDecl *DerivedMD) {
-  if (BaseMD->getTypeQualifiers() != DerivedMD->getTypeQualifiers())
-    return false;
-
   if (BaseMD->isStatic() != DerivedMD->isStatic())
     return false;
 
-  if (BaseMD->getAccess() != DerivedMD->getAccess())
-    return false;
-
   if (BaseMD->getType() == DerivedMD->getType())
     return true;
 
@@ -187,7 +177,8 @@ bool VirtualNearMissCheck::isPossibleToBeOverridden(
     return Iter->second;
 
   bool IsPossible = !BaseMD->isImplicit() && !isa<CXXConstructorDecl>(BaseMD) &&
-                    BaseMD->isVirtual();
+                    !isa<CXXDestructorDecl>(BaseMD) && BaseMD->isVirtual() &&
+                    !BaseMD->isOverloadedOperator();
   PossibleMap[Id] = IsPossible;
   return IsPossible;
 }
@@ -218,19 +209,23 @@ void VirtualNearMissCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus)
     return;
 
-  Finder->addMatcher(cxxMethodDecl(unless(anyOf(isOverride(), isImplicit(),
-                                                cxxConstructorDecl())))
-                         .bind("method"),
-                     this);
+  Finder->addMatcher(
+      cxxMethodDecl(unless(anyOf(isOverride(), isImplicit(),
+                                 cxxConstructorDecl(), cxxDestructorDecl())))
+          .bind("method"),
+      this);
 }
 
 void VirtualNearMissCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *DerivedMD = Result.Nodes.getNodeAs<CXXMethodDecl>("method");
-  assert(DerivedMD != nullptr);
+  assert(DerivedMD);
 
   if (DerivedMD->isStatic())
     return;
 
+  if (DerivedMD->isOverloadedOperator())
+    return;
+
   const ASTContext *Context = Result.Context;
 
   const auto *DerivedRD = DerivedMD->getParent();

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

@@ -34,7 +34,7 @@ public:
 
 private:
   /// Check if the given method is possible to be overridden by some other
-  /// method.
+  /// method. Operators and destructors are excluded.
   ///
   /// Results are memoized in PossibleMap.
   bool isPossibleToBeOverridden(const CXXMethodDecl *BaseMD);

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

@@ -3,6 +3,8 @@
 struct Base {
   virtual void func();
   virtual void gunk();
+  virtual ~Base();
+  virtual Base &operator=(const Base &);
 };
 
 struct Derived : Base {
@@ -20,6 +22,8 @@ struct Derived : Base {
 
   void fun();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::fun' has {{.*}} 'Base::func'
+
+  Derived &operator==(const Base &); // Should not warn: operators are ignored.
 };
 
 class Father {
@@ -36,6 +40,7 @@ public:
   static void method();
   virtual int method(int argc, const char **argv);
   virtual int method(int argc) const;
+  virtual int decay(const char *str);
 };
 
 class Child : private Father, private Mother {
@@ -47,7 +52,8 @@ public:
 
   int methoe(int x, char **strs); // Should not warn: parameter types don't match.
 
-  int methoe(int x); // Should not warn: method is not const.
+  int methoe(int x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::methoe' has {{.*}} 'Mother::method'
 
   void methof(int x, const char **strs); // Should not warn: return types don't match.
 
@@ -60,6 +66,10 @@ 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'
+
 private:
-  void funk(); // Should not warn: access qualifers don't match.
+  void funk();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::funk' has {{.*}} 'Father::func'
 };