From e0953cac41f439c11c949841f76d87f8708e6eb3 Mon Sep 17 00:00:00 2001 From: Peter Ammon Date: Sat, 8 Mar 2025 11:03:37 -0800 Subject: [PATCH] Use new scoped types for line_counter and caller_id --- src/builtins/function.rs | 2 +- src/parse_execution.rs | 21 ++++++++------------- src/parse_tree.rs | 10 +--------- src/parser.rs | 23 ++++++++++++----------- src/tests/parser.rs | 17 ++--------------- 5 files changed, 24 insertions(+), 49 deletions(-) diff --git a/src/builtins/function.rs b/src/builtins/function.rs index 2f6a36484..275b58b97 100644 --- a/src/builtins/function.rs +++ b/src/builtins/function.rs @@ -135,7 +135,7 @@ fn parse_cmd_opts( let e: EventDescription; if opt == 'j' && woptarg == "caller" { let caller_id = if parser.scope().is_subshell { - parser.libdata().caller_id + parser.scope().caller_id } else { 0 }; diff --git a/src/parse_execution.rs b/src/parse_execution.rs index b220ae5c5..e3a065633 100644 --- a/src/parse_execution.rs +++ b/src/parse_execution.rs @@ -11,8 +11,8 @@ STATUS_UNMATCHED_WILDCARD, }; use crate::common::{ - escape, scoped_push_replacer, should_suppress_stderr_for_tests, truncate_at_nul, - valid_var_name, ScopeGuard, ScopeGuarding, + escape, should_suppress_stderr_for_tests, truncate_at_nul, valid_var_name, ScopeGuard, + ScopeGuarding, ScopedRefCell, }; use crate::complete::CompletionList; use crate::env::{EnvMode, EnvStackSetResult, EnvVar, EnvVarFlags, Environment, Statuses}; @@ -52,7 +52,6 @@ use crate::wildcard::wildcard_match; use crate::wutil::{wgettext, wgettext_maybe_fmt}; use libc::{c_int, ENOTDIR, EXIT_SUCCESS, STDERR_FILENO, STDOUT_FILENO}; -use std::cell::RefCell; use std::io::ErrorKind; use std::rc::Rc; use std::sync::Arc; @@ -85,7 +84,7 @@ pub struct ExecutionContext { // Helper to count lines. // This is shared with the Parser so that the Parser can access the current line. - line_counter: Rc>>, + line_counter: Rc>>, /// The block IO chain. /// For example, in `begin; foo ; end < file.txt` this would have the 'file.txt' IO. @@ -118,7 +117,7 @@ impl<'a> ExecutionContext { pub fn new( pstree: ParsedSourceRef, block_io: IoChain, - line_counter: Rc>>, + line_counter: Rc>>, ) -> Self { Self { pstree, @@ -1533,10 +1532,7 @@ fn run_1_job( // Save the executing node. let line_counter = Rc::clone(&self.line_counter); - let _saved_node = scoped_push_replacer( - |node| line_counter.borrow_mut().set_node(node), - Some(job_node), - ); + let _saved_node = line_counter.scoped_set(job_node as *const _, |s| &mut s.node); // Profiling support. let profile_item_id = ctx.parser().create_profile_item(); @@ -1631,10 +1627,9 @@ fn run_1_job( // We are about to populate a job. One possible argument to the job is a command substitution // which may be interested in the job that's populating it, via '--on-job-exit caller'. Record // the job ID here. - let _caller_id = scoped_push_replacer( - |new_value| std::mem::replace(&mut ctx.parser().libdata_mut().caller_id, new_value), - job.internal_job_id, - ); + let _caller_id = ctx + .parser() + .push_scope(move |s| s.caller_id = job.internal_job_id); // Populate the job. This may fail for reasons like command_not_found. If this fails, an error // will have been printed. diff --git a/src/parse_tree.rs b/src/parse_tree.rs index eb940eba6..bfa4d2912 100644 --- a/src/parse_tree.rs +++ b/src/parse_tree.rs @@ -204,7 +204,7 @@ pub struct LineCounter { parsed_source: Pin>, /// The node itself. This points into the parsed source, or it may be null. - node: *const NodeType, + pub node: *const NodeType, // Cached line number information: the line number of the start of the node, and the number of newlines. cached_offset: usize, @@ -262,14 +262,6 @@ pub fn source_offset_of_node(&mut self) -> Option { Some(range.start()) } - // Set the node. The node must belong to the parsed source. - // Returns the original node. - pub fn set_node<'a>(&mut self, node: Option<&'a NodeType>) -> Option<&'a NodeType> { - let node_ptr = node.map_or(std::ptr::null(), |node| node); - 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 c7f2d9072..4a4327537 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -3,8 +3,8 @@ 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, - FilenameRef, ScopeGuarding, ScopedCell, PROFILING_ACTIVE, + escape_string, CancelChecker, EscapeFlags, EscapeStringStyle, FilenameRef, ScopeGuarding, + ScopedCell, ScopedRefCell, PROFILING_ACTIVE, }; use crate::complete::CompletionList; use crate::env::{EnvMode, EnvStack, EnvStackSetResult, Environment, Statuses}; @@ -241,6 +241,10 @@ pub struct ScopedData { /// Whether we are currently cleaning processes. pub is_cleaning_procs: bool, + + /// The internal job id of the job being populated, or 0 if none. + /// This supports the '--on-job-exit caller' feature. + pub caller_id: u64, // TODO should be InternalJobId } impl Default for ScopedData { @@ -253,6 +257,7 @@ fn default() -> Self { suppress_fish_trace: false, read_limit: 0, is_cleaning_procs: false, + caller_id: 0, } } } @@ -300,10 +305,6 @@ pub struct LibraryData { /// Whether we called builtin_complete -C without parameter. pub builtin_complete_current_commandline: bool, - /// The internal job id of the job being populated, or 0 if none. - /// This supports the '--on-job-exit caller' feature. - pub caller_id: u64, // TODO should be InternalJobId - /// Whether we should break or continue the current loop. /// This is set by the 'break' and 'continue' commands. pub loop_status: LoopStatus, @@ -397,7 +398,7 @@ pub enum CancelBehavior { pub struct Parser { /// 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>>, + line_counter: Rc>>, /// The jobs associated with this parser. job_list: RefCell, @@ -438,7 +439,7 @@ impl Parser { /// Create a parser. pub fn new(variables: Rc, cancel_behavior: CancelBehavior) -> Parser { let result = Self { - line_counter: Rc::new(RefCell::new(LineCounter::empty())), + line_counter: Rc::new(ScopedRefCell::new(LineCounter::empty())), job_list: RefCell::default(), wait_handles: RefCell::new(WaitHandleStore::new()), block_list: RefCell::default(), @@ -624,8 +625,8 @@ pub fn eval_node( // 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()); + let restore_line_counter = + line_counter.scoped_replace(ps.line_counter::()); // Create a new execution context. let mut execution_context = @@ -640,7 +641,7 @@ pub fn eval_node( let new_exec_count = self.libdata().exec_count; let new_status_count = self.libdata().status_count; - ScopeGuarding::commit(scoped_line_counter); + ScopeGuarding::commit(restore_line_counter); self.pop_block(scope_block); job_reap(self, false); // reap again diff --git a/src/tests/parser.rs b/src/tests/parser.rs index d34faa9d6..9f4f826fa 100644 --- a/src/tests/parser.rs +++ b/src/tests/parser.rs @@ -786,25 +786,14 @@ fn test_line_counter() { assert_eq!(line_offset, expected); } - fn ref_eq(a: Option<&T>, b: Option<&T>) -> bool { - match (a, b) { - (Some(a), Some(b)) => std::ptr::eq(a, b), - (None, None) => true, - _ => false, - } - } - let pipelines: Vec<_> = ps.ast.walk().filter_map(|n| n.as_job_pipeline()).collect(); assert_eq!(pipelines.len(), 3); let src_offsets = [0, 0, 2]; assert_eq!(line_counter.source_offset_of_node(), None); assert_eq!(line_counter.line_offset_of_node(), None); - let mut last_set = None; for (idx, &node) in pipelines.iter().enumerate() { - let orig = line_counter.set_node(Some(node)); - assert!(ref_eq(orig, last_set)); - last_set = Some(node); + line_counter.node = node as *const _; assert_eq!( line_counter.source_offset_of_node(), Some(node.source_range().start()) @@ -813,9 +802,7 @@ fn ref_eq(a: Option<&T>, b: Option<&T>) -> bool { } for (idx, &node) in pipelines.iter().enumerate().rev() { - let orig = line_counter.set_node(Some(node)); - assert!(ref_eq(orig, last_set)); - last_set = Some(node); + line_counter.node = node as *const _; assert_eq!( line_counter.source_offset_of_node(), Some(node.source_range().start())