alexfh 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()))
----------------
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.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:240
@@ -247,2 +239,3 @@
unsigned EditDistance =
- BaseMD->getName().edit_distance(DerivedMD->getName());
+ StringRef(BaseMD->getNameAsString())
+ .edit_distance(DerivedMD->getNameAsString());
----------------
xazax.hun wrote:
> congliu wrote:
> > NamedDecl::getName() directly returns a StringRef. Why using
> > "getNameAsString()"?
> Unfortunately getName will cause an assertion fail for methods that has a
> non-identifier name, such as destructors and overloaded operators.
Should we maybe exclude operators and destructors from this check? A typo in
destructor name won't compile. Do you have an example of a case where the check
could be useful in detecting a typo in the name of an overloaded operator?
It would be nice to avoid using the (more expensive) `getNameAsString` here.
http://reviews.llvm.org/D16179
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
