alexfh added a comment.
A few more comments.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:22
@@ +21,3 @@
+
+bool VirtualNearMissCheck::isOverrideMethod(const CXXMethodDecl *MD) {
+ return MD->size_overridden_methods() > 0 || MD->hasAttr<OverrideAttr>();
----------------
This should be a free-standing function.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:48
@@ +47,3 @@
+
+bool VirtualNearMissCheck::checkOverridingFunctionReturnType(
+ const ASTContext *Context, const CXXMethodDecl *BaseMD,
----------------
IIUC, this can be a free-standing function instead of a method, since it
doesn't use any class members.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:110
@@ +109,3 @@
+ // of D, or D is the same class which DerivedMD is in.
+ bool IsIteself = DRD == DerivedMD->getParent();
+ bool HasPublicAccess = false;
----------------
nit: typo: s/IsIteself/IsItself/
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:112
@@ +111,3 @@
+ bool HasPublicAccess = false;
+ for (CXXBasePaths::paths_iterator Path = Paths.begin(); Path !=
Paths.end();
+ ++Path) {
----------------
Why not a range-for loop?
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:118
@@ +117,3 @@
+ }
+ if (!(HasPublicAccess || IsIteself))
+ return false;
----------------
Please propagate the negation inside parentheses: `!HasPublicAccess &&
!IsItself`.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:129
@@ +128,3 @@
+ // The class type D should have the same cv-qualification as or less
+ // cv-qualification than the class type B
+ if (DTy.isMoreQualifiedThan(BTy))
----------------
nit: Please add a trailing period.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:136
@@ +135,3 @@
+
+bool VirtualNearMissCheck::checkParamType(const CXXMethodDecl *BaseMD,
+ const CXXMethodDecl *DerivedMD) {
----------------
This should be `checkParamTypes`, note the plural form.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:136
@@ +135,3 @@
+
+bool VirtualNearMissCheck::checkParamType(const CXXMethodDecl *BaseMD,
+ const CXXMethodDecl *DerivedMD) {
----------------
alexfh wrote:
> This should be `checkParamTypes`, note the plural form.
This can be a free-standing function, since it doesn't use any class members.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:138
@@ +137,3 @@
+ const CXXMethodDecl *DerivedMD) {
+ unsigned int NumParamA = BaseMD->getNumParams();
+ unsigned int NumParamB = DerivedMD->getNumParams();
----------------
I'd slightly prefer `unsigned` over `unsigned int`.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:163
@@ +162,3 @@
+
+std::string VirtualNearMissCheck::generateMethodId(const CXXMethodDecl *MD) {
+ std::string Id =
----------------
This should be a free-standing function.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:164
@@ +163,3 @@
+std::string VirtualNearMissCheck::generateMethodId(const CXXMethodDecl *MD) {
+ std::string Id =
+ MD->getQualifiedNameAsString() + " " + MD->getType().getAsString();
----------------
The variable is not needed here, just return the expression.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:169
@@ +168,3 @@
+
+bool VirtualNearMissCheck::isPossibleToBeOverriden(
+ const CXXMethodDecl *BaseMD) {
----------------
s/Overriden/Overridden/
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:175
@@ +174,3 @@
+ if (Iter != PossibleMap.end()) {
+ IsPossible = Iter->second;
+ } else {
----------------
I'd just `return Iter->second;` and remove `else`. Same below in the
`isOverridenByDerivedClass` method.
http://reviews.llvm.org/D15823
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
