[PATCH] D90240: [SyntaxTree] Add reverse links to syntax Nodes.

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.


================
Comment at: clang/include/clang/Tooling/Syntax/Tree.h:189
   /// EXPECTS: Child->Role != Detached
   void prependChildLowLevel(Node *Child);
   friend class TreeBuilder;
----------------
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.


================
Comment at: clang/lib/Tooling/Syntax/Tree.cpp:102
 
 void syntax::Tree::replaceChildRangeLowLevel(Node *BeforeBegin, Node *End,
                                              Node *New) {
----------------
Do you plan to remove BeforeBegin argument (in a follow-up if you prefer, but you could also change it here)


================
Comment at: clang/lib/Tooling/Syntax/Tree.cpp:262
+    ;
+  assert(Last == T->getLastChild() &&
+         "Last child is reachable by advancing from the first child.");
----------------
I think you could fold this check into the for loop below.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90240/new/

https://reviews.llvm.org/D90240