diff --git a/src/parse_tree.cpp b/src/parse_tree.cpp index 600d9818f..6481e754c 100644 --- a/src/parse_tree.cpp +++ b/src/parse_tree.cpp @@ -1377,18 +1377,6 @@ bool parse_node_tree_t::statement_is_in_pipeline(const parse_node_t &node, return result; } -const parse_node_t *parse_node_tree_t::header_node_for_block_statement( - const parse_node_t &node) const { - const parse_node_t *result = NULL; - if (node.type == symbol_block_statement) { - const parse_node_t *block_header = this->get_child(node, 0, symbol_block_header); - if (block_header != NULL) { - result = this->get_child(*block_header, 0); - } - } - return result; -} - parse_node_tree_t::parse_node_list_t parse_node_tree_t::comment_nodes_for_node( const parse_node_t &parent) const { parse_node_list_t result; diff --git a/src/parse_tree.h b/src/parse_tree.h index e69b27c1c..78f04d1ee 100644 --- a/src/parse_tree.h +++ b/src/parse_tree.h @@ -200,10 +200,12 @@ class parse_node_tree_t : public std::vector { /// only the second or additional commands are. bool statement_is_in_pipeline(const parse_node_t &node, bool include_first) const; - /// If the given node is a block statement, returns the header node (for_header, while_header, - /// begin_header, or function_header). Otherwise returns NULL. - const parse_node_t *header_node_for_block_statement(const parse_node_t &node) const; + /// Given a node, return all of its comment nodes. + parse_node_list_t comment_nodes_for_node(const parse_node_t &node) const; + private: + template + friend class tnode_t; /// Given a node list (e.g. of type symbol_job_list) and a node type (e.g. symbol_job), return /// the next element of the given type in that list, and the tail (by reference). Returns NULL /// if we've exhausted the list. @@ -211,10 +213,6 @@ class parse_node_tree_t : public std::vector { parse_token_type_t item_type, const parse_node_t **list_tail) const; - /// Given a node, return all of its comment nodes. - parse_node_list_t comment_nodes_for_node(const parse_node_t &node) const; - - private: // Finds the last node of a given type underneath a given node, or NULL if it could not be // found. If parent is NULL, this finds the last node in the tree of that type. const parse_node_t *find_last_node_of_type(parse_token_type_t type, @@ -266,6 +264,13 @@ class tnode_t { assert((!n || n->type == Type::token) && "node has wrong type"); } + // Try to create a tnode from the given tree and parse node. + // Returns an empty node if the parse node is null, or has the wrong type. + static tnode_t try_create(const parse_node_tree_t *tree, const parse_node_t *node) { + assert(tree && "tree cannot be null"); + return tnode_t(tree, node && node->type == Type::token ? node : nullptr); + } + /// Temporary conversion to parse_node_t to assist in migration. /* implicit */ operator const parse_node_t &() const { assert(nodeptr && "Empty tnode_t"); diff --git a/src/parse_util.cpp b/src/parse_util.cpp index 0753c4918..081617676 100644 --- a/src/parse_util.cpp +++ b/src/parse_util.cpp @@ -1183,6 +1183,7 @@ parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, if (maybe_t mcommand = command_for_plain_statement({&node_tree, &node}, buff_src)) { + using namespace grammar; wcstring command = std::move(*mcommand); // Check that we can expand the command. if (!expand_one(command, @@ -1202,17 +1203,16 @@ parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, // Check that we don't return from outside a function. But we allow it if it's // 'return --help'. if (!errored && command == L"return") { - const parse_node_t *ancestor = &node; bool found_function = false; - while (ancestor != NULL) { - const parse_node_t *possible_function_header = - node_tree.header_node_for_block_statement(*ancestor); - if (possible_function_header != NULL && - possible_function_header->type == symbol_function_header) { + for (const parse_node_t *ancestor = &node; ancestor != nullptr; + ancestor = node_tree.get_parent(*ancestor)) { + auto fh = tnode_t::try_create(&node_tree, ancestor) + .child<0>() + .try_get_child(); + if (fh) { found_function = true; break; } - ancestor = node_tree.get_parent(*ancestor); } if (!found_function && !first_argument_is_help(node_tree, node, buff_src)) { errored = append_syntax_error(&parse_errors, node.source_start, @@ -1227,35 +1227,23 @@ parser_test_error_bits_t parse_util_detect_errors(const wcstring &buff_src, // This is a little funny because we can't tell if it's a 'for' or 'while' // loop from the ancestor alone; we need the header. That is, we hit a // block_statement, and have to check its header. - bool found_loop = false, end_search = false; - const parse_node_t *ancestor = &node; - while (ancestor != NULL && !end_search) { - const parse_node_t *loop_or_function_header = - node_tree.header_node_for_block_statement(*ancestor); - if (loop_or_function_header != NULL) { - switch (loop_or_function_header->type) { - case symbol_while_header: - case symbol_for_header: { - // This is a loop header, so we can break or continue. - found_loop = true; - end_search = true; - break; - } - case symbol_function_header: { - // This is a function header, so we cannot break or - // continue. We stop our search here. - found_loop = false; - end_search = true; - break; - } - default: { - // Most likely begin / end style block, which makes no - // difference. - break; - } - } + bool found_loop = false; + for (const parse_node_t *ancestor = &node; ancestor != nullptr; + ancestor = node_tree.get_parent(*ancestor)) { + tnode_t bh = + tnode_t::try_create(&node_tree, ancestor) + .child<0>(); + if (bh.try_get_child() || + bh.try_get_child()) { + // This is a loop header, so we can break or continue. + found_loop = true; + break; + } else if (bh.try_get_child()) { + // This is a function header, so we cannot break or + // continue. We stop our search here. + found_loop = false; + break; } - ancestor = node_tree.get_parent(*ancestor); } if (!found_loop && !first_argument_is_help(node_tree, node, buff_src)) {