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 nodes are entirely collapsed. This isn't quite as
> > powerful as collapsing the detail *edges* only, which is worth considering.
> >
> > e.g. consider TUScheduler. The natural accounting of resources is (IMO)
> > ```
> > clangd.TUScheduler.<filename>.AST
> > clangd.TUScheduler.<filename>.Preamble.InMemory
> > clangd.TUScheduler.<filename>.Preamble.Inputs
> > ```
> > or something like that.
> >
> > Instead of aggregating to `clangd.TUScheduler` only, collapsing the
> > `<filename>` edges mean you get `clangd.TUScheduler.AST` etc.
> yes this was an option i hadn't considered. my only hesitation is this likely
> implies different paths with the same name. current api also doesn't protect
> against it, but they are a lot less likely as we collapse whole subtrees.
>
> I believe deleting the edge should also imply aggregating all the paths with
> the same name, e.g.
> `a.b.<deleted-1>.c, 5` and `a.b.<deleted-2>.c, 8` should end up being
> `a.b.c,13`. WDYT?
Agree with your desired behavior.
If you're willing to make collapsing a property of the tree rather than the
query, you avoid this problem, because you collapse up-front and make addChild
idempotent:
```
class Tree {
BumpPtrAllocator *DetailAllocator; // Null if no detail
StringMap<Tree> Children;
size_t Self = 0;
Tree* childImpl(StringRef Name) {
return &*Children.try_emplace(Name, DetailAllocator).first;
}
public:
Tree(BumpPtrAllocator *);
Tree* child(StringLiteral Name) { return childImpl(Name); }
Tree* detail(StringRef Name) {
return DetailAllocator ? child(DetailAllocator->copy(Name)) : this;
}
void add(unsigned Size) { Self += Size; }
};
```
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
