diff --git a/toolchain/check/check.cpp b/toolchain/check/check.cpp index 1d2f4dd049a71..c26925934d3e6 100644 --- a/toolchain/check/check.cpp +++ b/toolchain/check/check.cpp @@ -647,9 +647,10 @@ class NodeIdTraversal { : context_(context), next_deferred_definition_(&context.parse_tree()), worklist_(vlog_stream) { + auto range = context.parse_tree().postorder(); chunks_.push_back( - {.it = context.parse_tree().postorder().begin(), - .end = context.parse_tree().postorder().end(), + {.it = range.begin(), + .end = range.end(), .next_definition = Parse::DeferredDefinitionIndex::Invalid}); } @@ -717,12 +718,11 @@ class NodeIdTraversal { context_.parse_tree().deferred_definitions().Get(definition_index); HandleFunctionDefinitionResume(context_, definition_info.start_id, std::move(suspended_fn)); - chunks_.push_back( - {.it = context_.parse_tree().postorder(definition_info.start_id).end(), - .end = context_.parse_tree() - .postorder(definition_info.definition_id) - .end(), - .next_definition = next_deferred_definition_.index()}); + auto range = Parse::Tree::PostorderIterator::MakeRange( + definition_info.start_id, definition_info.definition_id); + chunks_.push_back({.it = range.begin() + 1, + .end = range.end(), + .next_definition = next_deferred_definition_.index()}); ++definition_index.index; next_deferred_definition_.SkipTo(definition_index); } @@ -776,7 +776,7 @@ auto NodeIdTraversal::Next() -> std::optional { // Continue type-checking the parse tree after the end of the definition. chunks_.back().it = - context_.parse_tree().postorder(definition_info.definition_id).end(); + Parse::Tree::PostorderIterator(definition_info.definition_id) + 1; next_deferred_definition_.SkipTo(definition_info.next_definition_index); continue; } diff --git a/toolchain/check/merge.cpp b/toolchain/check/merge.cpp index 68f9a6d410d2b..f241098c4b56e 100644 --- a/toolchain/check/merge.cpp +++ b/toolchain/check/merge.cpp @@ -312,10 +312,10 @@ static auto CheckRedeclParamSyntax(Context& context, << "prev_last_param_node_id.is_valid should match " "prev_first_param_node_id.is_valid"; - auto new_range = context.parse_tree().postorder(new_first_param_node_id, - new_last_param_node_id); - auto prev_range = context.parse_tree().postorder(prev_first_param_node_id, - prev_last_param_node_id); + auto new_range = Parse::Tree::PostorderIterator::MakeRange( + new_first_param_node_id, new_last_param_node_id); + auto prev_range = Parse::Tree::PostorderIterator::MakeRange( + prev_first_param_node_id, prev_last_param_node_id); // zip is using the shortest range. If they differ in length, there should be // some difference inside the range because the range includes parameter diff --git a/toolchain/parse/tree.cpp b/toolchain/parse/tree.cpp index 8423e8de6162f..64503bb15bda6 100644 --- a/toolchain/parse/tree.cpp +++ b/toolchain/parse/tree.cpp @@ -22,21 +22,10 @@ auto Tree::postorder() const -> llvm::iterator_range { auto Tree::postorder(NodeId n) const -> llvm::iterator_range { - CARBON_CHECK(n.is_valid()); // The postorder ends after this node, the root, and begins at the start of // its subtree. - int end_index = n.index + 1; - int start_index = end_index - node_impls_[n.index].subtree_size; - return llvm::iterator_range( - PostorderIterator(NodeId(start_index)), - PostorderIterator(NodeId(end_index))); -} - -auto Tree::postorder(NodeId begin, NodeId end) const - -> llvm::iterator_range { - CARBON_CHECK(begin.is_valid() && end.is_valid()); - return llvm::iterator_range( - PostorderIterator(begin), PostorderIterator(NodeId(end.index + 1))); + int start_index = n.index - node_impls_[n.index].subtree_size + 1; + return PostorderIterator::MakeRange(NodeId(start_index), n); } auto Tree::children(NodeId n) const -> llvm::iterator_range { @@ -307,6 +296,13 @@ auto Tree::Verify() const -> ErrorOr { return Success(); } +auto Tree::PostorderIterator::MakeRange(NodeId begin, NodeId end) + -> llvm::iterator_range { + CARBON_CHECK(begin.is_valid() && end.is_valid()); + return llvm::iterator_range( + PostorderIterator(begin), PostorderIterator(NodeId(end.index + 1))); +} + auto Tree::PostorderIterator::Print(llvm::raw_ostream& output) const -> void { output << node_; } diff --git a/toolchain/parse/tree.h b/toolchain/parse/tree.h index 8a204d952d15d..f0cf12fd87753 100644 --- a/toolchain/parse/tree.h +++ b/toolchain/parse/tree.h @@ -118,11 +118,6 @@ class Tree : public Printable { // descendants in depth-first postorder. auto postorder(NodeId n) const -> llvm::iterator_range; - // Returns an iterable range between the two parse tree nodes, in depth-first - // postorder. The range is inclusive of the bounds: [begin, end]. - auto postorder(NodeId begin, NodeId end) const - -> llvm::iterator_range; - // Returns an iterable range over the direct children of a node in the parse // tree. This is a forward range, but is constant time to increment. The order // of children is the same as would be found in a reverse postorder traversal. @@ -408,6 +403,15 @@ class Tree::PostorderIterator int, const NodeId*, NodeId>, public Printable { public: + // Returns an iterable range between the two parse tree nodes, in depth-first + // postorder. The range is inclusive of the bounds: [begin, end]. + static auto MakeRange(NodeId begin, NodeId end) + -> llvm::iterator_range; + + // Prefer using the `postorder` range calls, but direct construction is + // allowed if needed. + explicit PostorderIterator(NodeId n) : node_(n) {} + PostorderIterator() = delete; auto operator==(const PostorderIterator& rhs) const -> bool { @@ -442,8 +446,6 @@ class Tree::PostorderIterator private: friend class Tree; - explicit PostorderIterator(NodeId n) : node_(n) {} - NodeId node_; };