sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/support/MemoryTree.h:36
+ /// No copy of the \p Name.
+ MemoryTree *addChild(llvm::StringLiteral Name) { return &createChild(Name); }
+
----------------
sammccall wrote:
> actually, why do these return pointers rather than references?
reading call sites, `child()` might be both more fluent and more accurate than
`addChild` - we're not calling it for the side effect and there may or may not
be one.
================
Comment at: clang-tools-extra/clangd/support/MemoryTree.h:49
+ /// with dots(".").
+ void traverseTree(llvm::function_ref<void(size_t /*Size*/,
+ llvm::StringRef /*ComponentName*/)>
----------------
sammccall wrote:
> nit: need to be explicit whether this is self or subtree (or could pass both)
I think it'd be worth having `size_t total()` with a comment that it traverses
the tree.
We have places where we only want this info (e.g. log messages) and it's
probably nice in unit tests too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88411/new/
https://reviews.llvm.org/D88411
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
