SyntaxTreeNode: hand out ranges not containers.

This is work towards being able to exchange the container
for easier and faster memory management.

We can't quite replace the mutable container hand-out yet as there
is still one location that calls a container function (pop_back()).

Issues: #1523

common/text/concrete_syntax_tree.h

@@ -74,6 +74,8 @@ class SyntaxTreeNode final : public Symbol {
   // This container needs to provide a random access [] operator and
   // rbegin(), rend() iterators.
   using ChildContainer = std::vector<SymbolPtr>;
+  using ConstRange = iterator_range<ChildContainer::const_iterator>;
+  using MutableRange = iterator_range<ChildContainer::iterator>;
 
   explicit SyntaxTreeNode(const int tag = kUntagged) : tag_(tag) {}
 
@@ -128,9 +130,17 @@ class SyntaxTreeNode final : public Symbol {
   size_t size() const { return children_.size(); }
   bool empty() const { return children_.empty(); }
 
-  // TODO(hzeller): return ranges for these. Only used in range-loops.
-  const ChildContainer &children() const { return children_; }
+  ConstRange children() const {
+    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.

common/text/tree_utils.cc

@@ -58,7 +58,7 @@ const SyntaxTreeLeaf *GetRightmostLeaf(const Symbol &symbol) {
 
   const auto &node = SymbolCastToNode(symbol);
 
-  for (const auto &child : reversed_view(node.children())) {
+  for (const auto &child : const_reversed_view(node.children())) {
     if (child != nullptr) {
       const auto *leaf = GetRightmostLeaf(*child);
       if (leaf != nullptr) {

common/util/iterator_adaptors.h

@@ -50,6 +50,14 @@ reversed_view(T &t) {
   // in std:: when compiling with C++14 or newer.
 }
 
+template <class T>
+verible::iterator_range<
+    std::reverse_iterator<typename auto_iterator_selector<T>::type>>
+const_reversed_view(const T &t) {
+  return make_range(verible::make_reverse_iterator(t.end()),
+                    verible::make_reverse_iterator(t.begin()));
+}
+
 // Given a const_iterator and a mutable iterator to the original mutable
 // container, return the corresponding mutable iterator (without resorting to
 // const_cast).

verilog/CST/type.cc

@@ -48,12 +48,11 @@ SymbolPtr ReinterpretReferenceAsDataTypePackedDimensions(
   }
   verible::SyntaxTreeNode &base(verible::CheckSymbolAsNode(
       *ABSL_DIE_IF_NULL(reference_call_base), NodeEnum::kReference));
-  auto &children(base.mutable_children());
-  CHECK(!children.empty());
+  CHECK(!base.empty());
 
-  Symbol &local_root(*children.front());
+  Symbol &local_root(*base.front());
   if (local_root.Kind() != verible::SymbolKind::kNode ||
-      verible::SymbolCastToNode(*children.back())
+      verible::SymbolCastToNode(*base.back())
           .MatchesTag(NodeEnum::kHierarchyExtension)) {
     // function call -like syntax can never be interpreted as a type,
     // so return the whole subtree unmodified.
@@ -71,9 +70,14 @@ SymbolPtr ReinterpretReferenceAsDataTypePackedDimensions(
   verible::SyntaxTreeNode &local_root_with_extension_node(
       verible::SymbolCastToNode(*local_root_with_extension));
   local_root_with_extension_node.AppendChild(
-      ReinterpretLocalRootAsType(*children.front()));
+      ReinterpretLocalRootAsType(*base.front()));
 
-  for (auto &child : verible::make_range(++children.begin(), children.end())) {
+  bool is_first = true;
+  for (auto &child : base.mutable_children()) {
+    if (is_first) {
+      is_first = false;
+      continue;
+    }
     // Each child could be a call-extension or an index (bit-select/slice).
     // Only [] indices are valid, any others are syntax errors.
     // We discard syntax errors for now, but in the future should retain these
@@ -171,7 +175,7 @@ const verible::Symbol *GetBaseTypeFromDataType(
   if (local_root->Tag().tag != (int)NodeEnum::kLocalRoot) return local_root;
 
   CHECK(!local_root->empty());
-  auto &children = local_root->children();
+  const auto &children = local_root->children();
   verible::Symbol *last_child = nullptr;
   for (auto &child : children) {
     if (child != nullptr && child->Kind() == verible::SymbolKind::kNode) {

verilog/tools/kythe/indexing_facts_tree_extractor.cc

@@ -908,9 +908,12 @@ void IndexingFactsTreeExtractor::ExtractModuleInstantiation(
           GetSubtreeAsSymbol(*type, NodeEnum::kInstantiationType, 0);
       if (reference->Tag().tag == (int)NodeEnum::kReference &&
           SymbolCastToNode(*reference).size() > 1) {
-        const auto &children = SymbolCastToNode(*reference).children();
-        for (auto &child :
-             verible::make_range(++children.begin(), children.end())) {
+        bool is_first = true;
+        for (const auto &child : SymbolCastToNode(*reference).children()) {
+          if (is_first) {  // skip the first one.
+            is_first = false;
+            continue;
+          }
           if (child->Tag().tag == (int)NodeEnum::kHierarchyExtension) {
             Visit(verible::SymbolCastToNode(*child));
           }
@@ -1327,9 +1330,12 @@ void IndexingFactsTreeExtractor::ExtractFunctionOrTaskCall(
         *reference_call_base, NodeEnum::kReferenceCallBase, 0);
     if (reference->Tag().tag == (int)NodeEnum::kReference) {
       if (SymbolCastToNode(*reference).size() > 1) {
-        const auto &children = SymbolCastToNode(*reference).children();
-        for (auto &child :
-             verible::make_range(++children.begin(), children.end())) {
+        bool is_first = true;
+        for (const auto &child : SymbolCastToNode(*reference).children()) {
+          if (is_first) {  // skip the first one
+            is_first = false;
+            continue;
+          }
           if (child->Tag().tag == (int)NodeEnum::kHierarchyExtension) {
             Visit(verible::SymbolCastToNode(*child));
           }