sammccall added inline comments.
================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:109
Node *getNextSibling() { return NextSibling; }
+ const Node *getPreviousSibling() const { return PreviousSibling; }
+ Node *getPreviousSibling() { return PreviousSibling; }
----------------
(Not something to change in this patch...)
Since adding the "get" prefixes, getNextSibling() and now getPreviousSibling()
are pretty verbose, we might consider getNext()/getPrevious()
================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:189
/// EXPECTS: Child->Role != Detached
void prependChildLowLevel(Node *Child);
friend class TreeBuilder;
----------------
gribozavr2 wrote:
> eduucaldas wrote:
> > Should we provide an `appendChildLowLevel` as well?
> >
> > That has one use inside `foldChildren` in `BuildTree.cpp`.
> > Currently this function does a reverse iteration prepending children. We
> > could change that into a forward iteration appending. There is no impact in
> > time-complexity. This change would just improve readability inside this
> > function.
> There is some awkwardness in foldChildren because we can only go in reverse
> -- maybe append is indeed more natural.
Consider `insert(Node *Child, const Node *Before)` where Before=Null means
append.
This is fairly ergonomic for common cases:
- append: `insert(N, null)`
- prepend: `insert(N, N->firstChild())`
- insert-before: `insert(N, M)`
- insert-after: `insert(N, M->nextSibling())`
(Either before or after works fine, before matches STL insert better)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90240/new/
https://reviews.llvm.org/D90240
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
