sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Fantastic, thank you! Just a couple of nits.
Want me to land this for you?
================
Comment at: clang-tools-extra/clangd/DumpAST.cpp:339
+
+ bool TraverseTUDecl(TranslationUnitDecl *TU) {
+ return traverseNode("declaration", TU, [&] {
----------------
The naming causes a bit of confusion over whether this is a (CRTP) override of
a RecursiveASTVisitor method. (It's not, as that method is called
TraverseTranslationUnitDecl).
I'd suggest either:
- (trivial) rename to traverseTUDecl, and move this away from the comment
above that says this is overriding traversal
- (maybe more elegant) instead of this function change the lambda inside
TraverseDecl to be `if (is TUDecl) TraverseAST() else Base::TraverseDecl(D)`.
Then I don't think the special case in `dumpAST()` is needed at all.
================
Comment at: clang-tools-extra/clangd/DumpAST.cpp:405
+// Note: It's safe for N to be a TranslationUnitDecl, as this function
+// does not deserialize the preamble.
----------------
this comment should rather go in the .h file
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101057/new/
https://reviews.llvm.org/D101057
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
