JonasToth added a comment. > It took me far too long to come up with an update. Honestly, I was quite > demotivated as it turned out preventing name collisions of unqualifed names > after the rewrite was more difficult than I thought. Especially because this > error occurs sparsely when I test the check on bigger code bases. The current > approach with the AST visitor seems to work best and maybe I can even extend > it at some point to automatically qualify colliding names. But for now, I > would be really glad if we could achieve a state of the check that you guys > can accept and commit.
I totally know that feeling! If there is a wrong transformation it would always be catched on compilation, right? I am fine with a check that handles 99% of code correct and we do have other checks that have similar limitations. If you find a way to exclude the wrong cases from transformation even better. ================ Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:78 + // TraverseTypeLoc(). + // TODO: Maybe RecursiveASTVisitor should call + // getDerived().TraverseTypeLoc(...). ---------------- bernhardmgruber wrote: > TODO: is this an issue in RecursiveASTVisitor or is this behavior intended? > Everywhere else, it calls `getDerived().XXX();` I don't know, but its not a big issue, is it? ================ Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:335 + StringRef ReturnType = tooling::fixit::getText(ReturnTypeCVRange, Ctx); + StringRef Auto = std::isspace(*ReturnType.end()) // FIXME (dereferencing end) + ? "auto" ---------------- bernhardmgruber wrote: > FIXME: this feels wrong when I wrote it, but it works. I tried to fiddle with > the ReturnTypeCVRange to include the next charakter, but I could not get it > working without writing `tooling::fixit::getText` myself. Any ideas? That only works, because `StringRef` points within the orignal code buffer. IMHO just adding the space always and letting clang-format do its job is better then this potential out-of-bounds read. Maybe you could increase the `ReturnTypeCVRange` by one at the end, then its fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56160/new/ https://reviews.llvm.org/D56160 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
