[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