[PATCH] D69238: Fix clang-tidy readability-redundant-string-init for c++17/c++2a

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc added a comment.

In D69238#1739627 <https://reviews.llvm.org/D69238#1739627>, @NoQ wrote:

> Clang's `-ast-dump` <https://clang.llvm.org/docs/IntroductionToTheClangAST.html>.


Wow. That makes this so much easier... Thank you so much!

Looking at the AST showed that the `CXXConstructExpr`s that `readability-redundant-string-init` previously relied upon for `SourceRange` were being elided in C++17/2a, but that `VarDecl`s consistently held all the right info across all C++ modes. So I completely rewrote this patch to get the `SourceRange` from the `VarDecl`. All tests pass.

In case anyone cares or for posterity, what follows are a bunch of probably unnecessary details.

I made a super minimal readability-redundant-string-init.cpp:

  namespace std {
  struct string {
    string();
    string(const char *);
  };
  }
  
  void f() {
    std::string a = "", b = "foo", c, d(""), e("bar");
  }

Running `clang -Xclang -ast-dump -std=c++11 readability-redundant-string-init.cpp` yields:

  `-FunctionDecl 0x27a33bfd610 <line:8:1, line:10:1> line:8:6 f 'void ()'
    `-CompoundStmt 0x27a33bf6c78 <col:10, line:10:1>
      `-DeclStmt 0x27a33bf6c60 <line:9:3, col:52>
        |-VarDecl 0x27a33bfd788 <col:3, col:19> col:15 a 'std::string':'std::string' cinit
        | `-ExprWithCleanups 0x27a33bfddd0 <col:15, col:19> 'std::string':'std::string'
        |   `-CXXConstructExpr 0x27a33bfdda0 <col:15, col:19> 'std::string':'std::string' 'void (std::string &&) noexcept' elidable
        |     `-MaterializeTemporaryExpr 0x27a33bfdd48 <col:19> 'std::string':'std::string' xvalue
        |       `-ImplicitCastExpr 0x27a33bfdc20 <col:19> 'std::string':'std::string' <ConstructorConversion>
        |         `-CXXConstructExpr 0x27a33bfdbf0 <col:19> 'std::string':'std::string' 'void (const char *)'
        |           `-ImplicitCastExpr 0x27a33bfdbd8 <col:19> 'const char *' <ArrayToPointerDecay>
        |             `-StringLiteral 0x27a33bfd868 <col:19> 'const char [1]' lvalue ""
        |-VarDecl 0x27a33bfde08 <col:3, col:27> col:23 b 'std::string':'std::string' cinit
        | `-ExprWithCleanups 0x27a33bfdfb0 <col:23, col:27> 'std::string':'std::string'
        |   `-CXXConstructExpr 0x27a33bfdf80 <col:23, col:27> 'std::string':'std::string' 'void (std::string &&) noexcept' elidable
        |     `-MaterializeTemporaryExpr 0x27a33bfdf68 <col:27> 'std::string':'std::string' xvalue
        |       `-ImplicitCastExpr 0x27a33bfdf50 <col:27> 'std::string':'std::string' <ConstructorConversion>
        |         `-CXXConstructExpr 0x27a33bfdf20 <col:27> 'std::string':'std::string' 'void (const char *)'
        |           `-ImplicitCastExpr 0x27a33bfdf08 <col:27> 'const char *' <ArrayToPointerDecay>
        |             `-StringLiteral 0x27a33bfdee8 <col:27> 'const char [4]' lvalue "foo"
        |-VarDecl 0x27a33bfdfe8 <col:3, col:34> col:34 c 'std::string':'std::string' callinit
        | `-CXXConstructExpr 0x27a33bfe050 <col:34> 'std::string':'std::string' 'void ()'
        |-VarDecl 0x27a33bfe098 <col:3, col:41> col:37 d 'std::string':'std::string' callinit
        | `-CXXConstructExpr 0x27a33bf6af0 <col:37, col:41> 'std::string':'std::string' 'void (const char *)'
        |   `-ImplicitCastExpr 0x27a33bf6ad8 <col:39> 'const char *' <ArrayToPointerDecay>
        |     `-StringLiteral 0x27a33bf6aa0 <col:39> 'const char [1]' lvalue ""
        `-VarDecl 0x27a33bf6b40 <col:3, col:51> col:44 e 'std::string':'std::string' callinit
          `-CXXConstructExpr 0x27a33bf6c00 <col:44, col:51> 'std::string':'std::string' 'void (const char *)'
            `-ImplicitCastExpr 0x27a33bf6be8 <col:46> 'const char *' <ArrayToPointerDecay>
              `-StringLiteral 0x27a33bf6ba8 <col:46> 'const char [4]' lvalue "bar"

With -std=c++11, all of the outer `CXXConstructExpr` have the correct range to replace with just the variable names, which explains why all is fine with C++11.

Then running `clang -Xclang -ast-dump -std=c++17 readability-redundant-string-init.cpp` yields:

  `-FunctionDecl 0x2b8e6ac96b0 <line:8:1, line:10:1> line:8:6 f 'void ()'
    `-CompoundStmt 0x2b8e6acc608 <col:10, line:10:1>
      `-DeclStmt 0x2b8e6acc5f0 <line:9:3, col:52>
        |-VarDecl 0x2b8e6ac9828 <col:3, col:19> col:15 a 'std::string':'std::string' cinit
        | `-ImplicitCastExpr 0x2b8e6ac9cc0 <col:19> 'std::string':'std::string' <ConstructorConversion>
        |   `-CXXConstructExpr 0x2b8e6ac9c90 <col:19> 'std::string':'std::string' 'void (const char *)'
        |     `-ImplicitCastExpr 0x2b8e6ac9c78 <col:19> 'const char *' <ArrayToPointerDecay>
        |       `-StringLiteral 0x2b8e6ac9908 <col:19> 'const char [1]' lvalue ""
        |-VarDecl 0x2b8e6ac9e08 <col:3, col:27> col:23 b 'std::string':'std::string' cinit
        | `-ImplicitCastExpr 0x2b8e6ac9f50 <col:27> 'std::string':'std::string' <ConstructorConversion>
        |   `-CXXConstructExpr 0x2b8e6ac9f20 <col:27> 'std::string':'std::string' 'void (const char *)'
        |     `-ImplicitCastExpr 0x2b8e6ac9f08 <col:27> 'const char *' <ArrayToPointerDecay>
        |       `-StringLiteral 0x2b8e6ac9ee8 <col:27> 'const char [4]' lvalue "foo"
        |-VarDecl 0x2b8e6ac9f88 <col:3, col:34> col:34 c 'std::string':'std::string' callinit
        | `-CXXConstructExpr 0x2b8e6ac9ff0 <col:34> 'std::string':'std::string' 'void ()'
        |-VarDecl 0x2b8e6aca038 <col:3, col:41> col:37 d 'std::string':'std::string' callinit
        | `-CXXConstructExpr 0x2b8e6aca0f0 <col:37, col:41> 'std::string':'std::string' 'void (const char *)'
        |   `-ImplicitCastExpr 0x2b8e6aca0d8 <col:39> 'const char *' <ArrayToPointerDecay>
        |     `-StringLiteral 0x2b8e6aca0a0 <col:39> 'const char [1]' lvalue ""
        `-VarDecl 0x2b8e6acc4d0 <col:3, col:51> col:44 e 'std::string':'std::string' callinit
          `-CXXConstructExpr 0x2b8e6acc590 <col:44, col:51> 'std::string':'std::string' 'void (const char *)'
            `-ImplicitCastExpr 0x2b8e6acc578 <col:46> 'const char *' <ArrayToPointerDecay>
              `-StringLiteral 0x2b8e6acc538 <col:46> 'const char [4]' lvalue "bar"

With -std=c++17 the two elidable `CXXConstructExpr` are gone, and the previously nested `CXXConstructExpr` that remain have too-short ranges, which explains the C++17 failure.

Fortunately this dump showed that VarDecl contained all the information needed to obtain the //right// ranges. From

  VarDecl 0xfffffffffff <col:Begin, col:End> col:Loc var_name 'std::string':'std::string' cinit

we extract `SourceRange(Loc,End)` and everything works fine across C++ modes.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69238/new/

https://reviews.llvm.org/D69238