Re: [PATCH] D16179: [clang-tidy] Handle decayed types and other improvements in VirtualNearMiss check.

[Gábor Horváth via cfe-commits]

xazax.hun updated this revision to Diff 45022.
xazax.hun added a comment.

- Address a review comment.
- Fix an assertion failure for methods that has non-identifier name e.g.: 
destructors.
- Ignore qualifiers.


http://reviews.llvm.org/D16179

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
@@ -3,6 +3,7 @@
 struct Base {
   virtual void func();
   virtual void gunk();
+  virtual ~Base();
 };
 
 struct Derived : Base {
@@ -36,6 +37,7 @@
   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 +49,8 @@
 
   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 +63,10 @@
   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'
 };
Index: clang-tidy/misc/VirtualNearMissCheck.cpp
===================================================================
--- clang-tidy/misc/VirtualNearMissCheck.cpp
+++ clang-tidy/misc/VirtualNearMissCheck.cpp
@@ -52,22 +52,13 @@
   const CXXRecordDecl *BRD, *DRD;
 
   // 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())
+  if ((!BaseReturnTy->isPointerType() || !DerivedReturnTy->isPointerType()) &&
+      (!BaseReturnTy->isReferenceType() || !DerivedReturnTy->isReferenceType()))
     return false;
 
+  DTy = DerivedReturnTy->getPointeeType();
+  BTy = BaseReturnTy->getPointeeType();
+
   DRD = DTy->getAsCXXRecordDecl();
   BRD = BTy->getAsCXXRecordDecl();
   if (DRD == nullptr || BRD == nullptr)
@@ -116,6 +107,13 @@
   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 +123,8 @@
     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 +135,9 @@
 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;
 
@@ -226,7 +218,7 @@
 
 void VirtualNearMissCheck::check(const MatchFinder::MatchResult &Result) {
   const auto *DerivedMD = Result.Nodes.getNodeAs<CXXMethodDecl>("method");
-  assert(DerivedMD != nullptr);
+  assert(DerivedMD);
 
   if (DerivedMD->isStatic())
     return;
@@ -245,7 +237,8 @@
           continue;
 
         unsigned EditDistance =
-            BaseMD->getName().edit_distance(DerivedMD->getName());
+            StringRef(BaseMD->getNameAsString())
+                .edit_distance(DerivedMD->getNameAsString());
         if (EditDistance > 0 && EditDistance <= EditDistanceThreshold) {
           if (checkOverrideWithoutName(Context, BaseMD, DerivedMD)) {
             // A "virtual near miss" is found.

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits