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

[Richard via cfe-commits]

LegalizeAdulthood added a subscriber: LegalizeAdulthood.

================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:54-55
@@ -53,16 +53,4 @@
 
   // 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()))
----------------
alexfh wrote:
> It takes a non-trivial effort to understand the equivalence of the comment and the condition. I think, pulling the negations one level up would make the condition read easier:
> ```
> if (!(BaseReturnTy->isPointerType() && DerivedReturnTy->isPointerType()) &&
>     !(BaseReturnTy->isReferenceType() && DerivedReturnTy->isReferenceType()))
>   return;
> ```
> 
> Also, please move the definitions of the variables `BTy`, `DTy`, `BRD`, `DRD` after this `if` and merge them with their initialization. 
IMO, it would be even better would be to extract a predicate function with an intention-revealing name.


http://reviews.llvm.org/D16179