sammccall accepted this revision.
sammccall added a comment.
LGTM, thanks!
================
Comment at: clang-tools-extra/clangd/support/MemoryTree.h:57
+ /// Returns total number of bytes used by this sub-tree. Performs a
traversal.
+ size_t total() const;
+
----------------
oops, we should probably also expose self?
================
Comment at: clang-tools-extra/clangd/support/MemoryTree.h:36
+ /// No copy of the \p Name.
+ MemoryTree *addChild(llvm::StringLiteral Name) { return &createChild(Name); }
+
----------------
kadircet wrote:
> sammccall wrote:
> > 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.
> > actually, why do these return pointers rather than references?
>
> It is to make usage with `auto` safer, as it won't capture ref by default and
> make a copy. e.g. `auto child = MT.child(); child.addusage(1)` won't work as
> expected.
>
> I suppose we can make the object non-copyable and prevent such misuse.
Oh, right - yes ref but noncopyable seems even better to me
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
