Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

[Alexander Kornienko via cfe-commits]

alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

Looks good now. Thank you!


http://reviews.llvm.org/D16922



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

[Cong Liu via cfe-commits]

congliu closed this revision.
congliu added a comment.

Closed by commit rL260532: Merge branch 'arcpatch-D16922' 
 (authored by congliu 
)


http://reviews.llvm.org/D16922



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

[Cong Liu via cfe-commits]

congliu added a comment.

The strategy has changed. Now this check does not ignore template in 
instantiation, instead, it generate a single warning for some instantiation and 
ignore others, so that fix is able to apply.
We do this to walk around the function copied from ASTMatchFinder, since it's 
incorrect for some tricky situation. And make sense to analyse overriding only 
when there is fully instantiation of the primary template.


http://reviews.llvm.org/D16922



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

> The strategy has changed. Now this check does not ignore template in 
> instantiation, instead, it generate a 

>  single warning for some instantiation and ignore others, so that fix is able 
> to apply.


I'm not sure this is the right thing to do, since there might be a case when 
the presence of the pattern we consider a bug depends on the instantiation:

  struct A {
virtual void func() = 0;
  };
  struct B {
virtual void funk();
  };
  template
  struct D : public T {
virtual void func() {...}
  };
  void f() {
A *da = new D;
da->func();
B *db = new D; // If the check applies a fix issued when inspecting this 
instantiation, it will break the code on the previous line.
  }

We still might want to warn in this case though.



Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:253
@@ +252,3 @@
+SourceLocation Loc = DerivedMD->getLocStart();
+if (WarningSet.find(Loc.getRawEncoding()) != WarningSet.end())
+  continue;

This should be done using a single lookup:

  if (!WarningSet.insert(Loc).second)
continue;


Comment at: clang-tidy/misc/VirtualNearMissCheck.h:49
@@ -48,3 +48,3 @@
 
-  /// key: the unique ID of a method;
+  /// key: the unique ID of a method
   /// value: whether the method is possible to be overridden.

The "unique ID of a method" would better be described as a "pointer to the 
method definition" or "pointer to the canonical declaration of the method" 
depending on what is actually used as a key.

Also, please use proper capitalization and punctuation ("Key", and the trailing 
period).


Comment at: clang-tidy/misc/VirtualNearMissCheck.h:59
@@ -58,1 +58,3 @@
 
+  /// key: the source location id of a generated warning
+  std::unordered_set WarningSet;

Please use proper capitalization and punctuation. Also, I'd just say that we 
keep source locations of issued warnings to avoid duplicate warnings caused by 
multiple instantiations.


Comment at: clang-tidy/misc/VirtualNearMissCheck.h:60
@@ +59,3 @@
+  /// key: the source location id of a generated warning
+  std::unordered_set WarningSet;
+

You don't need to `getRawEncoding`. This can be a `std::set` or 
even better a `llvm::SmallPtrSet`, for example.


http://reviews.llvm.org/D16922



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

[Cong Liu via cfe-commits]

congliu updated this revision to Diff 47448.
congliu marked 2 inline comments as done.
congliu added a comment.

- Generate a single warning for multiple template instantiations.


http://reviews.llvm.org/D16922

Files:
  clang-tidy/misc/VirtualNearMissCheck.cpp
  clang-tidy/misc/VirtualNearMissCheck.h
  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
@@ -16,22 +16,57 @@
   // overriden by this class.
   virtual void funk();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::funk' has a similar name and the same signature as virtual method 'Base::func'; did you mean to override it? [misc-virtual-near-miss]
+  // CHECK-FIXES: virtual void func();
 
   void func2();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::func2' has {{.*}} 'Base::func'
+  // CHECK-FIXES: void func();
 
   void func22(); // Should not warn.
 
   void gunk(); // Should not warn: gunk is override.
 
   void fun();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::fun' has {{.*}} 'Base::func'
