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

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

xazax.hun created this revision.
xazax.hun added reviewers: alexfh, congliu.
xazax.hun added subscribers: cfe-commits, dkrupp.
Herald added subscribers: rengolin, aemerson.

First of all thank you for this great check!

In this patch I propose one small cleanup and two changes in the behaviour. The first is to desugar decayed types in the arguments of functions. This way it is possible to detect override candidates when a method that has a pointer as an argument tries to override a method that has an array as an argument that is decayed to a pointer. The other change is not to take visibility into account since it is perfectly valid to override a public method with a private one and such code exists.

I have one more proposal that is not implemented in this patch yet: it is a common mistake to not to override a method because the developer forget to add the qualifiers. What about a configuration option to also report near misses when only a qualifier is missing?

What do you think?


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
@@ -36,6 +36,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 {
@@ -60,6 +61,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,16 +52,10 @@
   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();
-    }
+  if ((BaseReturnTy->isPointerType() && DerivedReturnTy->isPointerType()) ||
+      (BaseReturnTy->isReferenceType() && DerivedReturnTy->isReferenceType())) {
+    DTy = DerivedReturnTy->getPointeeType();
+    BTy = BaseReturnTy->getPointeeType();
   }
 
   // The return types aren't either both pointers or references to a class type.
@@ -116,6 +110,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 +126,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;
@@ -143,9 +144,6 @@
   if (BaseMD->isStatic() != DerivedMD->isStatic())
     return false;
 
-  if (BaseMD->getAccess() != DerivedMD->getAccess())
-    return false;
-
   if (BaseMD->getType() == DerivedMD->getType())
     return true;
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D16179.44847.patch
Type: text/x-patch
Size: 3064 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160114/25e9a512/attachment.bin>