[PATCH] D16587: Fixed function params comparison. Updated docs and tests.

[Cong Liu via cfe-commits]

congliu created this revision.
congliu added a reviewer: alexfh.
congliu added a subscriber: cfe-commits.

"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.

http://reviews.llvm.org/D16587

Files:
  clang-tidy/misc/VirtualNearMissCheck.cpp
  docs/clang-tidy/checks/misc-virtual-near-miss.rst
  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
@@ -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,11 @@
   // 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'
Index: docs/clang-tidy/checks/misc-virtual-near-miss.rst
===================================================================
--- docs/clang-tidy/checks/misc-virtual-near-miss.rst
+++ 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-tidy/misc/VirtualNearMissCheck.cpp
===================================================================
--- clang-tidy/misc/VirtualNearMissCheck.cpp
+++ clang-tidy/misc/VirtualNearMissCheck.cpp
@@ -32,10 +32,14 @@
 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;
@@ -54,8 +58,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 +85,7 @@
     // 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 == DerivedMD->getParent()->getCanonicalDecl();
     bool HasPublicAccess = false;
     for (const auto &Path : Paths) {
       if (Path.Access == AS_public)
@@ -121,8 +125,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;
@@ -230,7 +235,7 @@
 
   const ASTContext *Context = Result.Context;
 
-  const auto *DerivedRD = DerivedMD->getParent();
+  const auto *DerivedRD = DerivedMD->getParent()->getCanonicalDecl();
 
   for (const auto &BaseSpec : DerivedRD->bases()) {
     if (const auto *BaseRD = BaseSpec.getType()->getAsCXXRecordDecl()) {

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