kadircet added a comment. > However those difficulties don't apply here - I believe you can take the > current final representation (vector<DocumentSymbol>) + the list of marks and > produce a markified vector<DocumentSymbol> via a fairly natural tree walk. > And this does a *really* good job of isolating this feature - we literally > run the rest of the algorithm without it. > (This is slightly different to what's done in the patch, I would actually > take it out of the DocumentOutline class entirely). > I don't think this hurts efficiency much (I think/hope this ends up being a > move rather than a copy for most nodes). And I think it actually makes the > markifying code easier to reason about in this case, as it just deals with > line numbers/positions, without having the distraction of SourceLocation. > @kadircet WDYT of this argument, before we make David go back and forth?
As discussed offline, I am sold. In the end both this approach and inserting marks as we build the tree share a lot in common. In both cases when inserting a node into the tree we need to figure out whether it should go under the current parent or there's a mark that should contain that node, which is possibly not present in the tree yet. Instead of looking at ast nodes, it is definitely more reasonable to look at the document symbol nodes. Also as you mentioned doing this afterwards definitely does a better job at isolating this from the rest + enables us to fix up ranges/ordering of the tree produced from ast. Sorry for the hassle here David :/ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105904/new/ https://reviews.llvm.org/D105904 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
