From c212ac95e9155b99e20640428aa021d9e76b581f Mon Sep 17 00:00:00 2001 From: Peter Ammon Date: Sat, 29 Jun 2024 14:56:03 -0700 Subject: [PATCH] Thread a reference to a line counter into parse execution Simplify Parser by removing the reference to the execution context --- src/parse_execution.rs | 22 +++++--------- src/parse_tree.rs | 5 +++ src/parser.rs | 69 +++++++++++++++--------------------------- 3 files changed, 37 insertions(+), 59 deletions(-) diff --git a/src/parse_execution.rs b/src/parse_execution.rs index 11bc840bb..45f011660 100644 --- a/src/parse_execution.rs +++ b/src/parse_execution.rs @@ -83,23 +83,14 @@ pub struct ExecutionContext { cancel_signal: RefCell>, // Helper to count lines. - line_counter: RefCell>, + // This is shared with the Parser so that the Parser can access the current line. + line_counter: Rc>>, /// The block IO chain. /// For example, in `begin; foo ; end < file.txt` this would have the 'file.txt' IO. block_io: RefCell, } -impl ExecutionContext { - pub fn swap(left: &Self, right: Self) -> Self { - left.pstree.swap(&right.pstree); - left.cancel_signal.swap(&right.cancel_signal); - left.line_counter.swap(&right.line_counter); - left.block_io.swap(&right.block_io); - right - } -} - // Report an error, setting $status to `status`. Always returns // 'end_execution_reason_t::error'. macro_rules! report_error { @@ -123,12 +114,15 @@ macro_rules! report_error_formatted { impl<'a> ExecutionContext { /// Construct a context in preparation for evaluating a node in a tree, with the given block_io. /// The execution context may access the parser and parent job group (if any) through ctx. - pub fn new(pstree: ParsedSourceRef, block_io: IoChain) -> Self { - let line_counter = pstree.line_counter(); + pub fn new( + pstree: ParsedSourceRef, + block_io: IoChain, + line_counter: Rc>>, + ) -> Self { Self { pstree: RefCell::new(pstree), cancel_signal: RefCell::default(), - line_counter: RefCell::new(line_counter), + line_counter, block_io: RefCell::new(block_io), } } diff --git a/src/parse_tree.rs b/src/parse_tree.rs index eae46c362..6419eb77a 100644 --- a/src/parse_tree.rs +++ b/src/parse_tree.rs @@ -267,4 +267,9 @@ impl LineCounter { let prev = std::mem::replace(&mut self.node, node_ptr); unsafe { prev.as_ref() } } + + // Return the source. + pub fn get_source(&self) -> &wstr { + &self.parsed_source.src + } } diff --git a/src/parser.rs b/src/parser.rs index cfe92c6bb..8ec2628a7 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -1,6 +1,6 @@ // The fish parser. Contains functions for parsing and evaluating code. -use crate::ast::{Ast, List, Node}; +use crate::ast::{self, Ast, List, Node}; use crate::builtins::shared::STATUS_ILLEGAL_CMD; use crate::common::{ escape_string, scoped_push_replacer, CancelChecker, EscapeFlags, EscapeStringStyle, @@ -22,7 +22,7 @@ use crate::parse_constants::{ SOURCE_LOCATION_UNKNOWN, }; use crate::parse_execution::{EndExecutionReason, ExecutionContext}; -use crate::parse_tree::{parse_source, ParsedSourceRef}; +use crate::parse_tree::{parse_source, LineCounter, ParsedSourceRef}; use crate::proc::{job_reap, JobGroupRef, JobList, JobRef, ProcStatus}; use crate::signal::{signal_check_cancel, signal_clear_cancel, Signal}; use crate::threads::assert_is_main_thread; @@ -362,8 +362,9 @@ pub enum CancelBehavior { } pub struct Parser { - /// The current execution context. - execution_context: RefCell>, + /// A shared line counter. This is handed out to each execution context + /// so they can communicate the line number back to this Parser. + line_counter: Rc>>, /// The jobs associated with this parser. job_list: RefCell, @@ -404,7 +405,7 @@ impl Parser { /// Create a parser. pub fn new(variables: Rc, cancel_behavior: CancelBehavior) -> Parser { let result = Self { - execution_context: RefCell::default(), + line_counter: Rc::new(RefCell::new(LineCounter::empty())), job_list: RefCell::default(), wait_handles: RefCell::new(WaitHandleStore::new()), block_list: RefCell::default(), @@ -429,10 +430,6 @@ impl Parser { result } - fn execution_context(&self) -> Ref<'_, Option> { - self.execution_context.borrow() - } - /// Adds a job to the beginning of the job list. pub fn job_add(&self, job: JobRef) { assert!(!job.processes().is_empty()); @@ -596,35 +593,23 @@ impl Parser { let cancel_checker: CancelChecker = Box::new(move || check_cancel_signal().is_some()); op_ctx.cancel_checker = cancel_checker; - // Create and set a new execution context. - let exc = scoped_push_replacer( - |new_value| { - if self.execution_context.borrow().is_none() || new_value.is_none() { - // Outermost node. - std::mem::replace(&mut self.execution_context.borrow_mut(), new_value) - } else { - #[allow(clippy::unnecessary_unwrap)] - Some(ExecutionContext::swap( - self.execution_context.borrow().as_ref().unwrap(), - new_value.unwrap(), - )) - } - }, - Some(ExecutionContext::new(ps.clone(), block_io.clone())), - ); + // Restore the line counter. + let line_counter = Rc::clone(&self.line_counter); + let scoped_line_counter = + scoped_push_replacer(|v| line_counter.replace(v), ps.line_counter()); + + // Create a new execution context. + let execution_context = + ExecutionContext::new(ps.clone(), block_io.clone(), Rc::clone(&line_counter)); // Check the exec count so we know if anything got executed. let prev_exec_count = self.libdata().exec_count; let prev_status_count = self.libdata().status_count; - let reason = - self.execution_context() - .as_ref() - .unwrap() - .eval_node(&op_ctx, node, Some(scope_block)); + let reason = execution_context.eval_node(&op_ctx, node, Some(scope_block)); let new_exec_count = self.libdata().exec_count; let new_status_count = self.libdata().status_count; - ScopeGuarding::commit(exc); + ScopeGuarding::commit(scoped_line_counter); self.pop_block(scope_block); job_reap(self, false); // reap again @@ -678,15 +663,7 @@ impl Parser { /// /// init.fish (line 127): ls|grep pancake pub fn current_line(&self) -> WString { - if self.execution_context().is_none() { - return WString::new(); - }; - let Some(source_offset) = self - .execution_context() - .as_ref() - .unwrap() - .get_current_source_offset() - else { + let Some(source_offset) = self.line_counter.borrow_mut().source_offset_of_node() else { return WString::new(); }; @@ -717,7 +694,7 @@ impl Parser { empty_error.source_start = source_offset; let mut line_info = empty_error.describe_with_prefix( - &self.execution_context().as_ref().unwrap().get_source(), + self.line_counter.borrow().get_source(), &prefix, self.is_interactive(), skip_caret, @@ -730,11 +707,13 @@ impl Parser { line_info } - /// Returns the current line number. + /// Returns the current line number, indexed from 1. pub fn get_lineno(&self) -> Option { - self.execution_context() - .as_ref() - .and_then(|ctx| ctx.get_current_line_number()) + // The offset is 0 based; the number is 1 based. + self.line_counter + .borrow_mut() + .line_offset_of_node() + .map(|offset| offset + 1) } /// Return whether we are currently evaluating a "block" such as an if statement.