alexfh added a comment.
A few minor comments.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:23
@@ +22,3 @@
+/// Finds out if the given method overrides some method.
+bool isOverrideMethod(const CXXMethodDecl *MD) {
+ return MD->size_overridden_methods() > 0 || MD->hasAttr<OverrideAttr>();
----------------
Please mark this and other functions `static` to make them local to the
translation unit.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:43
@@ +42,3 @@
+
+ // Check if return types are identical
+ if (Context->hasSameType(DerivedReturnTy, BaseReturnTy))
----------------
nit: Missing trailing period.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:68
@@ +67,3 @@
+ // The return types aren't either both pointers or references to a class
type.
+ if (DTy.isNull()) {
+ return false;
----------------
nit: No braces around one-line `if` bodies, please.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:90
@@ +89,3 @@
+ // Check ambiguity.
+ if (Paths.isAmbiguous(Context->getCanonicalType(BTy).getUnqualifiedType()))
+ return false;
----------------
I wonder whether `BTy.getCanonicalType().getUnqualifiedType()` will work here.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:99
@@ +98,3 @@
+ for (const auto &Path : Paths) {
+ if (Path.Access == AS_public) {
+ HasPublicAccess = true;
----------------
nit: No braces around one-line `if` bodies, please.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:188
@@ +187,3 @@
+ auto Iter = PossibleMap.find(Id);
+ if (Iter != PossibleMap.end()) {
+ return Iter->second;
----------------
nit: No braces around one-line `if` bodies, please.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:203
@@ +202,3 @@
+ auto Iter = OverriddenMap.find(Key);
+ if (Iter != OverriddenMap.end()) {
+ return Iter->second;
----------------
nit: No braces around one-line `if` bodies, please.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:258
@@ +257,3 @@
+ "'%0' has a similar name and the same "
+ "signature with virtual method '%1'. "
+ "Did you meant to override it?")
----------------
The diagnostic messages are not complete sentences. Please use a semicolon
instead of the period and a lower-case letter after it. Also, s/signature
with/signature as/ and s/meant/mean/. Overall it should be:
`method '%0' has a similar name and the same signature as method '%1'; did you
mean to override it?`.
================
Comment at: test/clang-tidy/misc-virtual-near-miss.cpp:15
@@ +14,3 @@
+ void func2();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'Derived::func2' has a similar
name and the same signature with virtual method 'Base::func'. Did you meant to
override it?
+
----------------
Please remove the `. Did you mean to override it?` part from all CHECK lines
except for the first one to make the test file easier to read. Maybe even cut
the static test in the middle:
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: 'Derived::func2' has {{.*}}
'Base::func'
================
Comment at: test/clang-tidy/misc-virtual-near-miss.cpp:42
@@ +41,3 @@
+class Child : Father, Mother {
+public:
+ Child();
----------------
Please mark the inheritance explicitly `private` then to avoid questions.
http://reviews.llvm.org/D15823
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
