[PATCH] D35078: [clang-tidy] Fix modernize-use-override incorrect replacement

[Victor Gao via Phabricator via cfe-commits]

vgao added a comment.

@alexfh Could you please land this for me? Thanks!


https://reviews.llvm.org/D35078



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

[PATCH] D35078: [clang-tidy] Fix modernize-use-override incorrect replacement

[Victor Gao via Phabricator via cfe-commits]

vgao updated this revision to Diff 105554.
vgao added a comment.

Change variable name `Parens` to `NestedParens`


https://reviews.llvm.org/D35078

Files:
  clang-tidy/modernize/UseOverrideCheck.cpp
  test/clang-tidy/modernize-use-override.cpp


Index: test/clang-tidy/modernize-use-override.cpp
===
--- test/clang-tidy/modernize-use-override.cpp
+++ test/clang-tidy/modernize-use-override.cpp
@@ -12,6 +12,10 @@
 
 struct MUST_USE_RESULT MustUseResultObject {};
 
+struct IntPair {
+  int First, Second;
+};
+
 struct Base {
   virtual ~Base() {}
   virtual void a();
@@ -41,6 +45,8 @@
 
   virtual void ne() noexcept(false);
   virtual void t() throw();
+
+  virtual void il(IntPair);
 };
 
 struct SimpleCases : public Base {
@@ -221,6 +227,14 @@
   // CHECK-FIXES: {{^}}  void cv2() const volatile override // some comment
 };
 
+struct DefaultArguments : public Base {
+  // Tests for default arguments (with initializer lists).
+  // Make sure the override fix is placed after the argument list.
+  void il(IntPair p = {1, (2 + (3))}) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: annotate this
+  // CHECK-FIXES: {{^}}  void il(IntPair p = {1, (2 + (3))}) override {}
+};
+
 struct Macros : public Base {
   // Tests for 'virtual' and 'override' being defined through macros. Basically
   // give up for now.
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -38,11 +38,16 @@
  File.end());
   SmallVector Tokens;
   Token Tok;
+  int NestedParens = 0;
   while (!RawLexer.LexFromRawLexer(Tok)) {
-if (Tok.is(tok::semi) || Tok.is(tok::l_brace))
+if ((Tok.is(tok::semi) || Tok.is(tok::l_brace)) && NestedParens == 0)
   break;
 if (Sources.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation()))
   break;
