[PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Tue Feb 9 08:35:15 PST 2016
alexfh added inline comments.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:183
@@ +182,3 @@
+///
+/// This function is copied from the one defined in ASTMatchFinder.cpp to solve
+/// the problem that QualType::getAsCXXRecordDecl does not work for template
----------------
I'm not fond of code duplication. If this function is useful in more than one place, it should be made public (in a separate patch, since this is a different repository) and reused, not just copied.
================
Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:270
@@ -225,1 +269,3 @@
+ isOverloadedOperator(),
+ clang::ast_matchers::isTemplateInstantiation())))
.bind("method"),
----------------
We usually use `isInTemplateInstantiation()`, which also ignores all non-template declarations which have template parents. Also, there's no need to fully qualify the name.
================
Comment at: test/clang-tidy/misc-virtual-near-miss.cpp:49
@@ +48,3 @@
+ // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: method 'TDerived::tfunk' has {{.*}} 'TBase::tfunc'
+ // CHECK-FIXES: virtual void tfunc(T t);
+};
----------------
This CHECK-FIXES pattern could also match the line inside the TBase class. We need to make this pattern more specific, e.g. like this:
// CHECK-FIXES: struct TDerived :
// CHECK-FIXES-NEXT: virtual void tfunc(T t);
================
Comment at: test/clang-tidy/misc-virtual-near-miss.cpp:55
@@ +54,3 @@
+
+// Should not fix macro definition
+#define MACRO1 void funcM()
----------------
This comment is not enough, please add a CHECK-FIXES ensuring the macro definitions are left intact.
http://reviews.llvm.org/D16922
More information about the cfe-commits
mailing list