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;
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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
