bernhardmgruber marked 2 inline comments as done.
bernhardmgruber added a comment.

Hi guys!

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 marked two lines with comments where I still need some feedback. Apart from 
that, I will run the check again on LLVM and other larger code bases to see if 
there are any further issues.

Thank you for your help!



================
Comment at: clang-tidy/modernize/UseTrailingReturnCheck.cpp:78
+  // TraverseTypeLoc().
+  // TODO: Maybe RecursiveASTVisitor should call
+  // getDerived().TraverseTypeLoc(...).
----------------
TODO: is this an issue in RecursiveASTVisitor or is this behavior intended? 
Everywhere else, it calls `getDerived().XXX();`


================
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"
----------------
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?


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
  • [PATCH] D56160: [c... Bernhard Manfred Gruber via Phabricator via cfe-commits
    • [PATCH] D5616... Roman Lebedev via Phabricator via cfe-commits
    • [PATCH] D5616... Bernhard Manfred Gruber via Phabricator via cfe-commits
    • [PATCH] D5616... Jonas Toth via Phabricator via cfe-commits

Reply via email to