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

Thu Oct 31 10:39:08 PDT 2019

gribozavr2 added inline comments.

================
Comment at: clang-tools-extra/clang-tidy/readability/RedundantStringInitCheck.cpp:67-70
+// With -std c++14 or earlier (!LangOps.CPlusPlus17), it was sufficient to
+// return CtorExpr->getSourceRange(). However, starting with c++17, parsing
+// the expression 'std::string Name = ""' results in a CtorExpr whose
+// SourceRange includes just '""' rather than the previous 'Name = ""'.
----------------
poelmanc wrote:
> aaron.ballman wrote:
> > Are you sure that this behavioral difference isn't just a bug in Clang? I can't think of why the source range should differ based on language mode, so I wonder if the correct thing to do is fix how we calculate the source range in Clang?
> Thanks and no, I'm not sure at all. At the end of my summary paragraph I wrote:
> 
> > Or, is it possible that this change in SourceRange is an unintentional difference in the parsing code? If so fixing that might make more sense.
> 
> Before starting this patch, I built a Debug build of LLVM and stepped through LLVM parsing code while running clang-tidy on the readability-redundant-string-init.cpp file. I set breakpoints and inspected variable values under both -std c++14 and -std c++17. The call stacks were clearly different. I could sometimes inspect character positions in the file and see where an expression's SourceLoc was set. However, a SourceLoc is a single character position; I couldn't figure out where an expression's SourceRange was even set. So I gave up and went down the current approach.
> 
> Should we ping the LLVM mailing list and see if this change rings a bell with anyone there?
Yes, I believe that changing Clang to produce consistent source ranges in C++14 and C++17 is the best way to go.

Manual parsing below is not a very maintainable solution.

Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D69238