[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<Token, 16> 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 &Info = Result.Context->Idents.get(StringRef(
           Sources.getCharacterData(Tok.getLocation()), Tok.getLength()));


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D35078.105495.patch
Type: text/x-patch
Size: 2017 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170706/df45e263/attachment.bin>