[PATCH] D88411: [clangd] Introduce MemoryTrees

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf9317f7bf6bd: [clangd] Introduce MemoryTrees (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D88411: [clangd] Introduce MemoryTrees

kadircet updated this revision to Diff 296910. kadircet marked an inline comment as done. kadircet added a comment. - Inline MemoryTree::self Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88411/new/ https://reviews.llvm.org/D88411 Files: clang-t

[PATCH] D88411: [clangd] Introduce MemoryTrees

kadircet added a comment. > Still LGTM, commit it already :-D Haha, waiting for dependents to be stamped as well :D Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88411/new/ https://reviews.llvm.org/D88411 _

[PATCH] D88411: [clangd] Introduce MemoryTrees

sammccall accepted this revision. sammccall added a comment. Still LGTM, commit it already :-D Comment at: clang-tools-extra/clangd/support/MemoryTree.cpp:26 + +size_t MemoryTree::self() const { return Size; } +} // namespace clangd nit: could inline this consi

[PATCH] D88411: [clangd] Introduce MemoryTrees

kadircet updated this revision to Diff 296677. kadircet marked 2 inline comments as done. kadircet added a comment. - Make MemoryTree move-only and return refs instead of pointers on child and detail. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D8

[PATCH] D88411: [clangd] Introduce MemoryTrees

kadircet updated this revision to Diff 296675. kadircet added a comment. - Add self Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88411/new/ https://reviews.llvm.org/D88411 Files: clang-tools-extra/clangd/support/CMakeLists.txt clang-tools-ext

[PATCH] D88411: [clangd] Introduce MemoryTrees

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

[PATCH] D88411: [clangd] Introduce MemoryTrees

kadircet updated this revision to Diff 296667. kadircet marked 9 inline comments as done. kadircet added a comment. - Rename add{Detail,Child} -> {detail,child} - Get rid of `traverseTree` API and only expose `total` - Add a `children()` getter Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D88411: [clangd] Introduce MemoryTrees

kadircet 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: > sammccall wrote: > > actually, why do

[PATCH] D88411: [clangd] Introduce MemoryTrees

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 pointe

[PATCH] D88411: [clangd] Introduce MemoryTrees

sammccall added inline comments. Comment at: clang-tools-extra/clangd/support/MemoryTree.h:35 + + /// No copy of the \p Name. + MemoryTree *addChild(llvm::StringLiteral Name) { return &createChild(Name); } Maybe mention that child pointers are invalidated by fu

[PATCH] D88411: [clangd] Introduce MemoryTrees

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Looks good - some concerns that the traversal isn't flexible enough but we can address that later once it's used. Comment at: clang-tools-extra/clangd/support/MemoryTr

[PATCH] D88411: [clangd] Introduce MemoryTrees

kadircet updated this revision to Diff 296111. kadircet marked 7 inline comments as done. kadircet added a comment. - Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88411/new/ https://reviews.llvm.org/D88411 Files: clang-tools-ex

[PATCH] D88411: [clangd] Introduce MemoryTrees

sammccall added inline comments. Comment at: clang-tools-extra/clangd/support/MemoryTree.cpp:28 +size_t ChildrenSize = 0; +for (const auto &C : Children) { + C.traverseTree(CollapseDetails, kadircet wrote: > sammccall wrote: > > Here the detailed nod

[PATCH] D88411: [clangd] Introduce MemoryTrees

kadircet marked an inline comment as done. kadircet added inline comments. Comment at: clang-tools-extra/clangd/support/MemoryTree.cpp:28 +size_t ChildrenSize = 0; +for (const auto &C : Children) { + C.traverseTree(CollapseDetails, sammccall wrote: >

[PATCH] D88411: [clangd] Introduce MemoryTrees

sammccall added inline comments. Comment at: clang-tools-extra/clangd/support/MemoryTree.cpp:28 +size_t ChildrenSize = 0; +for (const auto &C : Children) { + C.traverseTree(CollapseDetails, Here the detailed nodes are entirely collapsed. This isn't q

[PATCH] D88411: [clangd] Introduce MemoryTrees

sammccall added inline comments. Comment at: clang-tools-extra/clangd/support/MemoryTree.h:22 +/// preserving the hierarchy. +struct MemoryTree { +public: (I'm wondering if there are other resources we'd like to count, like disk size or so, but probably YAGNI an

[PATCH] D88411: [clangd] Introduce MemoryTrees

kadircet created this revision. kadircet added a reviewer: sammccall. Herald added subscribers: cfe-commits, usaxena95, arphaman, mgorny. Herald added a project: clang. kadircet requested review of this revision. Herald added subscribers: MaskRay, ilya-biryukov. A structure that can be used to rep