+  // CHECK-FIXES: void func();
 
   Derived ==(const Base &); // Should not warn: operators are ignored.
 
   virtual NoDefinedClass2 *f1(); // Should not crash: non-defined class return type is ignored.
 };
 
+template 
+struct TBase {
+  virtual void tfunc(T t);
+};
+
+template 
+struct TDerived : TBase {
+  virtual void tfunk(T t);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'TDerived::tfunk' has {{.*}} 'TBase::tfunc'
+  // CHECK-FIXES: struct TDerived : TBase {
+  // CHECK-FIXES-NEXT: virtual void tfunc(T t);
+};
+
+TDerived T1;
+TDerived T2;
+
+// Should not fix macro definition
+#define MACRO1 void funcM()
+// CHECK-FIXES: #define MACRO1 void funcM()
+#define MACRO2(m) void m()
+// CHECK-FIXES: #define MACRO2(m) void m()
+
+struct DerivedMacro : Base {
+  MACRO1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'DerivedMacro::funcM' has {{.*}} 'Base::func'
+  // CHECK-FIXES: MACRO1;
+
+  MACRO2(func3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'DerivedMacro::func3' has {{.*}} 'Base::func'
+  // CHECK-FIXES: MACRO2(func);
+};
+
 typedef Derived derived_type;
 
 class Father {
@@ -58,32 +93,40 @@
 
   virtual void func2();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::func2' has {{.*}} 'Father::func'
+  // CHECK-FIXES: virtual void func();
 
   int methoe(int x, char **strs); // Should not warn: parameter types don't match.
 
   int methoe(int x);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::methoe' has {{.*}} 'Mother::method'
+  // CHECK-FIXES: int method(int x);
 
   void methof(int x, const char **strs); // Should not warn: return types don't match.
 
   int methoh(int x, const char **strs);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::methoh' has {{.*}} 'Mother::method'
+  // CHECK-FIXES: int method(int x, const char **strs);
 
   virtual Child *creat(int i);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::creat' has {{.*}} 'Father::create'
+  // CHECK-FIXES: virtual Child *create(int i);
 
   virtual Derived &();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::generat' has {{.*}} 'Father::generate'
+  // CHECK-FIXES: virtual Derived &();
 
   int decaz(const char str[]);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::decaz' has {{.*}} 'Mother::decay'
+  // CHECK-FIXES: int decay(const char str[]);
 
   operator bool();
 
   derived_type *canonica(derived_type D);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::canonica' has {{.*}} 'Father::canonical'
+  // CHECK-FIXES: derived_type *canonical(derived_type D);
 
 private:
   void funk();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::funk' has {{.*}} 'Father::func'
+  // CHECK-FIXES: void func();
 };
Index: clang-tidy/misc/VirtualNearMissCheck.h
===
--- clang-tidy/misc/VirtualNearMissCheck.h
+++ clang-tidy/misc/VirtualNearMissCheck.h
@@ -12,7 +12,7 @@
 
 #include "../ClangTidy.h"
 #include 
-#include 
+#include 
 
 namespace clang {
 namespace tidy {
@@ -46,7 +46,7 @@
   bool isOverriddenByDerivedClass(const CXXMethodDecl *BaseMD,
   const CXXRecordDecl *DerivedRD);
 
-  /// key: the unique ID of a method;
+  /// key: the unique ID of a method
   /// value: whether the method is possible to be overridden.
   std::map PossibleMap;
 
@@ -56,6 +56,9 @@
   std::map
   OverriddenMap;
 
+  /// key: the source location id of a generated warning
+  std::unordered_set WarningSet;
+
   const unsigned EditDistanceThreshold = 1;
 };
 
Index: clang-tidy/misc/VirtualNearMissCheck.cpp
===
--- clang-tidy/misc/VirtualNearMissCheck.cpp

Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

[Cong Liu via cfe-commits]

congliu updated this revision to Diff 47461.
congliu marked 3 inline comments as done.
congliu added a comment.

- Allowed warnings but disallowed fix for template instantiations.
- Updated tests.


http://reviews.llvm.org/D16922

Files:
  clang-tidy/misc/VirtualNearMissCheck.cpp
  clang-tidy/misc/VirtualNearMissCheck.h
  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
@@ -16,22 +16,58 @@
   // overriden by this class.
   virtual void funk();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::funk' has a similar name and the same signature as virtual method 'Base::func'; did you mean to override it? [misc-virtual-near-miss]
+  // CHECK-FIXES: virtual void func();
 
   void func2();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::func2' has {{.*}} 'Base::func'
+  // CHECK-FIXES: void func();
 
   void func22(); // Should not warn.
 
   void gunk(); // Should not warn: gunk is override.
 
   void fun();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::fun' has {{.*}} 'Base::func'
+  // CHECK-FIXES: void func();
 
   Derived ==(const Base &); // Should not warn: operators are ignored.
 
   virtual NoDefinedClass2 *f1(); // Should not crash: non-defined class return type is ignored.
 };
 
+template 
+struct TBase {
+  virtual void tfunc(T t);
+};
+
+template 
+struct TDerived : TBase {
+  virtual void tfunk(T t);
+  // Should not apply fix for template.
+  // CHECK-MESSAGES: :[[@LINE-2]]:3: warning: method 'TDerived::tfunk' has {{.*}} 'TBase::tfunc'
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: method 'TDerived::tfunk' has {{.*}} 'TBase::tfunc'
+  // CHECK-FIXES: virtual void tfunk(T t);
+};
+
+TDerived T1;
+TDerived T2;
+
+// Should not fix macro definition
+#define MACRO1 void funcM()
+// CHECK-FIXES: #define MACRO1 void funcM()
+#define MACRO2(m) void m()
+// CHECK-FIXES: #define MACRO2(m) void m()
+
+struct DerivedMacro : Base {
+  MACRO1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'DerivedMacro::funcM' has {{.*}} 'Base::func'
+  // CHECK-FIXES: MACRO1;
+
+  MACRO2(func3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'DerivedMacro::func3' has {{.*}} 'Base::func'
+  // CHECK-FIXES: MACRO2(func);
+};
+
 typedef Derived derived_type;
 
 class Father {
@@ -58,32 +94,40 @@
 
   virtual void func2();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::func2' has {{.*}} 'Father::func'
+  // CHECK-FIXES: virtual void func();
 
   int methoe(int x, char **strs); // Should not warn: parameter types don't match.
 
   int methoe(int x);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::methoe' has {{.*}} 'Mother::method'
+  // CHECK-FIXES: int method(int x);
 
   void methof(int x, const char **strs); // Should not warn: return types don't match.
 
   int methoh(int x, const char **strs);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::methoh' has {{.*}} 'Mother::method'
+  // CHECK-FIXES: int method(int x, const char **strs);
 
   virtual Child *creat(int i);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::creat' has {{.*}} 'Father::create'
+  // CHECK-FIXES: virtual Child *create(int i);
 
   virtual Derived &();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::generat' has {{.*}} 'Father::generate'
+  // CHECK-FIXES: virtual Derived &();
 
   int decaz(const char str[]);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::decaz' has {{.*}} 'Mother::decay'
+  // CHECK-FIXES: int decay(const char str[]);
 
   operator bool();
 
   derived_type *canonica(derived_type D);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::canonica' has {{.*}} 'Father::canonical'
+  // CHECK-FIXES: derived_type *canonical(derived_type D);
 
 private:
   void funk();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::funk' has {{.*}} 'Father::func'
+  // CHECK-FIXES: void func();
 };
Index: clang-tidy/misc/VirtualNearMissCheck.h
===
--- clang-tidy/misc/VirtualNearMissCheck.h
+++ clang-tidy/misc/VirtualNearMissCheck.h
@@ -12,7 +12,6 @@
 
 #include "../ClangTidy.h"
 #include 
-#include 
 
 namespace clang {
 namespace tidy {
@@ -46,12 +45,12 @@
   bool isOverriddenByDerivedClass(const CXXMethodDecl *BaseMD,
   const CXXRecordDecl *DerivedRD);
 
-  /// key: the unique ID of a method;
-  /// value: whether the method is possible to be overridden.
+  /// Key: the unique ID of a method.
+  /// Value: whether the method is possible to be overridden.
   std::map PossibleMap;
 
-  /// key: 
-  /// value: whether the base method is overridden by some method in the derived
+  /// Key: 
+  /// Value: whether the base method is overridden by some method in the derived
   /// class.
   

Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

[Cong Liu via cfe-commits]

congliu updated this revision to Diff 47307.
congliu added a comment.

- Disallowed matches for method in template instantiations. Updated test.


http://reviews.llvm.org/D16922

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
@@ -16,22 +16,54 @@
   // overriden by this class.
   virtual void funk();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::funk' has a similar name and the same signature as virtual method 'Base::func'; did you mean to override it? [misc-virtual-near-miss]
+  // CHECK-FIXES: virtual void func();
 
   void func2();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::func2' has {{.*}} 'Base::func'
+  // CHECK-FIXES: void func();
 
   void func22(); // Should not warn.
 
   void gunk(); // Should not warn: gunk is override.
 
   void fun();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::fun' has {{.*}} 'Base::func'
+  // CHECK-FIXES: void func();
 
   Derived ==(const Base &); // Should not warn: operators are ignored.
 
   virtual NoDefinedClass2 *f1(); // Should not crash: non-defined class return type is ignored.
 };
 
+template 
+struct TBase {
+  virtual void tfunc(T t);
+};
+
+template 
+struct TDerived : TBase {
+  virtual void tfunk(T t);
+  // Should not warn for 'TDerived::tfunk' or 'TDerived::tfunk',
+  // because matches in template instantiations are ignored.
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: method 'TDerived::tfunk' has {{.*}} 'TBase::tfunc'
+  // CHECK-FIXES: virtual void tfunc(T t);
+};
+
+TDerived T1;
+TDerived T2;
+
+// Should not fix macro definition
+#define MACRO1 void funcM()
+#define MACRO2(m) void m()
+struct DerivedMacro : Base {
+  MACRO1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'DerivedMacro::funcM' has {{.*}} 'Base::func'
+
+  MACRO2(func3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'DerivedMacro::func3' has {{.*}} 'Base::func'
+  // CHECK-FIXES: MACRO2(func);
+};
+
 typedef Derived derived_type;
 
 class Father {
@@ -58,32 +90,40 @@
 
   virtual void func2();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::func2' has {{.*}} 'Father::func'
+  // CHECK-FIXES: virtual void func();
 
   int methoe(int x, char **strs); // Should not warn: parameter types don't match.
 
   int methoe(int x);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::methoe' has {{.*}} 'Mother::method'
+  // CHECK-FIXES: int method(int x);
 
   void methof(int x, const char **strs); // Should not warn: return types don't match.
 
   int methoh(int x, const char **strs);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::methoh' has {{.*}} 'Mother::method'
+  // CHECK-FIXES: int method(int x, const char **strs);
 
   virtual Child *creat(int i);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::creat' has {{.*}} 'Father::create'
+  // CHECK-FIXES: virtual Child *create(int i);
 
   virtual Derived &();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::generat' has {{.*}} 'Father::generate'
+  // CHECK-FIXES: virtual Derived &();
 
   int decaz(const char str[]);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::decaz' has {{.*}} 'Mother::decay'
+  // CHECK-FIXES: int decay(const char str[]);
 
   operator bool();
 
   derived_type *canonica(derived_type D);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::canonica' has {{.*}} 'Father::canonical'
+  // CHECK-FIXES: derived_type *canonical(derived_type D);
 
 private:
   void funk();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::funk' has {{.*}} 'Father::func'
+  // CHECK-FIXES: void func();
 };
Index: clang-tidy/misc/VirtualNearMissCheck.cpp
===
--- clang-tidy/misc/VirtualNearMissCheck.cpp
+++ clang-tidy/misc/VirtualNearMissCheck.cpp
@@ -178,6 +178,50 @@
   return false;
 }
 
+/// Get declaration given the type of a class (including template class).
+///
+/// This function is copied from the one defined in ASTMatchFinder.cpp to solve
+/// the problem that QualType::getAsCXXRecordDecl does not work for template
+/// class.
+static CXXRecordDecl *getAsCXXRecordDecl(const Type *TypeNode) {
+  if (TypeNode->getAs() != nullptr ||
+  TypeNode->getAs() != nullptr ||
+  TypeNode->getAs() != nullptr)
+// Dependent names and template TypeNode parameters will be matched when the
+// template is instantiated.
+return nullptr;
+  TemplateSpecializationType const *TemplateType =
+  TypeNode->getAs();
+  if (!TemplateType)
+return TypeNode->getAsCXXRecordDecl();
+
+  if (TemplateType->getTemplateName().isDependent())
+// Dependent template specializations will be matched when the template is
+// instantiated.
+return 

Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

[Alexander Kornienko via cfe-commits]

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



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: test/clang-tidy/misc-virtual-near-miss.cpp:1
@@ -1,2 +1,2 @@
 // RUN: %check_clang_tidy %s misc-virtual-near-miss %t
 

The check shouldn't make any changes to template classes based on any single 
instantiation. An easy way to ensure this is to disallow matches in template 
instantiations, e.g. using the `unless(isInTemplateInstantiation())` limiting 
matcher.


Comment at: test/clang-tidy/misc-virtual-near-miss.cpp:58
@@ +57,3 @@
+  MACRO1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::funcM' has 
{{.*}} 'Base::func'
+

Please move the #defines close to their usages and also verify they don't 
change as a result of applying fixes.


http://reviews.llvm.org/D16922



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

[Cong Liu via cfe-commits]

congliu updated this revision to Diff 47024.
congliu added a comment.

- Added test cases of macro and template.


http://reviews.llvm.org/D16922

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
@@ -1,5 +1,25 @@
 // RUN: %check_clang_tidy %s misc-virtual-near-miss %t
 
+#define MACRO1 void funcM()
+#define MACRO2(m) void m()
+
+template 
+struct TBase {
+  virtual void tfunc(T t);
+};
+
+template 
+struct TDerived : TBase {
+  virtual void tfunk(T t);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'TDerived::tfunk' has a similar name and the same signature as virtual method 'TBase::tfunc'; did you mean to override it? [misc-virtual-near-miss]
+  // CHECK-MESSAGES: note: this fix will not be applied because it overlaps with another fix
+  // CHECK-MESSAGES: :[[@LINE-3]]:3: warning: method 'TDerived::tfunk' has {{.*}} 'TBase::tfunc'
+  // CHECK-MESSAGES: note: this fix will not be applied because it overlaps with another fix
+};
+
+TDerived T1;
+TDerived T2;
+
 class NoDefinedClass1;
 class NoDefinedClass2;
 
@@ -15,21 +35,31 @@
   // Should not warn "do you want to override 'gunk'?", because gunk is already
   // overriden by this class.
   virtual void funk();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::funk' has a similar name and the same signature as virtual method 'Base::func'; did you mean to override it? [misc-virtual-near-miss]
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::funk' has {{.*}} 'Base::func'
+  // CHECK-FIXES: virtual void func();
 
   void func2();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::func2' has {{.*}} 'Base::func'
+  // CHECK-FIXES: void func();
 
   void func22(); // Should not warn.
 
   void gunk(); // Should not warn: gunk is override.
 
   void fun();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::fun' has {{.*}} 'Base::func'
+  // CHECK-FIXES: void func();
 
   Derived ==(const Base &); // Should not warn: operators are ignored.
 
   virtual NoDefinedClass2 *f1(); // Should not crash: non-defined class return type is ignored.
+
+  MACRO1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::funcM' has {{.*}} 'Base::func'
+
+  MACRO2(func3);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::func3' has {{.*}} 'Base::func'
+  // CHECK-FIXES: MACRO2(func);
 };
 
 typedef Derived derived_type;
@@ -58,32 +88,40 @@
 
   virtual void func2();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::func2' has {{.*}} 'Father::func'
+  // CHECK-FIXES: virtual void func();
 
   int methoe(int x, char **strs); // Should not warn: parameter types don't match.
 
   int methoe(int x);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::methoe' has {{.*}} 'Mother::method'
+  // CHECK-FIXES: int method(int x);
 
   void methof(int x, const char **strs); // Should not warn: return types don't match.
 
   int methoh(int x, const char **strs);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::methoh' has {{.*}} 'Mother::method'
+  // CHECK-FIXES: int method(int x, const char **strs);
 
   virtual Child *creat(int i);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::creat' has {{.*}} 'Father::create'
+  // CHECK-FIXES: virtual Child *create(int i);
 
   virtual Derived &();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::generat' has {{.*}} 'Father::generate'
+  // CHECK-FIXES: virtual Derived &();
 
   int decaz(const char str[]);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::decaz' has {{.*}} 'Mother::decay'
+  // CHECK-FIXES: int decay(const char str[]);
 
   operator bool();
 
   derived_type *canonica(derived_type D);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::canonica' has {{.*}} 'Father::canonical'
+  // CHECK-FIXES: derived_type *canonical(derived_type D);
 
 private:
   void funk();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::funk' has {{.*}} 'Father::func'
+  // CHECK-FIXES: void func();
 };
Index: clang-tidy/misc/VirtualNearMissCheck.cpp
===
--- clang-tidy/misc/VirtualNearMissCheck.cpp
+++ clang-tidy/misc/VirtualNearMissCheck.cpp
@@ -249,11 +249,14 @@
 if (EditDistance > 0 && EditDistance <= EditDistanceThreshold) {
   if (checkOverrideWithoutName(Context, BaseMD, DerivedMD)) {
 // A "virtual near miss" is found.
+auto Range = CharSourceRange::getTokenRange(
+SourceRange(DerivedMD->getLocation()));
 diag(DerivedMD->getLocStart(),
  "method '%0' has a similar name and the same signature as "
  "virtual method '%1'; did you mean to override it?")
 

[PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

[Cong Liu via cfe-commits]

congliu created this revision.
congliu added a reviewer: alexfh.
congliu added a subscriber: cfe-commits.

Addes FixItHint and updated test.

http://reviews.llvm.org/D16922

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
@@ -16,16 +16,19 @@
   // overriden by this class.
   virtual void funk();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::funk' has a 
similar name and the same signature as virtual method 'Base::func'; did you 
mean to override it? [misc-virtual-near-miss]
+  // CHECK-FIXES: virtual void func();
 
   void func2();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::func2' has 
{{.*}} 'Base::func'
+  // CHECK-FIXES: void func();
 
   void func22(); // Should not warn.
 
   void gunk(); // Should not warn: gunk is override.
 
   void fun();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::fun' has {{.*}} 
'Base::func'
+  // CHECK-FIXES: void func();
 
   Derived ==(const Base &); // Should not warn: operators are ignored.
 
@@ -58,32 +61,40 @@
 
   virtual void func2();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::func2' has {{.*}} 
'Father::func'
+  // CHECK-FIXES: virtual void func();
 
   int methoe(int x, char **strs); // Should not warn: parameter types don't 
match.
 
   int methoe(int x);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::methoe' has 
{{.*}} 'Mother::method'
+  // CHECK-FIXES: int method(int x);
 
   void methof(int x, const char **strs); // Should not warn: return types 
don't match.
 
   int methoh(int x, const char **strs);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::methoh' has 
{{.*}} 'Mother::method'
+  // CHECK-FIXES: int method(int x, const char **strs);
 
   virtual Child *creat(int i);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::creat' has {{.*}} 
'Father::create'
+  // CHECK-FIXES: virtual Child *create(int i);
 
   virtual Derived &();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::generat' has 
{{.*}} 'Father::generate'
+  // CHECK-FIXES: virtual Derived &();
 
   int decaz(const char str[]);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::decaz' has {{.*}} 
'Mother::decay'
+  // CHECK-FIXES: int decay(const char str[]);
 
   operator bool();
 
   derived_type *canonica(derived_type D);
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::canonica' has 
{{.*}} 'Father::canonical'
+  // CHECK-FIXES: derived_type *canonical(derived_type D);
 
 private:
   void funk();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Child::funk' has {{.*}} 
'Father::func'
+  // CHECK-FIXES: void func();
 };
Index: clang-tidy/misc/VirtualNearMissCheck.cpp
===
--- clang-tidy/misc/VirtualNearMissCheck.cpp
+++ clang-tidy/misc/VirtualNearMissCheck.cpp
@@ -249,11 +249,14 @@
 if (EditDistance > 0 && EditDistance <= EditDistanceThreshold) {
   if (checkOverrideWithoutName(Context, BaseMD, DerivedMD)) {
 // A "virtual near miss" is found.
+auto Range = CharSourceRange::getTokenRange(SourceRange(
+DerivedMD->getLocation(), DerivedMD->getLocation()));
 diag(DerivedMD->getLocStart(),
  "method '%0' has a similar name and the same signature as "
  "virtual method '%1'; did you mean to override it?")
 << DerivedMD->getQualifiedNameAsString()
-<< BaseMD->getQualifiedNameAsString();
+<< BaseMD->getQualifiedNameAsString()
+<< FixItHint::CreateReplacement(Range, BaseMD->getName());
   }
 }
   }


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
@@ -16,16 +16,19 @@
   // overriden by this class.
   virtual void funk();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::funk' has a similar name and the same signature as virtual method 'Base::func'; did you mean to override it? [misc-virtual-near-miss]
+  // CHECK-FIXES: virtual void func();
 
   void func2();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::func2' has {{.*}} 'Base::func'
+  // CHECK-FIXES: void func();
 
   void func22(); // Should not warn.
 
   void gunk(); // Should not warn: gunk is override.
 
   void fun();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: method 'Derived::fun' has {{.*}} 'Base::func'
+  // CHECK-FIXES: void func();
 
   Derived ==(const Base &); // Should not warn: operators are ignored.
 
@@ -58,32 +61,40 @@
 
   virtual void func2();
   // CHECK-MESSAGES: 

Re: [PATCH] D16922: [clang-tidy] Added check-fixes for misc-virtual-near-miss.

[Alexander Kornienko via cfe-commits]

alexfh added inline comments.


Comment at: clang-tidy/misc/VirtualNearMissCheck.cpp:252
@@ -251,1 +251,3 @@
 // A "virtual near miss" is found.
+auto Range = CharSourceRange::getTokenRange(SourceRange(
+DerivedMD->getLocation(), DerivedMD->getLocation()));

There are two ways to make it shorter:
  * use the `getTokenRange` overload for `SourceLocation`s: 
`CharSourceRange::getTokenRange(DerivedMD->getLocation(), 
DerivedMD->getLocation());`;
  * use the `SourceRange` constructor accepting a single location: 
`CharSourceRange::getTokenRange(SourceRange(DerivedMD->getLocation()));`.

Choose whatever you like more ;)


Comment at: test/clang-tidy/misc-virtual-near-miss.cpp:1
@@ -1,2 +1,2 @@
 // RUN: %check_clang_tidy %s misc-virtual-near-miss %t
 

Please add a test ensuring replacements are applied correctly in templates with 
multiple instantiations and in macros.


http://reviews.llvm.org/D16922



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits