[PATCH] D88411: [clangd] Introduce MemoryTrees

[Sam McCall via Phabricator via cfe-commits]

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