From c49846e53f9c6cd359d408262275f97e59254d06 Mon Sep 17 00:00:00 2001
From: Henner Zeller <h.zeller@acm.org>
Date: Thu, 21 Dec 2023 05:24:28 -0800
Subject: [PATCH] Remove last trace of child container assumptions in
 SyntaxTreeNode.

The container is now ready to be replaced with other implementations
for nicer node allocations.

Issues: #1523
---
 common/text/concrete_syntax_tree.h |  6 ++----
 common/text/tree_utils.cc          | 18 +++++++++++-------
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/common/text/concrete_syntax_tree.h b/common/text/concrete_syntax_tree.h
index 0ba4fae42..58f3f7111 100644
--- a/common/text/concrete_syntax_tree.h
+++ b/common/text/concrete_syntax_tree.h
@@ -127,6 +127,8 @@ class SyntaxTreeNode final : public Symbol {
   const SymbolPtr &back() const { return children_.back(); }
   SymbolPtr &back() { return children_.back(); }
 
+  void pop_back() { children_.pop_back(); }
+
   size_t size() const { return children_.size(); }
   bool empty() const { return children_.empty(); }
 
@@ -134,13 +136,9 @@ class SyntaxTreeNode final : public Symbol {
     return ConstRange(children_.cbegin(), children_.cend());
   }
 
-#if 0  // TODO: to switch, find solution for pop_back() in PruneTreeFromRight()
   MutableRange mutable_children() {
     return MutableRange(children_.begin(), children_.end());
   }
-#else
-  ChildContainer &mutable_children() { return children_; }
-#endif
 
   // Compares this node to an arbitrary symbol using the compare_tokens
   // function.
diff --git a/common/text/tree_utils.cc b/common/text/tree_utils.cc
index a192704b9..c7dc7c5ee 100644
--- a/common/text/tree_utils.cc
+++ b/common/text/tree_utils.cc
@@ -301,13 +301,13 @@ bool PruneTreeFromRight(ConcreteSyntaxTree *tree, const char *offset) {
     }
     case SymbolKind::kNode: {
       auto &node = down_cast<SyntaxTreeNode &>(*tree->get());
-      auto &children = node.mutable_children();
-      for (auto &child : reversed_view(children)) {
+      int prune_count = 0;
+      for (auto &child : const_reversed_view(node.mutable_children())) {
         if (child == nullptr) {
-          children.pop_back();  // pop_back() guaranteed to not realloc
+          ++prune_count;
         } else {
           if (PruneTreeFromRight(&child, offset)) {
-            children.pop_back();
+            ++prune_count;
           } else {
             // Since token locations are monotonic, we can stop checking
             // as soon as the above function returns false.
@@ -315,8 +315,12 @@ bool PruneTreeFromRight(ConcreteSyntaxTree *tree, const char *offset) {
           }
         }
       }
+      // To avoid iterator invalidation issues in the loop, we erase at end.
+      while (prune_count--) {
+        node.pop_back();
+      }
       // If no children remain, tell caller to delete this node.
-      return children.empty();
+      return node.empty();
     }
   }
 
@@ -343,8 +347,8 @@ ConcreteSyntaxTree *LeftSubtree(ConcreteSyntaxTree *tree) {
     // Leaves don't have subtrees.
     return nullptr;
   }
-  auto &children = down_cast<SyntaxTreeNode &>(*tree->get()).mutable_children();
-  for (auto &child : children) {
+  SyntaxTreeNode *const tree_node = down_cast<SyntaxTreeNode *>(tree->get());
+  for (auto &child : tree_node->mutable_children()) {
     if (child != nullptr) return &child;
   }
   return nullptr;