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

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

xazax.hun added inline comments.

================
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()))
----------------
LegalizeAdulthood wrote:
> 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.
I could not come up with a good name, so I left it as it is. If you have something in mind, feel free to tell me.


http://reviews.llvm.org/D16179