aaron.ballman 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 = ""'. ---------------- gribozavr2 wrote: > 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. > Yes, I believe that changing Clang to produce consistent source ranges in > C++14 and C++17 is the best way to go. +1, if this is possible to do. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69238/new/ https://reviews.llvm.org/D69238 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