+if (Tok.is(tok::l_paren))
+  ++NestedParens;
+else if (Tok.is(tok::r_paren))
+  --NestedParens;
 if (Tok.is(tok::raw_identifier)) {
   IdentifierInfo  = Result.Context->Idents.get(StringRef(
   Sources.getCharacterData(Tok.getLocation()), Tok.getLength()));


Index: test/clang-tidy/modernize-use-override.cpp
===
--- test/clang-tidy/modernize-use-override.cpp
+++ test/clang-tidy/modernize-use-override.cpp
@@ -12,6 +12,10 @@
 
 struct MUST_USE_RESULT MustUseResultObject {};
 
+struct IntPair {
+  int First, Second;
+};
+
 struct Base {
   virtual ~Base() {}
   virtual void a();
@@ -41,6 +45,8 @@
 
   virtual void ne() noexcept(false);
   virtual void t() throw();
+
+  virtual void il(IntPair);
 };
 
 struct SimpleCases : public Base {
@@ -221,6 +227,14 @@
   // CHECK-FIXES: {{^}}  void cv2() const volatile override // some comment
 };
 
+struct DefaultArguments : public Base {
+  // Tests for default arguments (with initializer lists).
+  // Make sure the override fix is placed after the argument list.
+  void il(IntPair p = {1, (2 + (3))}) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: annotate this
+  // CHECK-FIXES: {{^}}  void il(IntPair p = {1, (2 + (3))}) override {}
+};
+
 struct Macros : public Base {
   // Tests for 'virtual' and 'override' being defined through macros. Basically
   // give up for now.
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -38,11 +38,16 @@
  File.end());
   SmallVector Tokens;
   Token Tok;
+  int NestedParens = 0;
   while (!RawLexer.LexFromRawLexer(Tok)) {
-if (Tok.is(tok::semi) || Tok.is(tok::l_brace))
+if ((Tok.is(tok::semi) || Tok.is(tok::l_brace)) && NestedParens == 0)
   break;
 if (Sources.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation()))
   break;
+if (Tok.is(tok::l_paren))
+  ++NestedParens;
+else if (Tok.is(tok::r_paren))
+  --NestedParens;
 if (Tok.is(tok::raw_identifier)) {
   IdentifierInfo  = Result.Context->Idents.get(StringRef(
   Sources.getCharacterData(Tok.getLocation()), Tok.getLength()));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35078: [clang-tidy] Fix modernize-use-override incorrect replacement

[Victor Gao via Phabricator via cfe-commits]

vgao created this revision.
Herald added subscribers: xazax.hun, JDevlieghere.

For the following code: `modernize-use-override` generates a replacement with 
incorrect location.

  struct IntPair
  {
int first, second;
  };
  
  struct A
  {
virtual void il(IntPair);
  };
  
  struct B : A
  {
void il(IntPair p = {1, (2 + 3)}) {};
// Generated Fixit: void il(IntPair p = override {1, (2 + 3)}) {};
// Should be: void il(IntPair p = {1, (2 + 3)}) override {};
  };

This fixes that and adds a unit test.


Repository:
  rL LLVM

https://reviews.llvm.org/D35078

Files:
  clang-tidy/modernize/UseOverrideCheck.cpp
  test/clang-tidy/modernize-use-override.cpp


Index: test/clang-tidy/modernize-use-override.cpp
===
--- test/clang-tidy/modernize-use-override.cpp
+++ test/clang-tidy/modernize-use-override.cpp
@@ -12,6 +12,10 @@
 
 struct MUST_USE_RESULT MustUseResultObject {};
 
+struct IntPair {
+  int First, Second;
+};
+
 struct Base {
   virtual ~Base() {}
   virtual void a();
@@ -41,6 +45,8 @@
 
   virtual void ne() noexcept(false);
   virtual void t() throw();
+
+  virtual void il(IntPair);
 };
 
 struct SimpleCases : public Base {
@@ -221,6 +227,14 @@
   // CHECK-FIXES: {{^}}  void cv2() const volatile override // some comment
 };
 
+struct DefaultArguments : public Base {
+  // Tests for default arguments (with initializer lists).
+  // Make sure the override fix is placed after the argument list.
+  void il(IntPair p = {1, (2 + (3))}) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: annotate this
+  // CHECK-FIXES: {{^}}  void il(IntPair p = {1, (2 + (3))}) override {}
+};
+
 struct Macros : public Base {
   // Tests for 'virtual' and 'override' being defined through macros. Basically
   // give up for now.
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -38,11 +38,16 @@
  File.end());
   SmallVector Tokens;
   Token Tok;
+  int Parens = 0;
   while (!RawLexer.LexFromRawLexer(Tok)) {
-if (Tok.is(tok::semi) || Tok.is(tok::l_brace))
+if ((Tok.is(tok::semi) || Tok.is(tok::l_brace)) && !Parens)
   break;
 if (Sources.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation()))
   break;
+if (Tok.is(tok::l_paren))
+  ++Parens;
+else if (Tok.is(tok::r_paren))
+  --Parens;
 if (Tok.is(tok::raw_identifier)) {
   IdentifierInfo  = Result.Context->Idents.get(StringRef(
   Sources.getCharacterData(Tok.getLocation()), Tok.getLength()));


Index: test/clang-tidy/modernize-use-override.cpp
===
--- test/clang-tidy/modernize-use-override.cpp
+++ test/clang-tidy/modernize-use-override.cpp
@@ -12,6 +12,10 @@
 
 struct MUST_USE_RESULT MustUseResultObject {};
 
+struct IntPair {
+  int First, Second;
+};
+
 struct Base {
   virtual ~Base() {}
   virtual void a();
@@ -41,6 +45,8 @@
 
   virtual void ne() noexcept(false);
   virtual void t() throw();
+
+  virtual void il(IntPair);
 };
 
 struct SimpleCases : public Base {
@@ -221,6 +227,14 @@
   // CHECK-FIXES: {{^}}  void cv2() const volatile override // some comment
 };
 
+struct DefaultArguments : public Base {
+  // Tests for default arguments (with initializer lists).
+  // Make sure the override fix is placed after the argument list.
+  void il(IntPair p = {1, (2 + (3))}) {}
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: annotate this
+  // CHECK-FIXES: {{^}}  void il(IntPair p = {1, (2 + (3))}) override {}
+};
+
 struct Macros : public Base {
   // Tests for 'virtual' and 'override' being defined through macros. Basically
   // give up for now.
Index: clang-tidy/modernize/UseOverrideCheck.cpp
===
--- clang-tidy/modernize/UseOverrideCheck.cpp
+++ clang-tidy/modernize/UseOverrideCheck.cpp
@@ -38,11 +38,16 @@
  File.end());
   SmallVector Tokens;
   Token Tok;
+  int Parens = 0;
   while (!RawLexer.LexFromRawLexer(Tok)) {
-if (Tok.is(tok::semi) || Tok.is(tok::l_brace))
+if ((Tok.is(tok::semi) || Tok.is(tok::l_brace)) && !Parens)
   break;
 if (Sources.isBeforeInTranslationUnit(Range.getEnd(), Tok.getLocation()))
   break;
+if (Tok.is(tok::l_paren))
+  ++Parens;
+else if (Tok.is(tok::r_paren))
+  --Parens;
 if (Tok.is(tok::raw_identifier)) {
   IdentifierInfo  = Result.Context->Idents.get(StringRef(
   Sources.getCharacterData(Tok.getLocation()), Tok.getLength()));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits