From dbae271fe79c4093130ee542713f8cae275eb247 Mon Sep 17 00:00:00 2001 From: Peter Ammon Date: Sun, 27 Apr 2025 13:09:56 -0700 Subject: [PATCH] ast: remove StatementVariant Statement is the new StatementVariant. --- src/ast.rs | 184 +++++++++++++------------------------ src/highlight/highlight.rs | 2 +- src/parse_execution.rs | 145 ++++++++++++----------------- src/tests/parser.rs | 1 - 4 files changed, 122 insertions(+), 210 deletions(-) diff --git a/src/ast.rs b/src/ast.rs index 9ab9c3fbf..493a267d8 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -69,7 +69,7 @@ fn visit_block_statement_header( &mut self, _node: &mut BlockStatementHeaderVariant, ) -> VisitResult; - fn visit_statement(&mut self, _node: &mut StatementVariant) -> VisitResult; + fn visit_statement(&mut self, _node: &mut Statement) -> VisitResult; fn visit_decorated_statement_decorator( &mut self, @@ -944,9 +944,6 @@ macro_rules! visit_variant_field_mut { (BlockStatementHeaderVariant, $visitor:ident, $field:expr) => { $visitor.visit_block_statement_header(&mut $field) }; - (StatementVariant, $visitor:ident, $field:expr) => { - $visitor.visit_statement(&mut $field) - }; } macro_rules! visit_optional_field { @@ -1098,13 +1095,59 @@ fn can_be_parsed(pop: &mut Populator<'_>) -> bool { ); /// A statement is a normal command, or an if / while / etc -#[derive(Default, Debug)] -pub struct Statement { - pub contents: StatementVariant, +#[derive(Debug)] +pub enum Statement { + Decorated(DecoratedStatement), + Not(Box), + Block(Box), + Brace(Box), + If(Box), + Switch(Box), } implement_node!(Statement, statement); impl NodeSubTraits for Statement {} -implement_acceptor_for_branch!(Statement, (contents: (variant))); + +impl Default for Statement { + fn default() -> Self { + Self::Decorated(DecoratedStatement::default()) + } +} + +impl Statement { + // Convenience function to get this statement as a decorated statement, if it is one. + pub fn as_decorated_statement(&self) -> Option<&DecoratedStatement> { + match self { + Self::Decorated(child) => Some(child), + _ => None, + } + } + + // Return the node embedded in this statement. + fn embedded_node(&self) -> &dyn Node { + match self { + Self::Not(child) => &**child, + Self::Block(child) => &**child, + Self::Brace(child) => &**child, + Self::If(child) => &**child, + Self::Switch(child) => &**child, + Self::Decorated(child) => child, + } + } +} + +impl Acceptor for Statement { + fn accept<'a>(&'a self, visitor: &mut dyn NodeVisitor<'a>) { + visitor.visit(self.embedded_node()); + } +} + +impl AcceptorMut for Statement { + fn accept_mut(&mut self, visitor: &mut dyn NodeVisitorMut) { + visitor.will_visit_fields_of(self); + let flow = visitor.visit_statement(self); + visitor.did_visit_fields_of(self, flow); + } +} /// A job is a non-empty list of statements, separated by pipes. (Non-empty is useful for cases /// like if statements, where we require a command). @@ -1582,7 +1625,7 @@ fn can_be_parsed(pop: &mut Populator<'_>) -> bool { ParseTokenType::terminate => pop.allow_incomplete(), // We have e.g. `a= >` which is an error. // Note that we do not produce an error here. Instead we return false - // so this the token will be seen by allocate_populate_statement_contents. + // so this the token will be seen by allocate_populate_statement. _ => false, } } @@ -1783,104 +1826,6 @@ fn embedded_node(&self) -> &dyn NodeMut { } } -#[derive(Debug)] -pub enum StatementVariant { - NotStatement(Box), - BlockStatement(Box), - BraceStatement(Box), - IfStatement(Box), - SwitchStatement(Box), - DecoratedStatement(DecoratedStatement), -} - -impl Default for StatementVariant { - fn default() -> Self { - StatementVariant::DecoratedStatement(DecoratedStatement::default()) - } -} - -impl Acceptor for StatementVariant { - fn accept<'a>(&'a self, visitor: &mut dyn NodeVisitor<'a>) { - match self { - StatementVariant::NotStatement(node) => node.accept(visitor), - StatementVariant::BlockStatement(node) => node.accept(visitor), - StatementVariant::BraceStatement(node) => node.accept(visitor), - StatementVariant::IfStatement(node) => node.accept(visitor), - StatementVariant::SwitchStatement(node) => node.accept(visitor), - StatementVariant::DecoratedStatement(node) => node.accept(visitor), - } - } -} -impl AcceptorMut for StatementVariant { - fn accept_mut(&mut self, visitor: &mut dyn NodeVisitorMut) { - match self { - StatementVariant::NotStatement(node) => node.accept_mut(visitor), - StatementVariant::BlockStatement(node) => node.accept_mut(visitor), - StatementVariant::BraceStatement(node) => node.accept_mut(visitor), - StatementVariant::IfStatement(node) => node.accept_mut(visitor), - StatementVariant::SwitchStatement(node) => node.accept_mut(visitor), - StatementVariant::DecoratedStatement(node) => node.accept_mut(visitor), - } - } -} - -impl StatementVariant { - pub fn typ(&self) -> Type { - self.embedded_node().typ() - } - pub fn try_source_range(&self) -> Option { - self.embedded_node().try_source_range() - } - - pub fn as_not_statement(&self) -> Option<&NotStatement> { - match self { - StatementVariant::NotStatement(node) => Some(node), - _ => None, - } - } - pub fn as_block_statement(&self) -> Option<&BlockStatement> { - match self { - StatementVariant::BlockStatement(node) => Some(node), - _ => None, - } - } - pub fn as_brace_statement(&self) -> Option<&BraceStatement> { - match self { - StatementVariant::BraceStatement(node) => Some(node), - _ => None, - } - } - pub fn as_if_statement(&self) -> Option<&IfStatement> { - match self { - StatementVariant::IfStatement(node) => Some(node), - _ => None, - } - } - pub fn as_switch_statement(&self) -> Option<&SwitchStatement> { - match self { - StatementVariant::SwitchStatement(node) => Some(node), - _ => None, - } - } - pub fn as_decorated_statement(&self) -> Option<&DecoratedStatement> { - match self { - StatementVariant::DecoratedStatement(node) => Some(node), - _ => None, - } - } - - fn embedded_node(&self) -> &dyn NodeMut { - match self { - StatementVariant::NotStatement(node) => &**node, - StatementVariant::BlockStatement(node) => &**node, - StatementVariant::BraceStatement(node) => &**node, - StatementVariant::IfStatement(node) => &**node, - StatementVariant::SwitchStatement(node) => &**node, - StatementVariant::DecoratedStatement(node) => node, - } - } -} - /// Return a string literal name for an ast type. pub fn ast_type_to_string(t: Type) -> &'static wstr { match t { @@ -2574,8 +2519,8 @@ fn visit_block_statement_header( *node = self.allocate_populate_block_header(); VisitResult::Continue(()) } - fn visit_statement(&mut self, node: &mut StatementVariant) -> VisitResult { - *node = self.allocate_populate_statement_contents(); + fn visit_statement(&mut self, node: &mut Statement) -> VisitResult { + *node = self.allocate_populate_statement(); VisitResult::Continue(()) } @@ -3091,17 +3036,16 @@ fn populate_list(&mut self, list: &mut ListType, exhaust ); } - /// Allocate and populate a statement contents pointer. - /// This must never return null. - fn allocate_populate_statement_contents(&mut self) -> StatementVariant { + /// Allocate and populate a statement. + fn allocate_populate_statement(&mut self) -> Statement { // In case we get a parse error, we still need to return something non-null. Use a // decorated statement; all of its leaf nodes will end up unsourced. - fn got_error(slf: &mut Populator<'_>) -> StatementVariant { + fn got_error(slf: &mut Populator<'_>) -> Statement { assert!(slf.unwinding, "Should have produced an error"); new_decorated_statement(slf) } - fn new_decorated_statement(slf: &mut Populator<'_>) -> StatementVariant { + fn new_decorated_statement(slf: &mut Populator<'_>) -> Statement { let embedded = slf.allocate_visit::(); if !slf.unwinding && slf.peek_token(0).typ == ParseTokenType::left_brace { parse_error!( @@ -3116,7 +3060,7 @@ fn new_decorated_statement(slf: &mut Populator<'_>) -> StatementVariant { slf.peek_token(0).user_presentable_description() ); } - StatementVariant::DecoratedStatement(embedded) + Statement::Decorated(embedded) } if self.peek_token(0).typ == ParseTokenType::terminate && self.allow_incomplete() { @@ -3125,7 +3069,7 @@ fn new_decorated_statement(slf: &mut Populator<'_>) -> StatementVariant { self.allocate_visit::(); } else if self.peek_token(0).typ == ParseTokenType::left_brace { let embedded = self.allocate_boxed_visit::(); - return StatementVariant::BraceStatement(embedded); + return Statement::Brace(embedded); } else if self.peek_token(0).typ != ParseTokenType::string { // We may be unwinding already; do not produce another error. // For example in `true | and`. @@ -3196,22 +3140,22 @@ fn new_decorated_statement(slf: &mut Populator<'_>) -> StatementVariant { match self.peek_token(0).keyword { ParseKeyword::Not | ParseKeyword::Exclam => { let embedded = self.allocate_boxed_visit::(); - StatementVariant::NotStatement(embedded) + Statement::Not(embedded) } ParseKeyword::For | ParseKeyword::While | ParseKeyword::Function | ParseKeyword::Begin => { let embedded = self.allocate_boxed_visit::(); - StatementVariant::BlockStatement(embedded) + Statement::Block(embedded) } ParseKeyword::If => { let embedded = self.allocate_boxed_visit::(); - StatementVariant::IfStatement(embedded) + Statement::If(embedded) } ParseKeyword::Switch => { let embedded = self.allocate_boxed_visit::(); - StatementVariant::SwitchStatement(embedded) + Statement::Switch(embedded) } ParseKeyword::End => { // 'end' is forbidden as a command. diff --git a/src/highlight/highlight.rs b/src/highlight/highlight.rs index 2f52fbe5c..d42d797a9 100644 --- a/src/highlight/highlight.rs +++ b/src/highlight/highlight.rs @@ -279,7 +279,7 @@ fn autosuggest_parse_command( // Find the first statement. let jc = ast.top().as_job_list().unwrap().get(0)?; - let first_statement = jc.job.statement.contents.as_decorated_statement()?; + let first_statement = jc.job.statement.as_decorated_statement()?; if let Some(expanded_command) = statement_get_expanded_command(buff, first_statement, ctx) { let mut arg = WString::new(); diff --git a/src/parse_execution.rs b/src/parse_execution.rs index 54583735f..aa00b44f7 100644 --- a/src/parse_execution.rs +++ b/src/parse_execution.rs @@ -1,8 +1,7 @@ //! Provides the "linkage" between an ast and actual execution structures (job_t, etc.). use crate::ast::{ - self, unescape_keyword, BlockStatementHeaderVariant, Keyword, Leaf, Node, StatementVariant, - Token, + self, unescape_keyword, BlockStatementHeaderVariant, Keyword, Leaf, Node, Statement, Token, }; use crate::builtins; use crate::builtins::shared::{ @@ -160,20 +159,14 @@ fn eval_statement( associated_block: Option, ) -> EndExecutionReason { // Note we only expect block-style statements here. No not statements. - match &statement.contents { - StatementVariant::BlockStatement(block) => { - self.run_block_statement(ctx, block, associated_block) - } - StatementVariant::BraceStatement(brace_statement) => { + match &statement { + Statement::Block(block) => self.run_block_statement(ctx, block, associated_block), + Statement::Brace(brace_statement) => { self.run_begin_statement(ctx, &brace_statement.jobs) } - StatementVariant::IfStatement(ifstat) => { - self.run_if_statement(ctx, ifstat, associated_block) - } - StatementVariant::SwitchStatement(switchstat) => { - self.run_switch_statement(ctx, switchstat) - } - StatementVariant::DecoratedStatement(_) | StatementVariant::NotStatement(_) => panic!(), + Statement::If(ifstat) => self.run_if_statement(ctx, ifstat, associated_block), + Statement::Switch(switchstat) => self.run_switch_statement(ctx, switchstat), + Statement::Decorated(_) | Statement::Not(_) => panic!(), } } @@ -416,7 +409,7 @@ fn infinite_recursive_statement_in_job_list<'b>( // Helper to return if a statement is infinitely recursive in this function. let statement_recurses = |stat: &'b ast::Statement| -> Option<&'b ast::DecoratedStatement> { // Ignore non-decorated statements like `if`, etc. - let StatementVariant::DecoratedStatement(dc) = &stat.contents else { + let Statement::Decorated(dc) = &stat else { return None; }; @@ -558,14 +551,13 @@ fn job_is_simple_block(&self, job: &ast::JobPipeline) -> bool { let no_redirs = |list: &ast::ArgumentOrRedirectionList| !list.iter().any(|val| val.is_redirection()); - // Check if we're a block statement with redirections. We do it this obnoxious way to preserve - // type safety (in case we add more specific statement types). - match &job.statement.contents { - StatementVariant::BlockStatement(stmt) => no_redirs(&stmt.args_or_redirs), - StatementVariant::BraceStatement(stmt) => no_redirs(&stmt.args_or_redirs), - StatementVariant::SwitchStatement(stmt) => no_redirs(&stmt.args_or_redirs), - StatementVariant::IfStatement(stmt) => no_redirs(&stmt.args_or_redirs), - StatementVariant::NotStatement(_) | StatementVariant::DecoratedStatement(_) => { + // Check if we're a block statement with redirections. + match &job.statement { + Statement::Block(stmt) => no_redirs(&stmt.args_or_redirs), + Statement::Brace(stmt) => no_redirs(&stmt.args_or_redirs), + Statement::Switch(stmt) => no_redirs(&stmt.args_or_redirs), + Statement::If(stmt) => no_redirs(&stmt.args_or_redirs), + Statement::Not(_) | Statement::Decorated(_) => { // not block statements false } @@ -661,9 +653,6 @@ fn populate_job_process( statement: &ast::Statement, variable_assignments: &ast::VariableAssignmentList, ) -> EndExecutionReason { - // Get the "specific statement" which is boolean / block / if / switch / decorated. - let specific_statement = &statement.contents; - let mut block = None; let result = self.apply_variable_assignments(ctx, Some(proc), variable_assignments, &mut block); @@ -676,17 +665,14 @@ fn populate_job_process( return result; } - match &specific_statement { - StatementVariant::NotStatement(not_statement) => { + match &statement { + Statement::Not(not_statement) => { self.populate_not_process(ctx, job, proc, not_statement) } - StatementVariant::BlockStatement(_) - | StatementVariant::BraceStatement(_) - | StatementVariant::IfStatement(_) - | StatementVariant::SwitchStatement(_) => { - self.populate_block_process(ctx, proc, statement, specific_statement) + Statement::Block(_) | Statement::Brace(_) | Statement::If(_) | Statement::Switch(_) => { + self.populate_block_process(ctx, proc, statement) } - StatementVariant::DecoratedStatement(decorated_statement) => { + Statement::Decorated(decorated_statement) => { self.populate_plain_process(ctx, proc, decorated_statement) } } @@ -837,17 +823,16 @@ fn populate_block_process( ctx: &OperationContext<'_>, proc: &mut Process, statement: &ast::Statement, - specific_statement: &ast::StatementVariant, ) -> EndExecutionReason { - // We handle block statements by creating process_type_t::block_node, that will bounce back to + // We handle block statements by creating ProcessType::block_node, that will bounce back to // us when it's time to execute them. // Get the argument or redirections list. // TODO: args_or_redirs should be available without resolving the statement type. - let args_or_redirs = match specific_statement { - StatementVariant::BlockStatement(block_statement) => &block_statement.args_or_redirs, - StatementVariant::BraceStatement(brace_statement) => &brace_statement.args_or_redirs, - StatementVariant::IfStatement(if_statement) => &if_statement.args_or_redirs, - StatementVariant::SwitchStatement(switch_statement) => &switch_statement.args_or_redirs, + let args_or_redirs = match statement { + Statement::Block(block_statement) => &block_statement.args_or_redirs, + Statement::Brace(brace_statement) => &brace_statement.args_or_redirs, + Statement::If(if_statement) => &if_statement.args_or_redirs, + Statement::Switch(switch_statement) => &switch_statement.args_or_redirs, _ => panic!("Unexpected block node type"), }; @@ -1590,27 +1575,21 @@ fn run_1_job( } }); - let specific_statement = &job_node.statement.contents; - assert!(specific_statement_type_is_redirectable_block( - specific_statement - )); + let statement = &job_node.statement; + assert!(statement_is_redirectable_block(statement)); if result == EndExecutionReason::ok { - result = match &specific_statement { - StatementVariant::BlockStatement(block_statement) => { + result = match statement { + Statement::Block(block_statement) => { self.run_block_statement(ctx, block_statement, associated_block) } - StatementVariant::BraceStatement(brace_statement) => { + Statement::Brace(brace_statement) => { self.run_begin_statement(ctx, &brace_statement.jobs) } - StatementVariant::IfStatement(ifstmt) => { - self.run_if_statement(ctx, ifstmt, associated_block) - } - StatementVariant::SwitchStatement(switchstmt) => { - self.run_switch_statement(ctx, switchstmt) - } + Statement::If(ifstmt) => self.run_if_statement(ctx, ifstmt, associated_block), + Statement::Switch(switchstmt) => self.run_switch_statement(ctx, switchstmt), // Other types should be impossible due to the - // specific_statement_type_is_redirectable_block check. - StatementVariant::NotStatement(_) | StatementVariant::DecoratedStatement(_) => { + // statement_is_redirectable_block check. + Statement::Not(_) | Statement::Decorated(_) => { panic!() } }; @@ -1623,7 +1602,7 @@ fn run_1_job( profile_item.duration = ProfileItem::now() - start_time; profile_item.level = ctx.parser().scope().eval_level; profile_item.cmd = - profiling_cmd_name_for_redirectable_block(specific_statement, self.pstree()); + profiling_cmd_name_for_redirectable_block(statement, self.pstree()); profile_item.skipped = false; } @@ -1927,32 +1906,24 @@ enum Globspec { } type AstArgsList<'a> = Vec<&'a ast::Argument>; -/// These are the specific statement types that support redirections. -fn type_is_redirectable_block(typ: ast::Type) -> bool { - [ - ast::Type::block_statement, - ast::Type::brace_statement, - ast::Type::if_statement, - ast::Type::switch_statement, - ] - .contains(&typ) -} - -fn specific_statement_type_is_redirectable_block(node: &ast::StatementVariant) -> bool { - type_is_redirectable_block(node.typ()) +fn statement_is_redirectable_block(node: &ast::Statement) -> bool { + match node { + Statement::Decorated(_) | Statement::Not(_) => false, + Statement::Block(_) | Statement::Brace(_) | Statement::If(_) | Statement::Switch(_) => true, + } } /// Get the name of a redirectable block, for profiling purposes. fn profiling_cmd_name_for_redirectable_block( - node: &ast::StatementVariant, + node: &ast::Statement, pstree: &ParsedSourceRef, ) -> WString { - assert!(specific_statement_type_is_redirectable_block(node)); + assert!(statement_is_redirectable_block(node)); let source_range = node.try_source_range().expect("No source range for block"); let src_end = match node { - StatementVariant::BlockStatement(block_statement) => { + Statement::Block(block_statement) => { let block_header = &block_statement.header; match block_header { BlockStatementHeaderVariant::ForHeader(for_header) => { @@ -1970,13 +1941,9 @@ fn profiling_cmd_name_for_redirectable_block( BlockStatementHeaderVariant::None => panic!("Unexpected block header type"), } } - StatementVariant::BraceStatement(brace_statement) => { - brace_statement.left_brace.source_range().start() - } - StatementVariant::IfStatement(ifstmt) => { - ifstmt.if_clause.condition.job.source_range().end() - } - StatementVariant::SwitchStatement(switchstmt) => switchstmt.semi_nl.source_range().start(), + Statement::Brace(brace_statement) => brace_statement.left_brace.source_range().start(), + Statement::If(ifstmt) => ifstmt.if_clause.condition.job.source_range().end(), + Statement::Switch(switchstmt) => switchstmt.semi_nl.source_range().start(), _ => { panic!("Not a redirectable block_type"); } @@ -2007,17 +1974,19 @@ fn job_node_wants_timing(job_node: &ast::JobPipeline) -> bool { } // Helper to return true if a node is 'not time ...' or 'not not time...' or... - let is_timed_not_statement = |mut stat: &ast::Statement| loop { - match &stat.contents { - StatementVariant::NotStatement(ns) => { - if ns.time.is_some() { - return true; + fn is_timed_not_statement(mut stat: &ast::Statement) -> bool { + loop { + match &stat { + Statement::Not(ns) => { + if ns.time.is_some() { + return true; + } + stat = &ns.contents; } - stat = &ns.contents; + _ => return false, } - _ => return false, } - }; + } // Do we have a 'not time ...' anywhere in our pipeline? if is_timed_not_statement(&job_node.statement) { diff --git a/src/tests/parser.rs b/src/tests/parser.rs index e8ebab6df..f24523a8c 100644 --- a/src/tests/parser.rs +++ b/src/tests/parser.rs @@ -839,7 +839,6 @@ fn new(ast: &'a Ast) -> Self { let statement = &job_pipeline.statement; let decorated_statement = statement - .contents .as_decorated_statement() .expect("Expected decorated_statement"); let command = &decorated_statement.command;