From bcc3d9e6e44c27d015f7a8b266999de5bf493959 Mon Sep 17 00:00:00 2001 From: Julio Merino Date: Fri, 15 Nov 2024 17:37:39 -0800 Subject: [PATCH 1/9] Pull Context out of exec_with_data This is the first step in decoupling Context from exec to allow the calling user to control the execution loop. --- core/src/exec.rs | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/core/src/exec.rs b/core/src/exec.rs index d3072780..64910434 100644 --- a/core/src/exec.rs +++ b/core/src/exec.rs @@ -1527,10 +1527,13 @@ impl Machine { /// Executes the instructions given in `instr`. /// /// This is a helper to `exec`, which prepares the machine with the program's data upfront. - async fn exec_with_data(&mut self, instrs: &[Instruction]) -> Result { - let mut context = Context::default(); + async fn exec_with_data( + &mut self, + context: &mut Context, + instrs: &[Instruction], + ) -> Result { while context.pc < instrs.len() { - match self.exec_until_stop(&mut context, instrs) { + match self.exec_until_stop(context, instrs) { Ok(InternalStopReason::CheckStop) => { if self.should_stop().await { return Ok(StopReason::Break); @@ -1541,21 +1544,14 @@ impl Machine { let result; if let Some(return_type) = data.return_type { result = self - .function_call( - &mut context, - &data.name, - return_type, - data.pos, - data.nargs, - ) + .function_call(context, &data.name, return_type, data.pos, data.nargs) .await; } else { - result = - self.builtin_call(&mut context, &data.name, data.pos, data.nargs).await; + result = self.builtin_call(context, &data.name, data.pos, data.nargs).await; } match result { Ok(()) => context.pc += 1, - Err(e) => self.handle_error(instrs, &mut context, e)?, + Err(e) => self.handle_error(instrs, context, e)?, } } @@ -1567,7 +1563,7 @@ impl Machine { return Ok(StopReason::Exited(code)); } - Err(e) => self.handle_error(instrs, &mut context, e)?, + Err(e) => self.handle_error(instrs, context, e)?, } } Ok(StopReason::Eof) @@ -1580,9 +1576,11 @@ impl Machine { pub async fn exec(&mut self, input: &mut dyn io::Read) -> Result { let image = compiler::compile(input, &self.symbols)?; + let mut context = Context::default(); + assert!(self.data.is_empty()); self.data = image.data; - let result = self.exec_with_data(&image.instrs).await; + let result = self.exec_with_data(&mut context, &image.instrs).await; self.data.clear(); result } From c880c201a92c7a3bc705577f9612caa4a1efe528 Mon Sep 17 00:00:00 2001 From: Julio Merino Date: Fri, 15 Nov 2024 17:55:21 -0800 Subject: [PATCH 2/9] Move the Image into the Context The execution Context is what should track the Image (program and memory of the machine during execution), so move it there instead of keeping it at the Machine level. --- core/src/exec.rs | 158 +++++++++++++++++------------------------- core/src/testutils.rs | 4 +- std/src/data.rs | 2 +- 3 files changed, 68 insertions(+), 96 deletions(-) diff --git a/core/src/exec.rs b/core/src/exec.rs index 64910434..4b79b96b 100644 --- a/core/src/exec.rs +++ b/core/src/exec.rs @@ -352,9 +352,10 @@ impl Stack { } } -/// Provides controlled access to the parameters passed to a callable. +/// Provides controlled access to the environment passed to a callable. pub struct Scope<'s> { stack: &'s mut Stack, + data: &'s [Option], nargs: usize, fref_pos: LineCol, } @@ -366,10 +367,21 @@ impl Drop for Scope<'_> { } impl<'s> Scope<'s> { - /// Creates a new scope that wraps `nargs` arguments at the top of `stack`. - fn new(stack: &'s mut Stack, nargs: usize, fref_pos: LineCol) -> Self { + /// Creates a new scope that wraps `nargs` arguments at the top of `stack` and that offers + /// access to `data`. + fn new( + stack: &'s mut Stack, + data: &'s [Option], + nargs: usize, + fref_pos: LineCol, + ) -> Self { debug_assert!(nargs <= stack.len()); - Self { stack, nargs, fref_pos } + Self { stack, data, nargs, fref_pos } + } + + /// Obtains immutable access to the data values available during the *current* execution. + pub fn data(&self) -> &[Option] { + self.data } /// Removes all remaining arguments from the stack tracked by this scope. @@ -541,15 +553,19 @@ impl<'s> Scope<'s> { /// Machine state for the execution of an individual chunk of code. struct Context { + instrs: Vec, + data: Vec>, pc: Address, addr_stack: Vec
, value_stack: Stack, err_handler: ErrorHandlerISpan, } -impl Default for Context { - fn default() -> Self { +impl From for Context { + fn from(image: Image) -> Self { Self { + instrs: image.instrs, + data: image.data, pc: 0, addr_stack: vec![], value_stack: Stack::default(), @@ -565,7 +581,6 @@ pub struct Machine { yield_now_fn: Option, signals_chan: (Sender, Receiver), last_error: Option, - data: Vec>, } impl Default for Machine { @@ -592,7 +607,6 @@ impl Machine { yield_now_fn, signals_chan: signals, last_error: None, - data: vec![], } } @@ -630,11 +644,6 @@ impl Machine { self.last_error.as_deref() } - /// Obtains immutable access to the data values available during the *current* execution. - pub fn get_data(&self) -> &[Option] { - &self.data - } - /// Obtains immutable access to the state of the symbols. pub fn get_symbols(&self) -> &Symbols { &self.symbols @@ -661,18 +670,18 @@ impl Machine { /// Handles an array assignment. fn assign_array( &mut self, - context: &mut Context, + value_stack: &mut Stack, key: &SymbolKey, vref_pos: LineCol, nargs: usize, ) -> Result<()> { let mut ds = Vec::with_capacity(nargs); for _ in 0..nargs { - let i = context.value_stack.pop_integer(); + let i = value_stack.pop_integer(); ds.push(i); } - let (value, _pos) = context.value_stack.pop().unwrap(); + let (value, _pos) = value_stack.pop().unwrap(); match self.symbols.load_mut(key) { Some(Symbol::Array(array)) => { @@ -699,7 +708,7 @@ impl Machine { let metadata = b.metadata(); debug_assert!(!metadata.is_function()); - let scope = Scope::new(&mut context.value_stack, nargs, bref_pos); + let scope = Scope::new(&mut context.value_stack, &context.data, nargs, bref_pos); let b = b.clone(); b.exec(scope, self).await @@ -707,10 +716,10 @@ impl Machine { /// Handles an array definition. The array must not yet exist, and the name may not overlap /// function or variable names. - fn dim_array(&mut self, context: &mut Context, span: &DimArrayISpan) -> Result<()> { + fn dim_array(&mut self, value_stack: &mut Stack, span: &DimArrayISpan) -> Result<()> { let mut ds = Vec::with_capacity(span.dimensions); for _ in 0..span.dimensions { - let (i, pos) = context.value_stack.pop_integer_with_pos(); + let (i, pos) = value_stack.pop_integer_with_pos(); if i <= 0 { return new_syntax_error(pos, "Dimensions in DIM array must be positive"); } @@ -891,10 +900,10 @@ impl Machine { } /// Evaluates the subscripts of an array reference. - fn get_array_args(&self, context: &mut Context, nargs: usize) -> Result> { + fn get_array_args(&self, value_stack: &mut Stack, nargs: usize) -> Result> { let mut subscripts = Vec::with_capacity(nargs); for _ in 0..nargs { - let i = context.value_stack.pop_integer(); + let i = value_stack.pop_integer(); subscripts.push(i); } Ok(subscripts) @@ -912,7 +921,7 @@ impl Machine { let metadata = f.metadata(); debug_assert_eq!(return_type, metadata.return_type().unwrap()); - let scope = Scope::new(&mut context.value_stack, nargs, fref_pos); + let scope = Scope::new(&mut context.value_stack, &context.data, nargs, fref_pos); f.exec(scope, self).await?; if cfg!(debug_assertions) { match context.value_stack.top() { @@ -932,19 +941,19 @@ impl Machine { /// Handles an array reference. fn array_ref( &mut self, - context: &mut Context, + value_stack: &mut Stack, key: &SymbolKey, vref_pos: LineCol, nargs: usize, ) -> Result<()> { - let subscripts = self.get_array_args(context, nargs)?; + let subscripts = self.get_array_args(value_stack, nargs)?; match self.symbols.load(key) { Some(Symbol::Array(array)) => { let value = array .index(&subscripts) .cloned() .map_err(|e| Error::from_value_error(e, vref_pos))?; - context.value_stack.push((value, vref_pos)); + value_stack.push((value, vref_pos)); Ok(()) } Some(_) => unreachable!("Array type checking has been done at compile time"), @@ -989,7 +998,7 @@ impl Machine { fpos: LineCol, f: Rc, ) -> Result<()> { - let scope = Scope::new(&mut context.value_stack, 0, fpos); + let scope = Scope::new(&mut context.value_stack, &context.data, 0, fpos); f.exec(scope, self).await?; if cfg!(debug_assertions) { match context.value_stack.top() { @@ -1017,13 +1026,9 @@ impl Machine { /// Executes as many instructions as possible from `instrs`, starting at `context.pc`, until an /// instruction asks to stop or execution reaches the end of the program. - fn exec_until_stop( - &mut self, - context: &mut Context, - instrs: &[Instruction], - ) -> Result { - while context.pc < instrs.len() { - let instr = &instrs[context.pc]; + fn exec_until_stop(&mut self, context: &mut Context) -> Result { + while context.pc < context.instrs.len() { + let instr = &context.instrs[context.pc]; match instr { Instruction::LogicalAnd(pos) => { Machine::exec_logical_op2(context, |lhs, rhs| lhs && rhs, *pos); @@ -1261,12 +1266,12 @@ impl Machine { } Instruction::ArrayAssignment(name, vref_pos, nargs) => { - self.assign_array(context, name, *vref_pos, *nargs)?; + self.assign_array(&mut context.value_stack, name, *vref_pos, *nargs)?; context.pc += 1; } Instruction::ArrayLoad(name, pos, nargs) => { - self.array_ref(context, name, *pos, *nargs)?; + self.array_ref(&mut context.value_stack, name, *pos, *nargs)?; context.pc += 1; } @@ -1311,7 +1316,7 @@ impl Machine { } Instruction::DimArray(span) => { - self.dim_array(context, span)?; + self.dim_array(&mut context.value_stack, span)?; context.pc += 1; } @@ -1487,12 +1492,7 @@ impl Machine { /// Handles the given error `e` according to the current error handler previously set by /// `ON ERROR`. If the error can be handled gracefully, returns `Ok`; otherwise, returns the /// input error unmodified. - fn handle_error( - &mut self, - instrs: &[Instruction], - context: &mut Context, - e: Error, - ) -> Result<()> { + fn handle_error(&mut self, context: &mut Context, e: Error) -> Result<()> { if !e.is_catchable() { return Err(e); } @@ -1506,14 +1506,14 @@ impl Machine { } ErrorHandlerISpan::None => Err(e), ErrorHandlerISpan::ResumeNext => { - if instrs[context.pc].is_statement() { + if context.instrs[context.pc].is_statement() { context.pc += 1; } else { loop { context.pc += 1; - if context.pc >= instrs.len() { + if context.pc >= context.instrs.len() { break; - } else if instrs[context.pc].is_statement() { + } else if context.instrs[context.pc].is_statement() { context.pc += 1; break; } @@ -1524,16 +1524,12 @@ impl Machine { } } - /// Executes the instructions given in `instr`. + /// Executes the instructions in `context`. /// - /// This is a helper to `exec`, which prepares the machine with the program's data upfront. - async fn exec_with_data( - &mut self, - context: &mut Context, - instrs: &[Instruction], - ) -> Result { - while context.pc < instrs.len() { - match self.exec_until_stop(context, instrs) { + /// This is a helper to `exec` which prepares the `context` from a script. + async fn exec_with_context(&mut self, context: &mut Context) -> Result { + while context.pc < context.instrs.len() { + match self.exec_until_stop(context) { Ok(InternalStopReason::CheckStop) => { if self.should_stop().await { return Ok(StopReason::Break); @@ -1551,7 +1547,7 @@ impl Machine { } match result { Ok(()) => context.pc += 1, - Err(e) => self.handle_error(instrs, context, e)?, + Err(e) => self.handle_error(context, e)?, } } @@ -1563,7 +1559,7 @@ impl Machine { return Ok(StopReason::Exited(code)); } - Err(e) => self.handle_error(instrs, context, e)?, + Err(e) => self.handle_error(context, e)?, } } Ok(StopReason::Eof) @@ -1575,14 +1571,8 @@ impl Machine { /// different programs on the same machine, all sharing state. pub async fn exec(&mut self, input: &mut dyn io::Read) -> Result { let image = compiler::compile(input, &self.symbols)?; - - let mut context = Context::default(); - - assert!(self.data.is_empty()); - self.data = image.data; - let result = self.exec_with_data(&mut context, &image.instrs).await; - self.data.clear(); - result + let mut context = Context::from(image); + self.exec_with_context(&mut context).await } } @@ -1683,7 +1673,7 @@ mod tests { #[test] fn test_scope_and_stack_empty() { let mut stack = Stack::from([]); - let scope = Scope::new(&mut stack, 0, LineCol { line: 50, col: 60 }); + let scope = Scope::new(&mut stack, &[], 0, LineCol { line: 50, col: 60 }); drop(scope); assert_eq!(0, stack.len()); } @@ -1691,7 +1681,7 @@ mod tests { #[test] fn test_scope_no_args() { let mut stack = Stack::from([(Value::Integer(3), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, 0, LineCol { line: 50, col: 60 }); + let scope = Scope::new(&mut stack, &[], 0, LineCol { line: 50, col: 60 }); drop(scope); assert_eq!(1, stack.len()); } @@ -1704,7 +1694,7 @@ mod tests { (Value::Integer(2), LineCol { line: 1, col: 2 }), (Value::Integer(4), LineCol { line: 1, col: 2 }), ]); - let mut scope = Scope::new(&mut stack, 3, LineCol { line: 50, col: 60 }); + let mut scope = Scope::new(&mut stack, &[], 3, LineCol { line: 50, col: 60 }); assert_eq!(3, scope.nargs()); assert_eq!(4, scope.pop_integer()); assert_eq!(2, scope.nargs()); @@ -1724,7 +1714,7 @@ mod tests { (Value::Text("foo".to_owned()), LineCol { line: 1, col: 2 }), (Value::VarRef(SymbolKey::from("foo"), ExprType::Integer), LineCol { line: 1, col: 2 }), ]); - let mut scope = Scope::new(&mut stack, 5, LineCol { line: 50, col: 60 }); + let mut scope = Scope::new(&mut stack, &[], 5, LineCol { line: 50, col: 60 }); assert_eq!((SymbolKey::from("foo"), ExprType::Integer), scope.pop_varref()); assert_eq!("foo", scope.pop_string()); assert_eq!(2, scope.pop_integer()); @@ -1744,7 +1734,7 @@ mod tests { LineCol { line: 9, col: 10 }, ), ]); - let mut scope = Scope::new(&mut stack, 5, LineCol { line: 50, col: 60 }); + let mut scope = Scope::new(&mut stack, &[], 5, LineCol { line: 50, col: 60 }); assert_eq!( (SymbolKey::from("foo"), ExprType::Integer, LineCol { line: 9, col: 10 }), scope.pop_varref_with_pos() @@ -1758,7 +1748,7 @@ mod tests { #[test] fn test_scope_return_any() { let mut stack = Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, 0, LineCol { line: 50, col: 60 }); + let scope = Scope::new(&mut stack, &[], 0, LineCol { line: 50, col: 60 }); assert!(scope.return_any(Value::Boolean(true)).is_ok()); assert_eq!((true, LineCol { line: 50, col: 60 }), stack.pop_boolean_with_pos()); assert_eq!((false, LineCol { line: 1, col: 2 }), stack.pop_boolean_with_pos()); @@ -1767,7 +1757,7 @@ mod tests { #[test] fn test_scope_return_boolean() { let mut stack = Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, 0, LineCol { line: 50, col: 60 }); + let scope = Scope::new(&mut stack, &[], 0, LineCol { line: 50, col: 60 }); assert!(scope.return_boolean(true).is_ok()); assert_eq!((true, LineCol { line: 50, col: 60 }), stack.pop_boolean_with_pos()); assert_eq!((false, LineCol { line: 1, col: 2 }), stack.pop_boolean_with_pos()); @@ -1776,7 +1766,7 @@ mod tests { #[test] fn test_scope_return_double() { let mut stack = Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, 0, LineCol { line: 50, col: 60 }); + let scope = Scope::new(&mut stack, &[], 0, LineCol { line: 50, col: 60 }); assert!(scope.return_double(4.5).is_ok()); assert_eq!((4.5, LineCol { line: 50, col: 60 }), stack.pop_double_with_pos()); assert_eq!((false, LineCol { line: 1, col: 2 }), stack.pop_boolean_with_pos()); @@ -1785,7 +1775,7 @@ mod tests { #[test] fn test_scope_return_integer() { let mut stack = Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, 0, LineCol { line: 50, col: 60 }); + let scope = Scope::new(&mut stack, &[], 0, LineCol { line: 50, col: 60 }); assert!(scope.return_integer(7).is_ok()); assert_eq!((7, LineCol { line: 50, col: 60 }), stack.pop_integer_with_pos()); assert_eq!((false, LineCol { line: 1, col: 2 }), stack.pop_boolean_with_pos()); @@ -1794,7 +1784,7 @@ mod tests { #[test] fn test_scope_return_string() { let mut stack = Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, 0, LineCol { line: 50, col: 60 }); + let scope = Scope::new(&mut stack, &[], 0, LineCol { line: 50, col: 60 }); assert!(scope.return_string("foo").is_ok()); assert_eq!(("foo".to_owned(), LineCol { line: 50, col: 60 }), stack.pop_string_with_pos()); assert_eq!((false, LineCol { line: 1, col: 2 }), stack.pop_boolean_with_pos()); @@ -1828,18 +1818,15 @@ mod tests { } #[test] - fn test_get_data() { + fn test_data_access() { let captured_data = Rc::from(RefCell::from(vec![])); let mut machine = Machine::default(); machine.add_callable(GetDataCommand::new(captured_data.clone())); - assert!(machine.get_data().is_empty()); - assert_eq!( StopReason::Eof, block_on(machine.exec(&mut b"DATA 3: GETDATA".as_ref())).unwrap() ); - assert!(machine.get_data().is_empty()); assert_eq!(&[Some(Value::Integer(3))], captured_data.borrow().as_slice()); assert_eq!( @@ -1857,7 +1844,6 @@ mod tests { ) .unwrap() ); - assert!(machine.get_data().is_empty()); assert_eq!( &[ Some(Value::Integer(5)), @@ -1869,20 +1855,6 @@ mod tests { ); } - #[test] - fn test_get_data_is_empty_after_execution() { - let mut machine = Machine::default(); - - assert_eq!(StopReason::Eof, block_on(machine.exec(&mut b"DATA 3".as_ref())).unwrap()); - assert!(machine.get_data().is_empty()); - - block_on(machine.exec(&mut b"DATA 3: abc".as_ref())).unwrap_err(); - assert!(machine.get_data().is_empty()); - - block_on(machine.exec(&mut b"DATA 3: GOTO @foo".as_ref())).unwrap_err(); - assert!(machine.get_data().is_empty()); - } - /// Runs the `input` code on a new test machine. /// /// `golden_in` is the sequence of values to yield by `IN`. diff --git a/core/src/testutils.rs b/core/src/testutils.rs index ab64292c..34c128bc 100644 --- a/core/src/testutils.rs +++ b/core/src/testutils.rs @@ -265,9 +265,9 @@ impl Callable for GetDataCommand { &self.metadata } - async fn exec(&self, scope: Scope<'_>, machine: &mut Machine) -> Result<()> { + async fn exec(&self, scope: Scope<'_>, _machine: &mut Machine) -> Result<()> { assert_eq!(0, scope.nargs()); - *self.data.borrow_mut() = machine.get_data().to_vec(); + *self.data.borrow_mut() = scope.data().to_vec(); Ok(()) } } diff --git a/std/src/data.rs b/std/src/data.rs index 556fc0aa..983ca21f 100644 --- a/std/src/data.rs +++ b/std/src/data.rs @@ -94,7 +94,7 @@ impl Callable for ReadCommand { let mut index = self.index.borrow_mut(); for (vname, vtype, pos) in vrefs { let datum = { - let data = machine.get_data(); + let data = scope.data(); debug_assert!(*index <= data.len()); if *index == data.len() { return Err(Error::InternalError( From c08aaa37444ea4fa82f90ab91abffa2f36601eac Mon Sep 17 00:00:00 2001 From: Julio Merino Date: Fri, 15 Nov 2024 18:18:50 -0800 Subject: [PATCH 3/9] Separate compilation from execution Instead of compiling code within exec(), make the caller do the compilation first to prepare a Context, and then as the Machine to execute it. This removes a "wart" that has existed for a long time because compilation should always have been kept separate. --- cli/src/main.rs | 6 ++-- core/examples/config.rs | 3 +- core/examples/dsl.rs | 3 +- core/src/exec.rs | 57 ++++++++++++++++++++++------------- repl/src/lib.rs | 35 +++++++++++++++------ std/examples/script-runner.rs | 10 +++++- std/src/program.rs | 3 +- std/src/testutils.rs | 11 +++++-- 8 files changed, 90 insertions(+), 38 deletions(-) diff --git a/cli/src/main.rs b/cli/src/main.rs index 90a1b184..62a7f546 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -278,7 +278,8 @@ async fn run_repl_loop( async fn run_script>(path: P, console_spec: Option<&str>) -> Result { let mut machine = new_machine_builder(console_spec)?.build()?; let mut input = File::open(path)?; - Ok(machine.exec(&mut input).await?.as_exit_code()) + let mut context = machine.compile(&mut input)?; + Ok(machine.exec(&mut context).await?.as_exit_code()) } /// Executes the `path` program in a fresh machine allowing any interactive-only calls. @@ -320,7 +321,8 @@ async fn run_interactive( } None => { let mut input = File::open(path)?; - Ok(machine.exec(&mut input).await?.as_exit_code()) + let mut context = machine.compile(&mut input)?; + Ok(machine.exec(&mut context).await?.as_exit_code()) } } } diff --git a/core/examples/config.rs b/core/examples/config.rs index 341baa4a..52573c9f 100644 --- a/core/examples/config.rs +++ b/core/examples/config.rs @@ -40,8 +40,9 @@ fn main() { // Execute the sample script. All this script can do is modify the state of the machine itself. // In other words: the script can set variables in the machine's environment, but that's it. + let mut context = machine.compile(&mut INPUT.as_bytes()).expect("Compilation error"); loop { - match block_on(machine.exec(&mut INPUT.as_bytes())).expect("Execution error") { + match block_on(machine.exec(&mut context)).expect("Execution error") { StopReason::Eof => break, StopReason::Exited(i) => println!("Script explicitly exited with code {}", i), StopReason::Break => (), // Ignore signals. diff --git a/core/examples/dsl.rs b/core/examples/dsl.rs index 85a99cb4..213b6340 100644 --- a/core/examples/dsl.rs +++ b/core/examples/dsl.rs @@ -150,8 +150,9 @@ fn main() { // Execute the sample script, which will call back into our callable objects in Rust land to // manipulate the state of the lights. println!("Running script"); + let mut context = machine.compile(&mut INPUT.as_bytes()).expect("Compilation error"); loop { - match block_on(machine.exec(&mut INPUT.as_bytes())).expect("Execution error") { + match block_on(machine.exec(&mut context)).expect("Execution error") { StopReason::Eof => break, StopReason::Exited(i) => println!("Script explicitly exited with code {}", i), StopReason::Break => (), // Ignore signals. diff --git a/core/src/exec.rs b/core/src/exec.rs index 4b79b96b..db2fcfb0 100644 --- a/core/src/exec.rs +++ b/core/src/exec.rs @@ -552,7 +552,7 @@ impl<'s> Scope<'s> { } /// Machine state for the execution of an individual chunk of code. -struct Context { +pub struct Context { instrs: Vec, data: Vec>, pc: Address, @@ -1524,10 +1524,18 @@ impl Machine { } } - /// Executes the instructions in `context`. + /// Compiles a program extracted from `input` so that it can run on the configured machine and + /// returns an execution context. /// - /// This is a helper to `exec` which prepares the `context` from a script. - async fn exec_with_context(&mut self, context: &mut Context) -> Result { + /// The compiled program is bound to this specific machine configuration as it depends on the + /// symbols that are available in the machine at the time of compilation. + pub fn compile(&self, input: &mut dyn io::Read) -> Result { + let image = compiler::compile(input, &self.symbols)?; + Ok(Context::from(image)) + } + + /// Executes the instructions in `context`. + pub async fn exec(&mut self, context: &mut Context) -> Result { while context.pc < context.instrs.len() { match self.exec_until_stop(context) { Ok(InternalStopReason::CheckStop) => { @@ -1569,10 +1577,10 @@ impl Machine { /// /// Note that this does not consume `self`. As a result, it is possible to execute multiple /// different programs on the same machine, all sharing state. - pub async fn exec(&mut self, input: &mut dyn io::Read) -> Result { - let image = compiler::compile(input, &self.symbols)?; - let mut context = Context::from(image); - self.exec_with_context(&mut context).await + #[cfg(test)] + async fn compile_and_exec(&mut self, input: &mut dyn io::Read) -> Result { + let mut context = self.compile(input)?; + self.exec(&mut context).await } } @@ -1800,7 +1808,8 @@ mod tests { assert_eq!( StopReason::Eof, - block_on(machine.exec(&mut b"a = TRUE: b = 1".as_ref())).expect("Execution failed") + block_on(machine.compile_and_exec(&mut b"a = TRUE: b = 1".as_ref())) + .expect("Execution failed") ); match machine.get_symbols().get_auto("a") { Some(Symbol::Variable(Value::Boolean(_))) => (), @@ -1825,14 +1834,14 @@ mod tests { assert_eq!( StopReason::Eof, - block_on(machine.exec(&mut b"DATA 3: GETDATA".as_ref())).unwrap() + block_on(machine.compile_and_exec(&mut b"DATA 3: GETDATA".as_ref())).unwrap() ); assert_eq!(&[Some(Value::Integer(3))], captured_data.borrow().as_slice()); assert_eq!( StopReason::Eof, block_on( - machine.exec( + machine.compile_and_exec( &mut b" GETDATA IF FALSE THEN: DATA 5: ELSE: DATA 6: END IF @@ -1876,7 +1885,7 @@ mod tests { machine.add_callable(RaisefFunction::new()); machine.add_callable(SumFunction::new()); machine.add_callable(TypeCheckFunction::new(Value::Integer(5))); - block_on(machine.exec(&mut input.as_bytes())) + block_on(machine.compile_and_exec(&mut input.as_bytes())) } /// Runs the `input` code on a new test machine and verifies its output. @@ -2156,7 +2165,7 @@ mod tests { assert_eq!( StopReason::Exited(10), - block_on(machine.exec(&mut "OUT 1\nEND 10\nOUT 2".as_bytes())) + block_on(machine.compile_and_exec(&mut "OUT 1\nEND 10\nOUT 2".as_bytes())) .expect("Execution failed") ); assert_eq!(&["1"], captured_out.borrow().as_slice()); @@ -2164,7 +2173,7 @@ mod tests { captured_out.borrow_mut().clear(); assert_eq!( StopReason::Exited(11), - block_on(machine.exec(&mut "OUT 2\nEND 11\nOUT 3".as_bytes())) + block_on(machine.compile_and_exec(&mut "OUT 2\nEND 11\nOUT 3".as_bytes())) .expect("Execution failed") ); assert_eq!(&["2"], captured_out.borrow().as_slice()); @@ -2180,7 +2189,7 @@ mod tests { for _ in 0..10 { let input = &mut "WHILE TRUE: WEND".as_bytes(); machine.drain_signals(); - let future = machine.exec(input); + let future = machine.compile_and_exec(input); // There is no guarantee that the tight loop inside the machine is running immediately // at this point (particularly because we run with just one thread in this test), thus @@ -2202,7 +2211,7 @@ mod tests { tx.send(Signal::Break).await.unwrap(); let input = &mut code.as_bytes(); - assert_eq!(StopReason::Eof, machine.exec(input).await.unwrap()); + assert_eq!(StopReason::Eof, machine.compile_and_exec(input).await.unwrap()); assert_eq!(1, tx.len()); } @@ -2229,7 +2238,7 @@ mod tests { tx.send(Signal::Break).await.unwrap(); let input = &mut code.as_bytes(); - assert_eq!(StopReason::Break, machine.exec(input).await.unwrap()); + assert_eq!(StopReason::Break, machine.compile_and_exec(input).await.unwrap()); assert_eq!(0, tx.len()); } @@ -3066,7 +3075,10 @@ mod tests { "#; let mut machine = Machine::default(); - assert_eq!(StopReason::Eof, block_on(machine.exec(&mut code.as_bytes())).unwrap()); + assert_eq!( + StopReason::Eof, + block_on(machine.compile_and_exec(&mut code.as_bytes())).unwrap() + ); assert_eq!(1, machine.get_symbols().locals().len()); match machine.get_symbols().get_auto("I") { Some(Symbol::Variable(Value::Integer(i))) => assert_eq!(4, *i), @@ -3084,7 +3096,10 @@ mod tests { let mut machine = Machine::default(); machine.add_callable(ClearCommand::new()); - assert_eq!(StopReason::Eof, block_on(machine.exec(&mut code.as_bytes())).unwrap()); + assert_eq!( + StopReason::Eof, + block_on(machine.compile_and_exec(&mut code.as_bytes())).unwrap() + ); } #[test] @@ -3242,11 +3257,11 @@ mod tests { let mut machine = Machine::default(); assert_eq!( StopReason::Eof, - block_on(machine.exec(&mut b"a = 10".as_ref())).expect("Execution failed") + block_on(machine.compile_and_exec(&mut b"a = 10".as_ref())).expect("Execution failed") ); assert_eq!( StopReason::Eof, - block_on(machine.exec(&mut b"b = a".as_ref())).expect("Execution failed") + block_on(machine.compile_and_exec(&mut b"b = a".as_ref())).expect("Execution failed") ); } diff --git a/repl/src/lib.rs b/repl/src/lib.rs index 0fa257f3..b56fe2cf 100644 --- a/repl/src/lib.rs +++ b/repl/src/lib.rs @@ -71,7 +71,14 @@ pub async fn try_load_autoexec( }; console.borrow_mut().print("Loading AUTOEXEC.BAS...")?; - match machine.exec(&mut code.as_bytes()).await { + let mut context = match machine.compile(&mut code.as_bytes()) { + Ok(context) => context, + Err(e) => { + console.borrow_mut().print(&format!("AUTOEXEC.BAS failed: {}", e))?; + return Ok(()); + } + }; + match machine.exec(&mut context).await { Ok(_) => Ok(()), Err(e) => { console.borrow_mut().print(&format!("AUTOEXEC.BAS failed: {}", e))?; @@ -113,7 +120,9 @@ pub async fn run_from_cloud( console.borrow_mut().print("Starting...")?; console.borrow_mut().print("")?; - let result = machine.exec(&mut "RUN".as_bytes()).await; + let mut context = + machine.compile(&mut "RUN".as_bytes()).expect("Compilation of hardcoded code must succeed"); + let result = machine.exec(&mut context).await; let mut console = console.borrow_mut(); @@ -182,13 +191,21 @@ pub async fn run_repl_loop( machine.drain_signals(); match line { - Ok(line) => match machine.exec(&mut line.as_bytes()).await { - Ok(reason) => stop_reason = reason, - Err(e) => { - let mut console = console.borrow_mut(); - console.print(format!("ERROR: {}", e).as_str())?; - } - }, + Ok(line) => { + match machine.compile(&mut line.as_bytes()) { + Ok(mut context) => match machine.exec(&mut context).await { + Ok(reason) => stop_reason = reason, + Err(e) => { + let mut console = console.borrow_mut(); + console.print(format!("ERROR: {}", e).as_str())?; + } + }, + Err(e) => { + let mut console = console.borrow_mut(); + console.print(format!("ERROR: {}", e).as_str())?; + } + }; + } Err(e) => { if e.kind() == io::ErrorKind::Interrupted { let mut console = console.borrow_mut(); diff --git a/std/examples/script-runner.rs b/std/examples/script-runner.rs index d265b091..1845ea93 100644 --- a/std/examples/script-runner.rs +++ b/std/examples/script-runner.rs @@ -43,7 +43,15 @@ fn safe_main() -> i32 { } }; - match block_on(machine.exec(&mut input)) { + let mut context = match machine.compile(&mut input) { + Ok(context) => context, + Err(e) => { + eprintln!("ERROR: {}", e); + process::exit(1); + } + }; + + match block_on(machine.exec(&mut context)) { Ok(stop_reason) => stop_reason.as_exit_code(), Err(e) => { eprintln!("ERROR: {}", e); diff --git a/std/src/program.rs b/std/src/program.rs index dca447b4..aa031a68 100644 --- a/std/src/program.rs +++ b/std/src/program.rs @@ -518,7 +518,8 @@ impl Callable for RunCommand { machine.clear(); let program = self.program.borrow().text(); - let stop_reason = machine.exec(&mut program.as_bytes()).await?; + let mut context = machine.compile(&mut program.as_bytes())?; + let stop_reason = machine.exec(&mut context).await?; match stop_reason { StopReason::Break => { self.console.borrow_mut().print(BREAK_MSG).map_err(|e| scope.io_error(e))? diff --git a/std/src/testutils.rs b/std/src/testutils.rs index 23a96f40..4a6383e4 100644 --- a/std/src/testutils.rs +++ b/std/src/testutils.rs @@ -508,7 +508,11 @@ impl Tester { /// Runs `script` in the configured machine and returns a `Checker` object to validate /// expectations about the execution. pub fn run>(&mut self, script: S) -> Checker { - let result = block_on(self.machine.exec(&mut script.into().as_bytes())); + let mut context = match self.machine.compile(&mut script.into().as_bytes()) { + Ok(context) => context, + Err(e) => return Checker::new(self, Err(e)), + }; + let result = block_on(self.machine.exec(&mut context)); Checker::new(self, result) } @@ -523,7 +527,10 @@ impl Tester { pub fn run_n(&mut self, scripts: &[&str]) -> Checker { let mut result = Ok(StopReason::Eof); for script in scripts { - result = block_on(self.machine.exec(&mut script.as_bytes())); + result = match self.machine.compile(&mut script.as_bytes()) { + Ok(mut context) => block_on(self.machine.exec(&mut context)), + Err(e) => Err(e), + }; if result.is_err() { break; } From 267cf5af6221e0d3368ff658b55afdd95cb01700 Mon Sep 17 00:00:00 2001 From: Julio Merino Date: Fri, 15 Nov 2024 18:49:49 -0800 Subject: [PATCH 4/9] Move last_error into the Context/Scope The last_error is dependent on the program being run by the Context, so move it there and bubble access through the Scope. --- core/src/exec.rs | 67 +++++++++++++++++++++++++++---------------- core/src/testutils.rs | 9 ++++-- std/src/exec.rs | 9 ++++-- 3 files changed, 55 insertions(+), 30 deletions(-) diff --git a/core/src/exec.rs b/core/src/exec.rs index db2fcfb0..a47b5f69 100644 --- a/core/src/exec.rs +++ b/core/src/exec.rs @@ -356,6 +356,7 @@ impl Stack { pub struct Scope<'s> { stack: &'s mut Stack, data: &'s [Option], + last_error: Option<&'s str>, nargs: usize, fref_pos: LineCol, } @@ -372,11 +373,12 @@ impl<'s> Scope<'s> { fn new( stack: &'s mut Stack, data: &'s [Option], + last_error: Option<&'s str>, nargs: usize, fref_pos: LineCol, ) -> Self { debug_assert!(nargs <= stack.len()); - Self { stack, data, nargs, fref_pos } + Self { stack, data, last_error, nargs, fref_pos } } /// Obtains immutable access to the data values available during the *current* execution. @@ -400,6 +402,11 @@ impl<'s> Scope<'s> { Error::InternalError(self.fref_pos, msg.into()) } + /// Returns the last execution error. + pub fn last_error(&self) -> Option<&str> { + self.last_error + } + /// Returns the number of arguments that can still be consumed. pub fn nargs(&self) -> usize { self.nargs @@ -555,6 +562,7 @@ impl<'s> Scope<'s> { pub struct Context { instrs: Vec, data: Vec>, + last_error: Option, pc: Address, addr_stack: Vec
, value_stack: Stack, @@ -566,6 +574,7 @@ impl From for Context { Self { instrs: image.instrs, data: image.data, + last_error: None, pc: 0, addr_stack: vec![], value_stack: Stack::default(), @@ -580,7 +589,6 @@ pub struct Machine { clearables: Vec>, yield_now_fn: Option, signals_chan: (Sender, Receiver), - last_error: Option, } impl Default for Machine { @@ -606,7 +614,6 @@ impl Machine { clearables: vec![], yield_now_fn, signals_chan: signals, - last_error: None, } } @@ -636,12 +643,6 @@ impl Machine { clearable.reset_state(&mut self.symbols); } self.symbols.clear(); - self.last_error = None; - } - - /// Returns the last execution error. - pub fn last_error(&self) -> Option<&str> { - self.last_error.as_deref() } /// Obtains immutable access to the state of the symbols. @@ -708,7 +709,13 @@ impl Machine { let metadata = b.metadata(); debug_assert!(!metadata.is_function()); - let scope = Scope::new(&mut context.value_stack, &context.data, nargs, bref_pos); + let scope = Scope::new( + &mut context.value_stack, + &context.data, + context.last_error.as_deref(), + nargs, + bref_pos, + ); let b = b.clone(); b.exec(scope, self).await @@ -921,7 +928,13 @@ impl Machine { let metadata = f.metadata(); debug_assert_eq!(return_type, metadata.return_type().unwrap()); - let scope = Scope::new(&mut context.value_stack, &context.data, nargs, fref_pos); + let scope = Scope::new( + &mut context.value_stack, + &context.data, + context.last_error.as_deref(), + nargs, + fref_pos, + ); f.exec(scope, self).await?; if cfg!(debug_assertions) { match context.value_stack.top() { @@ -998,7 +1011,13 @@ impl Machine { fpos: LineCol, f: Rc, ) -> Result<()> { - let scope = Scope::new(&mut context.value_stack, &context.data, 0, fpos); + let scope = Scope::new( + &mut context.value_stack, + &context.data, + context.last_error.as_deref(), + 0, + fpos, + ); f.exec(scope, self).await?; if cfg!(debug_assertions) { match context.value_stack.top() { @@ -1492,12 +1511,12 @@ impl Machine { /// Handles the given error `e` according to the current error handler previously set by /// `ON ERROR`. If the error can be handled gracefully, returns `Ok`; otherwise, returns the /// input error unmodified. - fn handle_error(&mut self, context: &mut Context, e: Error) -> Result<()> { + fn handle_error(&self, context: &mut Context, e: Error) -> Result<()> { if !e.is_catchable() { return Err(e); } - self.last_error = Some(format!("{}", e)); + context.last_error = Some(format!("{}", e)); match context.err_handler { ErrorHandlerISpan::Jump(addr) => { @@ -1681,7 +1700,7 @@ mod tests { #[test] fn test_scope_and_stack_empty() { let mut stack = Stack::from([]); - let scope = Scope::new(&mut stack, &[], 0, LineCol { line: 50, col: 60 }); + let scope = Scope::new(&mut stack, &[], None, 0, LineCol { line: 50, col: 60 }); drop(scope); assert_eq!(0, stack.len()); } @@ -1689,7 +1708,7 @@ mod tests { #[test] fn test_scope_no_args() { let mut stack = Stack::from([(Value::Integer(3), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, &[], 0, LineCol { line: 50, col: 60 }); + let scope = Scope::new(&mut stack, &[], None, 0, LineCol { line: 50, col: 60 }); drop(scope); assert_eq!(1, stack.len()); } @@ -1702,7 +1721,7 @@ mod tests { (Value::Integer(2), LineCol { line: 1, col: 2 }), (Value::Integer(4), LineCol { line: 1, col: 2 }), ]); - let mut scope = Scope::new(&mut stack, &[], 3, LineCol { line: 50, col: 60 }); + let mut scope = Scope::new(&mut stack, &[], None, 3, LineCol { line: 50, col: 60 }); assert_eq!(3, scope.nargs()); assert_eq!(4, scope.pop_integer()); assert_eq!(2, scope.nargs()); @@ -1722,7 +1741,7 @@ mod tests { (Value::Text("foo".to_owned()), LineCol { line: 1, col: 2 }), (Value::VarRef(SymbolKey::from("foo"), ExprType::Integer), LineCol { line: 1, col: 2 }), ]); - let mut scope = Scope::new(&mut stack, &[], 5, LineCol { line: 50, col: 60 }); + let mut scope = Scope::new(&mut stack, &[], None, 5, LineCol { line: 50, col: 60 }); assert_eq!((SymbolKey::from("foo"), ExprType::Integer), scope.pop_varref()); assert_eq!("foo", scope.pop_string()); assert_eq!(2, scope.pop_integer()); @@ -1742,7 +1761,7 @@ mod tests { LineCol { line: 9, col: 10 }, ), ]); - let mut scope = Scope::new(&mut stack, &[], 5, LineCol { line: 50, col: 60 }); + let mut scope = Scope::new(&mut stack, &[], None, 5, LineCol { line: 50, col: 60 }); assert_eq!( (SymbolKey::from("foo"), ExprType::Integer, LineCol { line: 9, col: 10 }), scope.pop_varref_with_pos() @@ -1756,7 +1775,7 @@ mod tests { #[test] fn test_scope_return_any() { let mut stack = Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, &[], 0, LineCol { line: 50, col: 60 }); + let scope = Scope::new(&mut stack, &[], None, 0, LineCol { line: 50, col: 60 }); assert!(scope.return_any(Value::Boolean(true)).is_ok()); assert_eq!((true, LineCol { line: 50, col: 60 }), stack.pop_boolean_with_pos()); assert_eq!((false, LineCol { line: 1, col: 2 }), stack.pop_boolean_with_pos()); @@ -1765,7 +1784,7 @@ mod tests { #[test] fn test_scope_return_boolean() { let mut stack = Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, &[], 0, LineCol { line: 50, col: 60 }); + let scope = Scope::new(&mut stack, &[], None, 0, LineCol { line: 50, col: 60 }); assert!(scope.return_boolean(true).is_ok()); assert_eq!((true, LineCol { line: 50, col: 60 }), stack.pop_boolean_with_pos()); assert_eq!((false, LineCol { line: 1, col: 2 }), stack.pop_boolean_with_pos()); @@ -1774,7 +1793,7 @@ mod tests { #[test] fn test_scope_return_double() { let mut stack = Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, &[], 0, LineCol { line: 50, col: 60 }); + let scope = Scope::new(&mut stack, &[], None, 0, LineCol { line: 50, col: 60 }); assert!(scope.return_double(4.5).is_ok()); assert_eq!((4.5, LineCol { line: 50, col: 60 }), stack.pop_double_with_pos()); assert_eq!((false, LineCol { line: 1, col: 2 }), stack.pop_boolean_with_pos()); @@ -1783,7 +1802,7 @@ mod tests { #[test] fn test_scope_return_integer() { let mut stack = Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, &[], 0, LineCol { line: 50, col: 60 }); + let scope = Scope::new(&mut stack, &[], None, 0, LineCol { line: 50, col: 60 }); assert!(scope.return_integer(7).is_ok()); assert_eq!((7, LineCol { line: 50, col: 60 }), stack.pop_integer_with_pos()); assert_eq!((false, LineCol { line: 1, col: 2 }), stack.pop_boolean_with_pos()); @@ -1792,7 +1811,7 @@ mod tests { #[test] fn test_scope_return_string() { let mut stack = Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, &[], 0, LineCol { line: 50, col: 60 }); + let scope = Scope::new(&mut stack, &[], None, 0, LineCol { line: 50, col: 60 }); assert!(scope.return_string("foo").is_ok()); assert_eq!(("foo".to_owned(), LineCol { line: 50, col: 60 }), stack.pop_string_with_pos()); assert_eq!((false, LineCol { line: 1, col: 2 }), stack.pop_boolean_with_pos()); diff --git a/core/src/testutils.rs b/core/src/testutils.rs index 34c128bc..aea0fb2b 100644 --- a/core/src/testutils.rs +++ b/core/src/testutils.rs @@ -232,10 +232,13 @@ impl Callable for LastErrorFunction { &self.metadata } - async fn exec(&self, scope: Scope<'_>, machine: &mut Machine) -> Result<()> { + async fn exec(&self, scope: Scope<'_>, _machine: &mut Machine) -> Result<()> { assert_eq!(0, scope.nargs()); - match machine.last_error() { - Some(message) => scope.return_string(message), + match scope.last_error() { + Some(message) => { + let message = message.to_owned(); + scope.return_string(message) + } None => scope.return_string("".to_owned()), } } diff --git a/std/src/exec.rs b/std/src/exec.rs index 03228b2e..550df69a 100644 --- a/std/src/exec.rs +++ b/std/src/exec.rs @@ -99,11 +99,14 @@ impl Callable for ErrmsgFunction { &self.metadata } - async fn exec(&self, scope: Scope<'_>, machine: &mut Machine) -> Result<()> { + async fn exec(&self, scope: Scope<'_>, _machine: &mut Machine) -> Result<()> { debug_assert_eq!(0, scope.nargs()); - match machine.last_error() { - Some(message) => scope.return_string(message), + match scope.last_error() { + Some(message) => { + let message = message.to_owned(); + scope.return_string(message) + } None => scope.return_string("".to_owned()), } } From 10ede9e112ceee410d9033406144875e4d2efde1 Mon Sep 17 00:00:00 2001 From: Julio Merino Date: Fri, 15 Nov 2024 19:21:41 -0800 Subject: [PATCH 5/9] Pull upcall handling code into a separate function To keep the execution loop simpler, move upcall handling into its own function. This function is private for now, but it will become public once we are able to pull out the whole execution loop to the caller. --- core/src/exec.rs | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/core/src/exec.rs b/core/src/exec.rs index a47b5f69..00ab5692 100644 --- a/core/src/exec.rs +++ b/core/src/exec.rs @@ -1508,6 +1508,25 @@ impl Machine { Ok(InternalStopReason::Eof) } + /// Handles an upcall to the builtin callable described in `upcall`. + async fn handle_upcall(&mut self, context: &mut Context, upcall: &UpcallData) -> Result<()> { + let result; + if let Some(return_type) = upcall.return_type { + result = self + .function_call(context, &upcall.name, return_type, upcall.pos, upcall.nargs) + .await; + } else { + result = self.builtin_call(context, &upcall.name, upcall.pos, upcall.nargs).await; + } + match result { + Ok(()) => { + context.pc += 1; + Ok(()) + } + Err(e) => self.handle_error(context, e), + } + } + /// Handles the given error `e` according to the current error handler previously set by /// `ON ERROR`. If the error can be handled gracefully, returns `Ok`; otherwise, returns the /// input error unmodified. @@ -1564,18 +1583,7 @@ impl Machine { } Ok(InternalStopReason::Upcall(data)) => { - let result; - if let Some(return_type) = data.return_type { - result = self - .function_call(context, &data.name, return_type, data.pos, data.nargs) - .await; - } else { - result = self.builtin_call(context, &data.name, data.pos, data.nargs).await; - } - match result { - Ok(()) => context.pc += 1, - Err(e) => self.handle_error(context, e)?, - } + self.handle_upcall(context, &data).await?; } Ok(InternalStopReason::Eof) => { From c70a2c5c73096e65980552b92efee46a7e289edd Mon Sep 17 00:00:00 2001 From: Julio Merino Date: Sat, 16 Nov 2024 07:44:31 -0800 Subject: [PATCH 6/9] Simplify Scope creation Now that Scope requires multiple parts of the Context, pass in the Context as a parameter. This will allow us to interact with even more parts of the Context. --- core/src/exec.rs | 247 +++++++++++++++++++++++++++-------------------- 1 file changed, 142 insertions(+), 105 deletions(-) diff --git a/core/src/exec.rs b/core/src/exec.rs index 00ab5692..df5419c4 100644 --- a/core/src/exec.rs +++ b/core/src/exec.rs @@ -354,41 +354,33 @@ impl Stack { /// Provides controlled access to the environment passed to a callable. pub struct Scope<'s> { - stack: &'s mut Stack, - data: &'s [Option], - last_error: Option<&'s str>, + context: &'s mut Context, nargs: usize, fref_pos: LineCol, } impl Drop for Scope<'_> { fn drop(&mut self) { - self.stack.discard(self.nargs); + self.context.value_stack.discard(self.nargs); } } impl<'s> Scope<'s> { /// Creates a new scope that wraps `nargs` arguments at the top of `stack` and that offers /// access to `data`. - fn new( - stack: &'s mut Stack, - data: &'s [Option], - last_error: Option<&'s str>, - nargs: usize, - fref_pos: LineCol, - ) -> Self { - debug_assert!(nargs <= stack.len()); - Self { stack, data, last_error, nargs, fref_pos } + fn new(context: &'s mut Context, nargs: usize, fref_pos: LineCol) -> Self { + debug_assert!(nargs <= context.value_stack.len()); + Self { context, nargs, fref_pos } } /// Obtains immutable access to the data values available during the *current* execution. pub fn data(&self) -> &[Option] { - self.data + &self.context.data } /// Removes all remaining arguments from the stack tracked by this scope. fn drain(&mut self) { - self.stack.discard(self.nargs); + self.context.value_stack.discard(self.nargs); self.nargs = 0; } @@ -404,7 +396,7 @@ impl<'s> Scope<'s> { /// Returns the last execution error. pub fn last_error(&self) -> Option<&str> { - self.last_error + self.context.last_error.as_deref() } /// Returns the number of arguments that can still be consumed. @@ -424,7 +416,7 @@ impl<'s> Scope<'s> { pub fn pop_boolean_with_pos(&mut self) -> (bool, LineCol) { debug_assert!(self.nargs > 0, "Not enough arguments in scope"); self.nargs -= 1; - self.stack.pop_boolean_with_pos() + self.context.value_stack.pop_boolean_with_pos() } /// Pops the top of the stack as a double value. @@ -439,7 +431,7 @@ impl<'s> Scope<'s> { pub fn pop_double_with_pos(&mut self) -> (f64, LineCol) { debug_assert!(self.nargs > 0, "Not enough arguments in scope"); self.nargs -= 1; - self.stack.pop_double_with_pos() + self.context.value_stack.pop_double_with_pos() } /// Pops the top of the stack as an integer value. @@ -454,7 +446,7 @@ impl<'s> Scope<'s> { pub fn pop_integer_with_pos(&mut self) -> (i32, LineCol) { debug_assert!(self.nargs > 0, "Not enough arguments in scope"); self.nargs -= 1; - self.stack.pop_integer_with_pos() + self.context.value_stack.pop_integer_with_pos() } /// Pops the top of the stack as a separator tag. @@ -485,7 +477,7 @@ impl<'s> Scope<'s> { pub fn pop_string_with_pos(&mut self) -> (String, LineCol) { debug_assert!(self.nargs > 0, "Not enough arguments in scope"); self.nargs -= 1; - self.stack.pop_string_with_pos() + self.context.value_stack.pop_string_with_pos() } /// Pops the top of the stack as a value tag. @@ -519,41 +511,41 @@ impl<'s> Scope<'s> { pub fn pop_varref_with_pos(&mut self) -> (SymbolKey, ExprType, LineCol) { debug_assert!(self.nargs > 0, "Not enough arguments in scope"); self.nargs -= 1; - self.stack.pop_varref_with_pos() + self.context.value_stack.pop_varref_with_pos() } /// Sets the return value of this function to `value`. pub fn return_any(mut self, value: Value) -> Result<()> { self.drain(); - self.stack.push((value, self.fref_pos)); + self.context.value_stack.push((value, self.fref_pos)); Ok(()) } /// Sets the return value of this function to the boolean `value`. pub fn return_boolean(mut self, value: bool) -> Result<()> { self.drain(); - self.stack.push((Value::Boolean(value), self.fref_pos)); + self.context.value_stack.push((Value::Boolean(value), self.fref_pos)); Ok(()) } /// Sets the return value of this function to the double `value`. pub fn return_double(mut self, value: f64) -> Result<()> { self.drain(); - self.stack.push((Value::Double(value), self.fref_pos)); + self.context.value_stack.push((Value::Double(value), self.fref_pos)); Ok(()) } /// Sets the return value of this function to the integer `value`. pub fn return_integer(mut self, value: i32) -> Result<()> { self.drain(); - self.stack.push((Value::Integer(value), self.fref_pos)); + self.context.value_stack.push((Value::Integer(value), self.fref_pos)); Ok(()) } /// Sets the return value of this function to the string `value`. pub fn return_string>(mut self, value: S) -> Result<()> { self.drain(); - self.stack.push((Value::Text(value.into()), self.fref_pos)); + self.context.value_stack.push((Value::Text(value.into()), self.fref_pos)); Ok(()) } } @@ -569,11 +561,11 @@ pub struct Context { err_handler: ErrorHandlerISpan, } -impl From for Context { - fn from(image: Image) -> Self { +impl Default for Context { + fn default() -> Self { Self { - instrs: image.instrs, - data: image.data, + instrs: vec![], + data: vec![], last_error: None, pc: 0, addr_stack: vec![], @@ -583,6 +575,12 @@ impl From for Context { } } +impl From for Context { + fn from(image: Image) -> Self { + Context { instrs: image.instrs, data: image.data, ..Default::default() } + } +} + /// Executes an EndBASIC program and tracks its state. pub struct Machine { symbols: Symbols, @@ -709,13 +707,7 @@ impl Machine { let metadata = b.metadata(); debug_assert!(!metadata.is_function()); - let scope = Scope::new( - &mut context.value_stack, - &context.data, - context.last_error.as_deref(), - nargs, - bref_pos, - ); + let scope = Scope::new(context, nargs, bref_pos); let b = b.clone(); b.exec(scope, self).await @@ -928,13 +920,7 @@ impl Machine { let metadata = f.metadata(); debug_assert_eq!(return_type, metadata.return_type().unwrap()); - let scope = Scope::new( - &mut context.value_stack, - &context.data, - context.last_error.as_deref(), - nargs, - fref_pos, - ); + let scope = Scope::new(context, nargs, fref_pos); f.exec(scope, self).await?; if cfg!(debug_assertions) { match context.value_stack.top() { @@ -1011,13 +997,7 @@ impl Machine { fpos: LineCol, f: Rc, ) -> Result<()> { - let scope = Scope::new( - &mut context.value_stack, - &context.data, - context.last_error.as_deref(), - 0, - fpos, - ); + let scope = Scope::new(context, 0, fpos); f.exec(scope, self).await?; if cfg!(debug_assertions) { match context.value_stack.top() { @@ -1707,49 +1687,64 @@ mod tests { #[test] fn test_scope_and_stack_empty() { - let mut stack = Stack::from([]); - let scope = Scope::new(&mut stack, &[], None, 0, LineCol { line: 50, col: 60 }); + let mut context = Context::default(); + let scope = Scope::new(&mut context, 0, LineCol { line: 50, col: 60 }); drop(scope); - assert_eq!(0, stack.len()); + assert_eq!(0, context.value_stack.len()); } #[test] fn test_scope_no_args() { - let mut stack = Stack::from([(Value::Integer(3), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, &[], None, 0, LineCol { line: 50, col: 60 }); + let mut context = Context { + value_stack: Stack::from([(Value::Integer(3), LineCol { line: 1, col: 2 })]), + ..Default::default() + }; + let scope = Scope::new(&mut context, 0, LineCol { line: 50, col: 60 }); drop(scope); - assert_eq!(1, stack.len()); + assert_eq!(1, context.value_stack.len()); } #[test] fn test_scope_pop_remaining_on_drop() { - let mut stack = Stack::from([ - (Value::Integer(3), LineCol { line: 1, col: 2 }), - (Value::Integer(1), LineCol { line: 1, col: 2 }), - (Value::Integer(2), LineCol { line: 1, col: 2 }), - (Value::Integer(4), LineCol { line: 1, col: 2 }), - ]); - let mut scope = Scope::new(&mut stack, &[], None, 3, LineCol { line: 50, col: 60 }); + let mut context = Context { + value_stack: Stack::from([ + (Value::Integer(3), LineCol { line: 1, col: 2 }), + (Value::Integer(1), LineCol { line: 1, col: 2 }), + (Value::Integer(2), LineCol { line: 1, col: 2 }), + (Value::Integer(4), LineCol { line: 1, col: 2 }), + ]), + ..Default::default() + }; + let mut scope = Scope::new(&mut context, 3, LineCol { line: 50, col: 60 }); assert_eq!(3, scope.nargs()); assert_eq!(4, scope.pop_integer()); assert_eq!(2, scope.nargs()); assert_eq!(2, scope.pop_integer()); assert_eq!(1, scope.nargs()); drop(scope); - assert_eq!(1, stack.len()); - assert_eq!((Value::Integer(3), LineCol { line: 1, col: 2 }), stack.pop().unwrap()); + assert_eq!(1, context.value_stack.len()); + assert_eq!( + (Value::Integer(3), LineCol { line: 1, col: 2 }), + context.value_stack.pop().unwrap() + ); } #[test] fn test_scope_pop_types() { - let mut stack = Stack::from([ - (Value::Boolean(false), LineCol { line: 1, col: 2 }), - (Value::Double(1.2), LineCol { line: 1, col: 2 }), - (Value::Integer(2), LineCol { line: 1, col: 2 }), - (Value::Text("foo".to_owned()), LineCol { line: 1, col: 2 }), - (Value::VarRef(SymbolKey::from("foo"), ExprType::Integer), LineCol { line: 1, col: 2 }), - ]); - let mut scope = Scope::new(&mut stack, &[], None, 5, LineCol { line: 50, col: 60 }); + let mut context = Context { + value_stack: Stack::from([ + (Value::Boolean(false), LineCol { line: 1, col: 2 }), + (Value::Double(1.2), LineCol { line: 1, col: 2 }), + (Value::Integer(2), LineCol { line: 1, col: 2 }), + (Value::Text("foo".to_owned()), LineCol { line: 1, col: 2 }), + ( + Value::VarRef(SymbolKey::from("foo"), ExprType::Integer), + LineCol { line: 1, col: 2 }, + ), + ]), + ..Default::default() + }; + let mut scope = Scope::new(&mut context, 5, LineCol { line: 50, col: 60 }); assert_eq!((SymbolKey::from("foo"), ExprType::Integer), scope.pop_varref()); assert_eq!("foo", scope.pop_string()); assert_eq!(2, scope.pop_integer()); @@ -1759,17 +1754,20 @@ mod tests { #[test] fn test_scope_pop_types_with_pos() { - let mut stack = Stack::from([ - (Value::Boolean(false), LineCol { line: 1, col: 2 }), - (Value::Double(1.2), LineCol { line: 3, col: 4 }), - (Value::Integer(2), LineCol { line: 5, col: 6 }), - (Value::Text("foo".to_owned()), LineCol { line: 7, col: 8 }), - ( - Value::VarRef(SymbolKey::from("foo"), ExprType::Integer), - LineCol { line: 9, col: 10 }, - ), - ]); - let mut scope = Scope::new(&mut stack, &[], None, 5, LineCol { line: 50, col: 60 }); + let mut context = Context { + value_stack: Stack::from([ + (Value::Boolean(false), LineCol { line: 1, col: 2 }), + (Value::Double(1.2), LineCol { line: 3, col: 4 }), + (Value::Integer(2), LineCol { line: 5, col: 6 }), + (Value::Text("foo".to_owned()), LineCol { line: 7, col: 8 }), + ( + Value::VarRef(SymbolKey::from("foo"), ExprType::Integer), + LineCol { line: 9, col: 10 }, + ), + ]), + ..Default::default() + }; + let mut scope = Scope::new(&mut context, 5, LineCol { line: 50, col: 60 }); assert_eq!( (SymbolKey::from("foo"), ExprType::Integer, LineCol { line: 9, col: 10 }), scope.pop_varref_with_pos() @@ -1782,47 +1780,86 @@ mod tests { #[test] fn test_scope_return_any() { - let mut stack = Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, &[], None, 0, LineCol { line: 50, col: 60 }); + let mut context = Context { + value_stack: Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]), + ..Default::default() + }; + let scope = Scope::new(&mut context, 0, LineCol { line: 50, col: 60 }); assert!(scope.return_any(Value::Boolean(true)).is_ok()); - assert_eq!((true, LineCol { line: 50, col: 60 }), stack.pop_boolean_with_pos()); - assert_eq!((false, LineCol { line: 1, col: 2 }), stack.pop_boolean_with_pos()); + assert_eq!( + (true, LineCol { line: 50, col: 60 }), + context.value_stack.pop_boolean_with_pos() + ); + assert_eq!( + (false, LineCol { line: 1, col: 2 }), + context.value_stack.pop_boolean_with_pos() + ); } #[test] fn test_scope_return_boolean() { - let mut stack = Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, &[], None, 0, LineCol { line: 50, col: 60 }); + let mut context = Context { + value_stack: Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]), + ..Default::default() + }; + let scope = Scope::new(&mut context, 0, LineCol { line: 50, col: 60 }); assert!(scope.return_boolean(true).is_ok()); - assert_eq!((true, LineCol { line: 50, col: 60 }), stack.pop_boolean_with_pos()); - assert_eq!((false, LineCol { line: 1, col: 2 }), stack.pop_boolean_with_pos()); + assert_eq!( + (true, LineCol { line: 50, col: 60 }), + context.value_stack.pop_boolean_with_pos() + ); + assert_eq!( + (false, LineCol { line: 1, col: 2 }), + context.value_stack.pop_boolean_with_pos() + ); } #[test] fn test_scope_return_double() { - let mut stack = Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, &[], None, 0, LineCol { line: 50, col: 60 }); + let mut context = Context { + value_stack: Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]), + ..Default::default() + }; + let scope = Scope::new(&mut context, 0, LineCol { line: 50, col: 60 }); assert!(scope.return_double(4.5).is_ok()); - assert_eq!((4.5, LineCol { line: 50, col: 60 }), stack.pop_double_with_pos()); - assert_eq!((false, LineCol { line: 1, col: 2 }), stack.pop_boolean_with_pos()); + assert_eq!((4.5, LineCol { line: 50, col: 60 }), context.value_stack.pop_double_with_pos()); + assert_eq!( + (false, LineCol { line: 1, col: 2 }), + context.value_stack.pop_boolean_with_pos() + ); } #[test] fn test_scope_return_integer() { - let mut stack = Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, &[], None, 0, LineCol { line: 50, col: 60 }); + let mut context = Context { + value_stack: Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]), + ..Default::default() + }; + let scope = Scope::new(&mut context, 0, LineCol { line: 50, col: 60 }); assert!(scope.return_integer(7).is_ok()); - assert_eq!((7, LineCol { line: 50, col: 60 }), stack.pop_integer_with_pos()); - assert_eq!((false, LineCol { line: 1, col: 2 }), stack.pop_boolean_with_pos()); + assert_eq!((7, LineCol { line: 50, col: 60 }), context.value_stack.pop_integer_with_pos()); + assert_eq!( + (false, LineCol { line: 1, col: 2 }), + context.value_stack.pop_boolean_with_pos() + ); } #[test] fn test_scope_return_string() { - let mut stack = Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]); - let scope = Scope::new(&mut stack, &[], None, 0, LineCol { line: 50, col: 60 }); + let mut context = Context { + value_stack: Stack::from([(Value::Boolean(false), LineCol { line: 1, col: 2 })]), + ..Default::default() + }; + let scope = Scope::new(&mut context, 0, LineCol { line: 50, col: 60 }); assert!(scope.return_string("foo").is_ok()); - assert_eq!(("foo".to_owned(), LineCol { line: 50, col: 60 }), stack.pop_string_with_pos()); - assert_eq!((false, LineCol { line: 1, col: 2 }), stack.pop_boolean_with_pos()); + assert_eq!( + ("foo".to_owned(), LineCol { line: 50, col: 60 }), + context.value_stack.pop_string_with_pos() + ); + assert_eq!( + (false, LineCol { line: 1, col: 2 }), + context.value_stack.pop_boolean_with_pos() + ); } #[test] From c21e5f99fa731609242b85859436ed1649775252 Mon Sep 17 00:00:00 2001 From: Julio Merino Date: Mon, 18 Nov 2024 08:15:41 -0800 Subject: [PATCH 7/9] Handle chained executions (RUN) via the main loop Instead of having the RUN command execute the stored program in a nested machine invocation, chain the execution contexts so that the machine's main loop can handle the nesting on its own. This should allow exposing the execution loop to callers as a fundamental primitive. --- core/examples/config.rs | 6 ++- core/examples/dsl.rs | 6 ++- core/src/exec.rs | 109 ++++++++++++++++++++++++++++------------ repl/src/lib.rs | 23 ++++++--- std/src/program.rs | 34 +++---------- 5 files changed, 109 insertions(+), 69 deletions(-) diff --git a/core/examples/config.rs b/core/examples/config.rs index 52573c9f..9e25e18b 100644 --- a/core/examples/config.rs +++ b/core/examples/config.rs @@ -44,8 +44,10 @@ fn main() { loop { match block_on(machine.exec(&mut context)).expect("Execution error") { StopReason::Eof => break, - StopReason::Exited(i) => println!("Script explicitly exited with code {}", i), - StopReason::Break => (), // Ignore signals. + StopReason::Exited(i, _is_final) => { + println!("Script explicitly exited with code {}", i) + } + StopReason::Break(_is_final) => (), // Ignore signals. } } diff --git a/core/examples/dsl.rs b/core/examples/dsl.rs index 213b6340..7e2709ba 100644 --- a/core/examples/dsl.rs +++ b/core/examples/dsl.rs @@ -154,8 +154,10 @@ fn main() { loop { match block_on(machine.exec(&mut context)).expect("Execution error") { StopReason::Eof => break, - StopReason::Exited(i) => println!("Script explicitly exited with code {}", i), - StopReason::Break => (), // Ignore signals. + StopReason::Exited(i, _is_final) => { + println!("Script explicitly exited with code {}", i) + } + StopReason::Break(_is_final) => (), // Ignore signals. } } diff --git a/core/src/exec.rs b/core/src/exec.rs index df5419c4..baad010a 100644 --- a/core/src/exec.rs +++ b/core/src/exec.rs @@ -135,10 +135,18 @@ pub enum StopReason { Eof, /// Execution terminated because the machine was asked to terminate with `END`. - Exited(u8), + /// + /// The boolean indicates whether this stop is final. If false, the stop request comes from a + /// nested invocation triggered by `RUN` so the caller may want to capture the stop and print a + /// message instead of exiting. + Exited(u8, bool), /// Execution terminated because the machine received a break signal. - Break, + /// + /// The boolean indicates whether this stop is final. If false, the stop request comes from + /// cancelling the execution of a nested `RUN` so the caller may want to capture the stop and + /// print a message instead of exiting. + Break(bool), } impl StopReason { @@ -146,8 +154,8 @@ impl StopReason { pub fn as_exit_code(&self) -> i32 { match self { StopReason::Eof => 0, - StopReason::Exited(i) => *i as i32, - StopReason::Break => { + StopReason::Exited(i, _is_final) => *i as i32, + StopReason::Break(_is_final) => { // This mimics the behavior of typical Unix shells, which translate a signal to a // numerical exit code, but this is not accurate. First, because a CTRL+C sequence // should be exposed as a SIGINT signal to whichever process is waiting for us, and @@ -548,6 +556,14 @@ impl<'s> Scope<'s> { self.context.value_stack.push((Value::Text(value.into()), self.fref_pos)); Ok(()) } + + /// Asks the machine to continue execution on `next_context`, abandoning the current execution. + /// + /// This exists to support the semantics of the `RUN` command. + pub fn exec_next(self, next_context: Context) -> Result<()> { + self.context.run_next = Some(Box::from(next_context)); + Ok(()) + } } /// Machine state for the execution of an individual chunk of code. @@ -559,6 +575,8 @@ pub struct Context { addr_stack: Vec
, value_stack: Stack, err_handler: ErrorHandlerISpan, + run_next: Option>, + run_prev: Option>, } impl Default for Context { @@ -571,6 +589,8 @@ impl Default for Context { addr_stack: vec![], value_stack: Stack::default(), err_handler: ErrorHandlerISpan::None, + run_next: None, + run_prev: None, } } } @@ -1501,6 +1521,17 @@ impl Machine { match result { Ok(()) => { context.pc += 1; + + // If we are coming back from a `RUN` command invocation, swap the current context + // with the one passed to `RUN` (stored in `run_next`), and save the current context + // in `run_prev` so that we can resume execution later on. + if let Some(mut next) = context.run_next.take() { + let mut prev = Context::default(); + std::mem::swap(context, &mut prev); + next.run_prev = Some(Box::from(prev)); + *context = *next; + } + Ok(()) } Err(e) => self.handle_error(context, e), @@ -1554,27 +1585,43 @@ impl Machine { /// Executes the instructions in `context`. pub async fn exec(&mut self, context: &mut Context) -> Result { - while context.pc < context.instrs.len() { - match self.exec_until_stop(context) { - Ok(InternalStopReason::CheckStop) => { - if self.should_stop().await { - return Ok(StopReason::Break); + loop { + while context.pc < context.instrs.len() { + match self.exec_until_stop(context) { + Ok(InternalStopReason::CheckStop) => { + if self.should_stop().await { + return Ok(StopReason::Break(context.run_prev.is_none())); + } } - } - Ok(InternalStopReason::Upcall(data)) => { - self.handle_upcall(context, &data).await?; - } + Ok(InternalStopReason::Upcall(data)) => { + self.handle_upcall(context, &data).await?; + } - Ok(InternalStopReason::Eof) => { - return Ok(StopReason::Eof); - } + Ok(InternalStopReason::Eof) => { + if context.run_prev.is_some() { + break; + } + + return Ok(StopReason::Eof); + } - Ok(InternalStopReason::Exited(code)) => { - return Ok(StopReason::Exited(code)); + Ok(InternalStopReason::Exited(code)) => { + if context.run_prev.is_some() { + break; + } + + return Ok(StopReason::Exited(code, context.run_prev.is_none())); + } + + Err(e) => self.handle_error(context, e)?, } + } - Err(e) => self.handle_error(context, e)?, + if let Some(prev) = context.run_prev.take() { + *context = *prev; + } else { + break; } } Ok(StopReason::Eof) @@ -2131,7 +2178,7 @@ mod tests { fn test_end_no_code() { let captured_out = Rc::from(RefCell::from(vec![])); assert_eq!( - StopReason::Exited(5), + StopReason::Exited(5, true), run("OUT 1\nEND 5\nOUT 2", &[], captured_out.clone()).expect("Execution failed") ); assert_eq!(&["1"], captured_out.borrow().as_slice()); @@ -2140,7 +2187,7 @@ mod tests { fn do_end_with_code_test(code: u8) { let captured_out = Rc::from(RefCell::from(vec![])); assert_eq!( - StopReason::Exited(code), + StopReason::Exited(code, true), run(&format!("OUT 1: END {}: OUT 2", code), &[], captured_out.clone()) .expect("Execution failed") ); @@ -2148,7 +2195,7 @@ mod tests { let captured_out = Rc::from(RefCell::from(vec![])); assert_eq!( - StopReason::Exited(code), + StopReason::Exited(code, true), run(&format!("OUT 1: END {}.2: OUT 2", code), &[], captured_out.clone()) .expect("Execution failed") ); @@ -2185,7 +2232,7 @@ mod tests { OUT OUTF(5, 500) END IF "#; - assert_eq!(StopReason::Exited(0), run(input, &[], captured_out.clone()).unwrap()); + assert_eq!(StopReason::Exited(0, true), run(input, &[], captured_out.clone()).unwrap()); assert_eq!(&["100", "1"], captured_out.borrow().as_slice()); } @@ -2193,7 +2240,7 @@ mod tests { fn test_end_for() { let captured_out = Rc::from(RefCell::from(vec![])); let input = r#"FOR i = 1 TO OUTF(10, i * 100): IF i = 3 THEN: END: END IF: OUT i: NEXT"#; - assert_eq!(StopReason::Exited(0), run(input, &[], captured_out.clone()).unwrap()); + assert_eq!(StopReason::Exited(0, true), run(input, &[], captured_out.clone()).unwrap()); assert_eq!(&["100", "1", "200", "2", "300"], captured_out.borrow().as_slice()); } @@ -2201,7 +2248,7 @@ mod tests { fn test_end_while() { let captured_out = Rc::from(RefCell::from(vec![])); let input = r#"i = 1: WHILE i < OUTF(10, i * 100): IF i = 4 THEN: END: END IF: OUT i: i = i + 1: WEND"#; - assert_eq!(StopReason::Exited(0), run(input, &[], captured_out.clone()).unwrap()); + assert_eq!(StopReason::Exited(0, true), run(input, &[], captured_out.clone()).unwrap()); assert_eq!(&["100", "1", "200", "2", "300", "3", "400"], captured_out.borrow().as_slice()); } @@ -2209,7 +2256,7 @@ mod tests { fn test_end_nested() { let captured_out = Rc::from(RefCell::from(vec![])); assert_eq!( - StopReason::Exited(42), + StopReason::Exited(42, true), run( "FOR a = 0 TO 10\nOUT a\nIF a = 3 THEN\nEND 42\nOUT \"no\"\nEND IF\nNEXT", &[], @@ -2228,7 +2275,7 @@ mod tests { machine.add_callable(SumFunction::new()); assert_eq!( - StopReason::Exited(10), + StopReason::Exited(10, true), block_on(machine.compile_and_exec(&mut "OUT 1\nEND 10\nOUT 2".as_bytes())) .expect("Execution failed") ); @@ -2236,7 +2283,7 @@ mod tests { captured_out.borrow_mut().clear(); assert_eq!( - StopReason::Exited(11), + StopReason::Exited(11, true), block_on(machine.compile_and_exec(&mut "OUT 2\nEND 11\nOUT 3".as_bytes())) .expect("Execution failed") ); @@ -2264,7 +2311,7 @@ mod tests { signals_tx.send(Signal::Break).await.unwrap(); let result = future.await; - assert_eq!(StopReason::Break, result.unwrap()); + assert_eq!(StopReason::Break(true), result.unwrap()); } } @@ -2302,7 +2349,7 @@ mod tests { tx.send(Signal::Break).await.unwrap(); let input = &mut code.as_bytes(); - assert_eq!(StopReason::Break, machine.compile_and_exec(input).await.unwrap()); + assert_eq!(StopReason::Break(true), machine.compile_and_exec(input).await.unwrap()); assert_eq!(0, tx.len()); } @@ -2820,7 +2867,7 @@ mod tests { fn test_goto_as_last_statement() { let captured_out = Rc::from(RefCell::from(vec![])); assert_eq!( - StopReason::Exited(5), + StopReason::Exited(5, true), run( "i = 0: @a: IF i = 5 THEN: END i: END IF: i = i + 1: GOTO @a", &[], diff --git a/repl/src/lib.rs b/repl/src/lib.rs index b56fe2cf..6bb17f7c 100644 --- a/repl/src/lib.rs +++ b/repl/src/lib.rs @@ -24,7 +24,7 @@ use endbasic_core::exec::{Machine, StopReason}; use endbasic_std::console::{self, is_narrow, refill_and_print, Console}; -use endbasic_std::program::{continue_if_modified, Program, BREAK_MSG}; +use endbasic_std::program::{continue_if_modified, Program}; use endbasic_std::storage::Storage; use std::cell::RefCell; use std::io; @@ -33,6 +33,9 @@ use std::rc::Rc; pub mod demos; pub mod editor; +/// Message to print on the console when receiving a break signal. +const BREAK_MSG: &str = "**** BREAK ****"; + /// Prints the EndBASIC welcome message to the given console. pub fn print_welcome(console: Rc>) -> io::Result<()> { let mut console = console.borrow_mut(); @@ -132,12 +135,12 @@ pub async fn run_from_cloud( console.print("**** Program exited due to EOF ****")?; r.as_exit_code() } - Ok(r @ StopReason::Exited(_)) => { + Ok(r @ StopReason::Exited(_code, _is_final)) => { let code = r.as_exit_code(); console.print(&format!("**** Program exited with code {} ****", code))?; code } - Ok(r @ StopReason::Break) => { + Ok(r @ StopReason::Break(_is_final)) => { console.print("**** Program stopped due to BREAK ****")?; r.as_exit_code() } @@ -216,20 +219,26 @@ pub async fn run_repl_loop( } else if e.kind() == io::ErrorKind::UnexpectedEof { let mut console = console.borrow_mut(); console.print("End of input by CTRL-D")?; - stop_reason = StopReason::Exited(0); + stop_reason = StopReason::Exited(0, true); } else { - stop_reason = StopReason::Exited(1); + stop_reason = StopReason::Exited(1, true); } } } match stop_reason { StopReason::Eof => (), - StopReason::Break => { + StopReason::Break(_is_final) => { console.borrow_mut().print("**** BREAK ****")?; stop_reason = StopReason::Eof; } - StopReason::Exited(_) => { + StopReason::Exited(_, false) => { + console + .borrow_mut() + .print(&format!("Program exited with code {}", stop_reason.as_exit_code()))?; + stop_reason = StopReason::Eof; + } + StopReason::Exited(_, true) => { if !continue_if_modified(&*program.borrow(), &mut *console.borrow_mut()).await? { console.borrow_mut().print("Exit aborted; resuming REPL loop.")?; stop_reason = StopReason::Eof; diff --git a/std/src/program.rs b/std/src/program.rs index aa031a68..449d451a 100644 --- a/std/src/program.rs +++ b/std/src/program.rs @@ -21,7 +21,7 @@ use crate::strings::parse_boolean; use async_trait::async_trait; use endbasic_core::ast::ExprType; use endbasic_core::compiler::{compile, ArgSepSyntax, RequiredValueSyntax, SingularArgSyntax}; -use endbasic_core::exec::{Machine, Result, Scope, StopReason}; +use endbasic_core::exec::{Machine, Result, Scope}; use endbasic_core::syms::{Callable, CallableMetadata, CallableMetadataBuilder}; use std::borrow::Cow; use std::cell::RefCell; @@ -44,9 +44,6 @@ have forgotten to do so, but it's better to get in the habit of saving often. See the \"File system\" help topic for information on where the programs can be saved and loaded \ from."; -/// Message to print on the console when receiving a break signal. -pub const BREAK_MSG: &str = "**** BREAK ****"; - /// Representation of the single program that we can keep in memory. #[async_trait(?Send)] pub trait Program { @@ -482,7 +479,6 @@ impl Callable for NewCommand { /// The `RUN` command. pub struct RunCommand { metadata: CallableMetadata, - console: Rc>, program: Rc>, } @@ -490,7 +486,7 @@ impl RunCommand { /// Creates a new `RUN` command that executes the `program`. /// /// Reports any non-successful return codes from the program to the console. - pub fn new(console: Rc>, program: Rc>) -> Rc { + pub fn new(program: Rc>) -> Rc { Rc::from(Self { metadata: CallableMetadataBuilder::new("RUN") .with_syntax(&[(&[], None)]) @@ -501,7 +497,6 @@ This issues a CLEAR operation before starting the program to prevent previous le from interfering with the new execution.", ) .build(), - console, program, }) } @@ -515,25 +510,10 @@ impl Callable for RunCommand { async fn exec(&self, scope: Scope<'_>, machine: &mut Machine) -> Result<()> { debug_assert_eq!(0, scope.nargs()); - - machine.clear(); let program = self.program.borrow().text(); - let mut context = machine.compile(&mut program.as_bytes())?; - let stop_reason = machine.exec(&mut context).await?; - match stop_reason { - StopReason::Break => { - self.console.borrow_mut().print(BREAK_MSG).map_err(|e| scope.io_error(e))? - } - stop_reason => { - if stop_reason.as_exit_code() != 0 { - self.console - .borrow_mut() - .print(&format!("Program exited with code {}", stop_reason.as_exit_code())) - .map_err(|e| scope.io_error(e))?; - } - } - } - Ok(()) + machine.clear(); + let context = machine.compile(&mut program.as_bytes())?; + scope.exec_next(context) } } @@ -633,7 +613,7 @@ pub fn add_all( machine.add_callable(ListCommand::new(console.clone(), program.clone())); machine.add_callable(LoadCommand::new(console.clone(), storage.clone(), program.clone())); machine.add_callable(NewCommand::new(console.clone(), program.clone())); - machine.add_callable(RunCommand::new(console.clone(), program.clone())); + machine.add_callable(RunCommand::new(program.clone())); machine.add_callable(SaveCommand::new(console, storage, program)); } @@ -1034,7 +1014,7 @@ mod tests { .set_program(Some("untouched.bas"), program) .run(r#"RUN: PRINT "after""#) .expect_clear() - .expect_prints([" 5", "Program exited with code 1", "after"]) + .expect_prints([" 5", "after"]) .expect_program(Some("untouched.bas"), program) .check(); } From 5358f325b5d8f599e1aaabc0e3027808d4da04e9 Mon Sep 17 00:00:00 2001 From: Julio Merino Date: Mon, 18 Nov 2024 19:09:49 -0800 Subject: [PATCH 8/9] Pull the main loop from the Machine to the REPL Add a new Machine::resume() method that takes a Context and returns control as soon as the machine hits an interruptible point. This allows the REPL to implement its own execution loop and is one step more towards the removal of async from the core. --- core/src/exec.rs | 105 +++++++++++++++++++++++++++++------------------ repl/src/lib.rs | 44 ++++++++++++++++---- 2 files changed, 101 insertions(+), 48 deletions(-) diff --git a/core/src/exec.rs b/core/src/exec.rs index baad010a..bc6d48a4 100644 --- a/core/src/exec.rs +++ b/core/src/exec.rs @@ -87,7 +87,7 @@ pub enum Signal { /// Request to exit the VM execution loop to execute a native command or function. #[derive(Clone, Debug, Eq, PartialEq)] -struct UpcallData { +pub struct UpcallData { /// Name of the callable to execute. name: SymbolKey, @@ -111,16 +111,24 @@ struct UpcallData { /// inner loop of the bytecode interpreter sync. Early benchmarks showed a 25% performance /// improvement just by removing async from the loop and pushing the infrequent async awaits to an /// outer loop. -enum InternalStopReason { +pub enum InternalStopReason { /// Execution terminated because the bytecode reached a point in the instructions where an /// interruption, if any, should be processed. - CheckStop, + /// + /// The boolean indicates whether this stop is final. If false, the stop request comes from a + /// nested invocation triggered by `RUN` so the caller may want to capture the stop and print a + /// message instead of exiting. + CheckStop(bool), /// Execution terminated because the machine reached the end of the input. Eof, /// Execution terminated because the machine was asked to terminate with `END`. - Exited(u8), + /// + /// The boolean indicates whether this stop is final. If false, the stop request comes from + /// cancelling the execution of a nested `RUN` so the caller may want to capture the stop and + /// print a message instead of exiting. + Exited(u8, bool), /// Execution terminated because the bytecode requires the caller to issue a builtin function /// or command call. @@ -674,7 +682,7 @@ impl Machine { } /// Returns true if execution should stop because we have hit a stop condition. - async fn should_stop(&mut self) -> bool { + pub async fn should_stop(&mut self) -> bool { if let Some(yield_now) = self.yield_now_fn.as_ref() { (yield_now)().await; } @@ -779,7 +787,7 @@ impl Machine { } else { 0 }; - Ok(InternalStopReason::Exited(code)) + Ok(InternalStopReason::Exited(code, context.run_prev.is_none())) } /// Handles a unary logical operator that cannot fail. @@ -1359,7 +1367,7 @@ impl Machine { let old_pc = context.pc; context.pc = span.addr; if span.addr <= old_pc { - return Ok(InternalStopReason::CheckStop); + return Ok(InternalStopReason::CheckStop(context.run_prev.is_none())); } } @@ -1368,7 +1376,7 @@ impl Machine { let old_pc = context.pc; context.pc = span.addr; if span.addr <= old_pc { - return Ok(InternalStopReason::CheckStop); + return Ok(InternalStopReason::CheckStop(context.run_prev.is_none())); } } else { context.pc += 1; @@ -1381,7 +1389,7 @@ impl Machine { let old_pc = context.pc; context.pc = *addr; if *addr <= old_pc { - return Ok(InternalStopReason::CheckStop); + return Ok(InternalStopReason::CheckStop(context.run_prev.is_none())); } } else { context.pc += 1; @@ -1396,7 +1404,7 @@ impl Machine { let old_pc = context.pc; context.pc = *addr; if *addr <= old_pc { - return Ok(InternalStopReason::CheckStop); + return Ok(InternalStopReason::CheckStop(context.run_prev.is_none())); } } } @@ -1486,7 +1494,7 @@ impl Machine { Instruction::Return(pos) => match context.addr_stack.pop() { Some(addr) => { context.pc = addr; - return Ok(InternalStopReason::CheckStop); + return Ok(InternalStopReason::CheckStop(context.run_prev.is_none())); } None => return new_syntax_error(*pos, "No address to return to".to_owned()), }, @@ -1509,7 +1517,11 @@ impl Machine { } /// Handles an upcall to the builtin callable described in `upcall`. - async fn handle_upcall(&mut self, context: &mut Context, upcall: &UpcallData) -> Result<()> { + pub async fn handle_upcall( + &mut self, + context: &mut Context, + upcall: &UpcallData, + ) -> Result<()> { let result; if let Some(return_type) = upcall.return_type { result = self @@ -1583,37 +1595,27 @@ impl Machine { Ok(Context::from(image)) } - /// Executes the instructions in `context`. - pub async fn exec(&mut self, context: &mut Context) -> Result { + /// Executes the instructions in `context` until the bytecode performs an explicit exit or until + /// the bytecode requests the caller to perform a service on its behalf (such as an interrupt + /// check or a call to a built-in). + pub async fn resume(&mut self, context: &mut Context) -> Result { + let mut stop_reason = InternalStopReason::Eof; loop { while context.pc < context.instrs.len() { match self.exec_until_stop(context) { - Ok(InternalStopReason::CheckStop) => { - if self.should_stop().await { - return Ok(StopReason::Break(context.run_prev.is_none())); - } - } - - Ok(InternalStopReason::Upcall(data)) => { - self.handle_upcall(context, &data).await?; - } - - Ok(InternalStopReason::Eof) => { - if context.run_prev.is_some() { - break; - } - - return Ok(StopReason::Eof); - } - - Ok(InternalStopReason::Exited(code)) => { - if context.run_prev.is_some() { - break; - } - - return Ok(StopReason::Exited(code, context.run_prev.is_none())); + // Controlled stops asking for service do not abort chained execution because we + // expect execution to resume. + Ok(sr @ InternalStopReason::CheckStop(..)) => return Ok(sr), + Ok(sr @ InternalStopReason::Upcall(..)) => return Ok(sr), + + // Explicit exits from the execution loop abort the current context but we need + // to resume chained contexts if necessary. + Ok(sr @ InternalStopReason::Eof) | Ok(sr @ InternalStopReason::Exited(..)) => { + stop_reason = sr; + break; } + // Errors abort chained execution early. Err(e) => self.handle_error(context, e)?, } } @@ -1624,7 +1626,32 @@ impl Machine { break; } } - Ok(StopReason::Eof) + Ok(stop_reason) + } + + /// Executes the instructions in `context` through completion. + pub async fn exec(&mut self, context: &mut Context) -> Result { + loop { + match self.resume(context).await? { + InternalStopReason::CheckStop(is_final) => { + if self.should_stop().await { + return Ok(StopReason::Break(is_final)); + } + } + + InternalStopReason::Upcall(data) => { + self.handle_upcall(context, &data).await?; + } + + InternalStopReason::Eof => { + return Ok(StopReason::Eof); + } + + InternalStopReason::Exited(code, is_final) => { + return Ok(StopReason::Exited(code, is_final)); + } + } + } } /// Executes a program extracted from the `input` readable. diff --git a/repl/src/lib.rs b/repl/src/lib.rs index 6bb17f7c..34efde77 100644 --- a/repl/src/lib.rs +++ b/repl/src/lib.rs @@ -22,7 +22,7 @@ #![warn(unused, unused_extern_crates, unused_import_braces, unused_qualifications)] #![warn(unsafe_code)] -use endbasic_core::exec::{Machine, StopReason}; +use endbasic_core::exec::{InternalStopReason, Machine, StopReason}; use endbasic_std::console::{self, is_narrow, refill_and_print, Console}; use endbasic_std::program::{continue_if_modified, Program}; use endbasic_std::storage::Storage; @@ -195,19 +195,45 @@ pub async fn run_repl_loop( match line { Ok(line) => { - match machine.compile(&mut line.as_bytes()) { - Ok(mut context) => match machine.exec(&mut context).await { - Ok(reason) => stop_reason = reason, - Err(e) => { - let mut console = console.borrow_mut(); - console.print(format!("ERROR: {}", e).as_str())?; - } - }, + let mut context = match machine.compile(&mut line.as_bytes()) { + Ok(context) => context, Err(e) => { let mut console = console.borrow_mut(); console.print(format!("ERROR: {}", e).as_str())?; + continue; } }; + + loop { + match machine.resume(&mut context).await { + Ok(InternalStopReason::CheckStop(is_final)) => { + if machine.should_stop().await { + stop_reason = StopReason::Break(is_final); + break; + } + } + Ok(InternalStopReason::Upcall(data)) => { + if let Err(e) = machine.handle_upcall(&mut context, &data).await { + let mut console = console.borrow_mut(); + console.print(format!("ERROR: {}", e).as_str())?; + break; + } + } + Ok(InternalStopReason::Eof) => { + stop_reason = StopReason::Eof; + break; + } + Ok(InternalStopReason::Exited(code, is_final)) => { + stop_reason = StopReason::Exited(code, is_final); + break; + } + Err(e) => { + let mut console = console.borrow_mut(); + console.print(format!("ERROR: {}", e).as_str())?; + break; + } + } + } } Err(e) => { if e.kind() == io::ErrorKind::Interrupted { From 29ee707ec7cca37653026bcb85024d49f9b480e1 Mon Sep 17 00:00:00 2001 From: Julio Merino Date: Fri, 20 Dec 2024 17:52:29 -0800 Subject: [PATCH 9/9] i don't know what this is --- cli/Cargo.toml | 1 - cli/src/main.rs | 45 +++++++------------- core/Cargo.toml | 1 - core/examples/config.rs | 1 - core/examples/dsl.rs | 1 - core/src/exec.rs | 93 ++++------------------------------------- repl/src/lib.rs | 43 +++++++++++++++---- sdl/Cargo.toml | 1 - sdl/src/console.rs | 45 ++++++++++---------- sdl/src/host.rs | 11 ----- sdl/src/lib.rs | 10 ++--- std/Cargo.toml | 1 - std/src/console/mod.rs | 12 ++++++ std/src/lib.rs | 25 +---------- terminal/src/lib.rs | 79 ++++++++++++++++------------------ 15 files changed, 134 insertions(+), 235 deletions(-) diff --git a/cli/Cargo.toml b/cli/Cargo.toml index e2cdc022..79e3bd76 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -19,7 +19,6 @@ rpi = ["endbasic-rpi"] [dependencies] anyhow = "1.0" -async-channel = "2.2" dirs = "5.0" getopts = "0.2" thiserror = "1.0" diff --git a/cli/src/main.rs b/cli/src/main.rs index 62a7f546..09f29433 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -23,8 +23,6 @@ #![warn(unsafe_code)] use anyhow::{anyhow, Result}; -use async_channel::Sender; -use endbasic_core::exec::Signal; use endbasic_std::console::Console; use endbasic_std::storage::Storage; use getopts::Options; @@ -107,10 +105,8 @@ fn new_machine_builder(console_spec: Option<&str>) -> io::Result) -> Result { } /// Sets up the console. -fn setup_console( - console_spec: Option<&str>, - signals_tx: Sender, -) -> io::Result>> { +fn setup_console(console_spec: Option<&str>) -> io::Result>> { /// Creates the textual console when crossterm support is built in. #[cfg(feature = "crossterm")] - fn setup_text_console(signals_tx: Sender) -> io::Result>> { - Ok(Rc::from(RefCell::from(endbasic_terminal::TerminalConsole::from_stdio(signals_tx)?))) + fn setup_text_console() -> io::Result>> { + Ok(Rc::from(RefCell::from(endbasic_terminal::TerminalConsole::from_stdio()?))) } /// Creates the textual console with very basic features when crossterm support is not built in. #[cfg(not(feature = "crossterm"))] - fn setup_text_console(_signals_tx: Sender) -> io::Result>> { + fn setup_text_console() -> io::Result>> { Ok(Rc::from(RefCell::from(endbasic_std::console::TrivialConsole::default()))) } /// Creates the graphical console when SDL support is built in. #[cfg(feature = "sdl")] - pub fn setup_graphics_console( - signals_tx: Sender, - spec: &str, - ) -> io::Result>> { - endbasic_sdl::setup(spec, signals_tx) + pub fn setup_graphics_console(spec: &str) -> io::Result>> { + endbasic_sdl::setup(spec) } /// Errors out during the creation of the graphical console when SDL support is not compiled in. #[cfg(not(feature = "sdl"))] - pub fn setup_graphics_console( - _signals_tx: Sender, - _spec: &str, - ) -> io::Result>> { + pub fn setup_graphics_console(_spec: &str) -> io::Result>> { // TODO(jmmv): Make this io::ErrorKind::Unsupported when our MSRV allows it. Err(io::Error::new(io::ErrorKind::InvalidInput, "SDL support not compiled in")) } #[cfg(feature = "rpi")] - fn setup_st7735s_console(signals_tx: Sender) -> io::Result>> { - Ok(Rc::from(RefCell::from(endbasic_rpi::new_st7735s_console(signals_tx)?))) + fn setup_st7735s_console() -> io::Result>> { + Ok(Rc::from(RefCell::from(endbasic_rpi::new_st7735s_console()?))) } #[cfg(not(feature = "rpi"))] - pub fn setup_st7735s_console( - _signals_tx: Sender, - ) -> io::Result>> { + pub fn setup_st7735s_console() -> io::Result>> { Err(io::Error::new(io::ErrorKind::InvalidInput, "ST7735S support not compiled in")) } let console: Rc> = match console_spec { - None | Some("text") => setup_text_console(signals_tx)?, + None | Some("text") => setup_text_console()?, - Some("graphics") => setup_graphics_console(signals_tx, "")?, - Some("st7735s") => setup_st7735s_console(signals_tx)?, + Some("graphics") => setup_graphics_console("")?, + Some("st7735s") => setup_st7735s_console()?, Some(text) if text.starts_with("graphics:") => { - setup_graphics_console(signals_tx, &text["graphics:".len()..])? + setup_graphics_console(&text["graphics:".len()..])? } Some(text) => { diff --git a/core/Cargo.toml b/core/Cargo.toml index 28ec89ad..4a796962 100644 --- a/core/Cargo.toml +++ b/core/Cargo.toml @@ -12,7 +12,6 @@ readme = "README.md" edition = "2018" [dependencies] -async-channel = "2.2" async-trait = "0.1" thiserror = "1.0" diff --git a/core/examples/config.rs b/core/examples/config.rs index 9e25e18b..5bc03b8e 100644 --- a/core/examples/config.rs +++ b/core/examples/config.rs @@ -47,7 +47,6 @@ fn main() { StopReason::Exited(i, _is_final) => { println!("Script explicitly exited with code {}", i) } - StopReason::Break(_is_final) => (), // Ignore signals. } } diff --git a/core/examples/dsl.rs b/core/examples/dsl.rs index 7e2709ba..9b2aeb0b 100644 --- a/core/examples/dsl.rs +++ b/core/examples/dsl.rs @@ -157,7 +157,6 @@ fn main() { StopReason::Exited(i, _is_final) => { println!("Script explicitly exited with code {}", i) } - StopReason::Break(_is_final) => (), // Ignore signals. } } diff --git a/core/src/exec.rs b/core/src/exec.rs index bc6d48a4..64686b82 100644 --- a/core/src/exec.rs +++ b/core/src/exec.rs @@ -22,10 +22,7 @@ use crate::reader::LineCol; use crate::syms::{Callable, Symbol, SymbolKey, Symbols}; use crate::value; use crate::value::double_to_integer; -use async_channel::{Receiver, Sender, TryRecvError}; -use std::future::Future; use std::io; -use std::pin::Pin; use std::rc::Rc; /// Execution errors. @@ -78,13 +75,6 @@ fn new_syntax_error>(pos: LineCol, message: S) -> Result { Err(Error::SyntaxError(pos, message.into())) } -/// Signals that can be delivered to the machine. -#[derive(Clone, Debug, Eq, PartialEq)] -pub enum Signal { - /// Asks the machine to stop execution of the currently-running program. - Break, -} - /// Request to exit the VM execution loop to execute a native command or function. #[derive(Clone, Debug, Eq, PartialEq)] pub struct UpcallData { @@ -148,13 +138,6 @@ pub enum StopReason { /// nested invocation triggered by `RUN` so the caller may want to capture the stop and print a /// message instead of exiting. Exited(u8, bool), - - /// Execution terminated because the machine received a break signal. - /// - /// The boolean indicates whether this stop is final. If false, the stop request comes from - /// cancelling the execution of a nested `RUN` so the caller may want to capture the stop and - /// print a message instead of exiting. - Break(bool), } impl StopReason { @@ -163,14 +146,6 @@ impl StopReason { match self { StopReason::Eof => 0, StopReason::Exited(i, _is_final) => *i as i32, - StopReason::Break(_is_final) => { - // This mimics the behavior of typical Unix shells, which translate a signal to a - // numerical exit code, but this is not accurate. First, because a CTRL+C sequence - // should be exposed as a SIGINT signal to whichever process is waiting for us, and - // second because this is not meaningful on Windows. But for now this will do. - const SIGINT: i32 = 2; - 128 + SIGINT - } } } } @@ -182,9 +157,6 @@ pub trait Clearable { fn reset_state(&self, syms: &mut Symbols); } -/// Type of the function used by the execution loop to yield execution. -pub type YieldNowFn = Box Pin + 'static>>>; - /// Tags used in the value stack to identify the type of their corresponding value. pub enum ValueTag { /// Represents that there is no next value to consume. Can only appear for command invocations. @@ -610,39 +582,13 @@ impl From for Context { } /// Executes an EndBASIC program and tracks its state. +#[derive(Default)] pub struct Machine { symbols: Symbols, clearables: Vec>, - yield_now_fn: Option, - signals_chan: (Sender, Receiver), -} - -impl Default for Machine { - fn default() -> Self { - Self::with_signals_chan_and_yield_now_fn(async_channel::unbounded(), None) - } } impl Machine { - /// Constructs a new empty machine with the given signals communication channel. - pub fn with_signals_chan(signals: (Sender, Receiver)) -> Self { - Self::with_signals_chan_and_yield_now_fn(signals, None) - } - - /// Constructs a new empty machine with the given signals communication channel and yielding - /// function. - pub fn with_signals_chan_and_yield_now_fn( - signals: (Sender, Receiver), - yield_now_fn: Option, - ) -> Self { - Self { - symbols: Symbols::default(), - clearables: vec![], - yield_now_fn, - signals_chan: signals, - } - } - /// Registers the given clearable. /// /// In the common case, functions and commands hold a reference to the out-of-machine state @@ -658,11 +604,6 @@ impl Machine { self.symbols.add_callable(callable) } - /// Obtains a channel via which to send signals to the machine during execution. - pub fn get_signals_tx(&self) -> Sender { - self.signals_chan.0.clone() - } - /// Resets the state of the machine by clearing all variable. pub fn clear(&mut self) { for clearable in self.clearables.as_slice() { @@ -681,19 +622,6 @@ impl Machine { &mut self.symbols } - /// Returns true if execution should stop because we have hit a stop condition. - pub async fn should_stop(&mut self) -> bool { - if let Some(yield_now) = self.yield_now_fn.as_ref() { - (yield_now)().await; - } - - match self.signals_chan.1.try_recv() { - Ok(Signal::Break) => true, - Err(TryRecvError::Empty) => false, - Err(TryRecvError::Closed) => panic!("Channel unexpectedly closed"), - } - } - /// Handles an array assignment. fn assign_array( &mut self, @@ -760,13 +688,6 @@ impl Machine { Ok(()) } - /// Consumes any pending signals so that they don't interfere with an upcoming execution. - pub fn drain_signals(&mut self) { - while self.signals_chan.1.try_recv().is_ok() { - // Do nothing. - } - } - /// Tells the machine to stop execution at the next statement boundary. fn end(&mut self, context: &mut Context, has_code: bool) -> Result { let code = if has_code { @@ -1630,14 +1551,13 @@ impl Machine { } /// Executes the instructions in `context` through completion. + /// + /// This execution loop is _not_ interruptible. If the caller wants to respect stop signals, + /// the caller must use `resume()` instead. pub async fn exec(&mut self, context: &mut Context) -> Result { loop { match self.resume(context).await? { - InternalStopReason::CheckStop(is_final) => { - if self.should_stop().await { - return Ok(StopReason::Break(is_final)); - } - } + InternalStopReason::CheckStop(_is_final) => (), InternalStopReason::Upcall(data) => { self.handle_upcall(context, &data).await?; @@ -2317,6 +2237,8 @@ mod tests { assert_eq!(&["2"], captured_out.borrow().as_slice()); } + /* DO NOT SUBMIT + #[tokio::test] async fn test_signals_stop() { let mut machine = Machine::default(); @@ -2421,6 +2343,7 @@ mod tests { do_check_stop_test("WHILE TRUE: WEND").await; do_check_stop_test("WHILE TRUE: a = 1: WEND").await; } + */ #[test] fn test_do_infinite_ok() { diff --git a/repl/src/lib.rs b/repl/src/lib.rs index 34efde77..760708df 100644 --- a/repl/src/lib.rs +++ b/repl/src/lib.rs @@ -36,6 +36,31 @@ pub mod editor; /// Message to print on the console when receiving a break signal. const BREAK_MSG: &str = "**** BREAK ****"; +/* +/// Type of the function used by the execution loop to yield execution. +pub type YieldNowFn = Box Pin + 'static>>>; + +/// Consumes any pending signals so that they don't interfere with an upcoming execution. +pub fn drain_signals(&mut self) { + while self.signals_chan.1.try_recv().is_ok() { + // Do nothing. + } +} + +/// Returns true if execution should stop because we have hit a stop condition. +pub async fn should_stop(&mut self) -> bool { + if let Some(yield_now) = self.yield_now_fn.as_ref() { + (yield_now)().await; + } + + match self.signals_chan.1.try_recv() { + Ok(Signal::Break) => true, + Err(TryRecvError::Empty) => false, + Err(TryRecvError::Closed) => panic!("Channel unexpectedly closed"), + } +} +*/ + /// Prints the EndBASIC welcome message to the given console. pub fn print_welcome(console: Rc>) -> io::Result<()> { let mut console = console.borrow_mut(); @@ -140,10 +165,12 @@ pub async fn run_from_cloud( console.print(&format!("**** Program exited with code {} ****", code))?; code } + /* DO NOT SUBMIT Ok(r @ StopReason::Break(_is_final)) => { console.print("**** Program stopped due to BREAK ****")?; r.as_exit_code() } + */ Err(e) => { console.print(&format!("**** ERROR: {} ****", e))?; 1 @@ -191,7 +218,9 @@ pub async fn run_repl_loop( // Any signals entered during console input should not impact upcoming execution. Drain // them all. - machine.drain_signals(); + // DO NOT SUBMIT + //machine.drain_signals(); + let _ = console.borrow_mut().take_signal().await; match line { Ok(line) => { @@ -206,9 +235,11 @@ pub async fn run_repl_loop( loop { match machine.resume(&mut context).await { - Ok(InternalStopReason::CheckStop(is_final)) => { - if machine.should_stop().await { - stop_reason = StopReason::Break(is_final); + Ok(InternalStopReason::CheckStop(_is_final)) => { + let mut console = console.borrow_mut(); + if console.take_signal().await.is_some() { + console.print("**** BREAK ****")?; + stop_reason = StopReason::Eof; break; } } @@ -254,10 +285,6 @@ pub async fn run_repl_loop( match stop_reason { StopReason::Eof => (), - StopReason::Break(_is_final) => { - console.borrow_mut().print("**** BREAK ****")?; - stop_reason = StopReason::Eof; - } StopReason::Exited(_, false) => { console .borrow_mut() diff --git a/sdl/Cargo.toml b/sdl/Cargo.toml index 07f2b981..d08646b0 100644 --- a/sdl/Cargo.toml +++ b/sdl/Cargo.toml @@ -12,7 +12,6 @@ readme = "README.md" edition = "2018" [dependencies] -async-channel = "2.2" async-trait = "0.1" once_cell = "1.8" tempfile = "3" diff --git a/sdl/src/console.rs b/sdl/src/console.rs index c42d0c74..3d274036 100644 --- a/sdl/src/console.rs +++ b/sdl/src/console.rs @@ -17,11 +17,9 @@ use crate::host::{self, Request, Response}; use crate::spec::Resolution; -use async_channel::Sender; use async_trait::async_trait; -use endbasic_core::exec::Signal; use endbasic_std::console::{ - remove_control_chars, CharsXY, ClearType, Console, Key, PixelsXY, SizeInPixels, + remove_control_chars, CharsXY, ClearType, Console, Key, PixelsXY, Signal, SizeInPixels, }; use std::io; use std::path::PathBuf; @@ -39,6 +37,7 @@ pub(crate) struct SdlConsole { on_key_rx: Receiver, fg_color: Option, bg_color: Option, + pending_signal: Option, } impl SdlConsole { @@ -53,21 +52,12 @@ impl SdlConsole { resolution: Resolution, font_path: PathBuf, font_size: u16, - signals_tx: Sender, ) -> io::Result { let (request_tx, request_rx) = mpsc::sync_channel(1); let (response_tx, response_rx) = mpsc::sync_channel(1); let (on_key_tx, on_key_rx) = mpsc::channel(); let handle = thread::spawn(move || { - host::run( - resolution, - font_path, - font_size, - request_rx, - response_tx, - on_key_tx, - signals_tx, - ); + host::run(resolution, font_path, font_size, request_rx, response_tx, on_key_tx); }); // Wait for the console to be up and running. We must do this for error propagation but @@ -80,6 +70,7 @@ impl SdlConsole { on_key_rx, fg_color: None, bg_color: None, + pending_signal: None, }), Response::Empty(Err(e)) => Err(e), r => panic!("Unexpected response {:?}", r), @@ -155,16 +146,32 @@ impl Console for SdlConsole { self.call(Request::Print(text)) } + async fn take_signal(&mut self) -> Option { + if self.pending_signal.is_none() { + let _ = self.poll_key().await; + } + self.pending_signal.take() + } + async fn poll_key(&mut self) -> io::Result> { match self.on_key_rx.try_recv() { - Ok(k) => Ok(Some(k)), + Ok(k) => { + if k == Key::Interrupt { + self.pending_signal = Some(Signal::Break); + } + Ok(Some(k)) + } Err(TryRecvError::Empty) => Ok(None), Err(TryRecvError::Disconnected) => panic!("Channel must be alive"), } } async fn read_key(&mut self) -> io::Result { - Ok(self.on_key_rx.recv().expect("Channel must be alive")) + let key = self.on_key_rx.recv().expect("Channel must be alive"); + if key == Key::Interrupt { + self.pending_signal = Some(Signal::Break); + } + Ok(key) } fn show_cursor(&mut self) -> io::Result<()> { @@ -232,7 +239,6 @@ impl Console for SdlConsole { #[cfg(test)] mod testutils { use super::*; - use async_channel::{Receiver, TryRecvError}; use flate2::read::GzDecoder; use flate2::write::GzEncoder; use flate2::Compression; @@ -281,9 +287,6 @@ mod testutils { /// The SDL console under test. console: SdlConsole, - /// Channel via which we receive signals from the console. - signals_rx: Receiver, - /// Guard to ensure there is a single `SdlConsole` alive at any given time. This must come /// after `console` because the Rust drop rules dictate that struct elements are dropped in /// the order in which they are defined. @@ -294,15 +297,13 @@ mod testutils { /// Creates a new test context and ensures no other test is running at the same time. pub(crate) fn new() -> Self { let lock = TEST_LOCK.lock().unwrap(); - let signals_chan = async_channel::unbounded(); let console = SdlConsole::new( Resolution::windowed(800, 600).unwrap(), src_path("sdl/src/IBMPlexMono-Regular-6.0.0.ttf"), 16, - signals_chan.0, ) .unwrap(); - Self { _lock: lock, signals_rx: signals_chan.1, console } + Self { _lock: lock, console } } /// Obtains access to the SDL console. diff --git a/sdl/src/host.rs b/sdl/src/host.rs index 2615ff20..bbd9b705 100644 --- a/sdl/src/host.rs +++ b/sdl/src/host.rs @@ -22,7 +22,6 @@ use crate::font::{font_error_to_io_error, MonospacedFont}; use crate::spec::Resolution; use crate::string_error_to_io_error; use async_trait::async_trait; -use endbasic_core::exec::Signal; use endbasic_std::console::drawing::{draw_circle, draw_circle_filled}; use endbasic_std::console::graphics::{ClampedInto, ClampedMul, InputOps, RasterInfo, RasterOps}; use endbasic_std::console::{ @@ -599,7 +598,6 @@ pub(crate) fn run( request_rx: Receiver, response_tx: SyncSender, on_key_tx: Sender, - signals_tx: async_channel::Sender, ) { let ctx = match Context::new(resolution, font_path, font_size) { Ok(ctx) => ctx, @@ -678,15 +676,6 @@ pub(crate) fn run( if let Some(event) = ctx.poll_event() { if let Some(key) = parse_event(event) { - if key == Key::Interrupt { - // signals_tx is an async channel because that's what the execution engine - // needs. This means that we cannot use a regular "send" here because we - // would need to await for it, which is a no-no because we are not in an - // async context. Using "try_send" should be sufficient though given that - // the channel we use is not bounded. - signals_tx.try_send(Signal::Break).expect("Channel must be alive and not full") - } - on_key_tx.send(key).expect("Channel must be alive"); } diff --git a/sdl/src/lib.rs b/sdl/src/lib.rs index 343accce..6f47f4de 100644 --- a/sdl/src/lib.rs +++ b/sdl/src/lib.rs @@ -22,8 +22,6 @@ #![warn(unused, unused_extern_crates, unused_import_braces, unused_qualifications)] #![warn(unsafe_code)] -use async_channel::Sender; -use endbasic_core::exec::Signal; use endbasic_std::console::Console; use std::cell::RefCell; use std::io; @@ -40,18 +38,16 @@ fn string_error_to_io_error(e: String) -> io::Error { } /// Creates the graphical console based on the given `spec`. -pub fn setup(spec: &str, signals_tx: Sender) -> io::Result>> { +pub fn setup(spec: &str) -> io::Result>> { let spec = spec::parse_graphics_spec(spec)?; let console = match spec.1 { None => { let default_font = spec::TempFont::default_font()?; - console::SdlConsole::new(spec.0, default_font.path(), spec.2, signals_tx)? + console::SdlConsole::new(spec.0, default_font.path(), spec.2)? // The console has been created at this point, so it should be safe to drop // default_font and clean up the on-disk file backing it up. } - Some(font_path) => { - console::SdlConsole::new(spec.0, font_path.to_owned(), spec.2, signals_tx)? - } + Some(font_path) => console::SdlConsole::new(spec.0, font_path.to_owned(), spec.2)?, }; Ok(Rc::from(RefCell::from(console))) } diff --git a/std/Cargo.toml b/std/Cargo.toml index b82dc2ee..c004afd6 100644 --- a/std/Cargo.toml +++ b/std/Cargo.toml @@ -12,7 +12,6 @@ readme = "README.md" edition = "2018" [dependencies] -async-channel = "2.2" async-trait = "0.1" futures-lite = "2.2" radix_trie = "0.2" diff --git a/std/src/console/mod.rs b/std/src/console/mod.rs index 24057d7a..9ab56360 100644 --- a/std/src/console/mod.rs +++ b/std/src/console/mod.rs @@ -44,6 +44,13 @@ pub use trivial::TrivialConsole; mod linebuffer; pub use linebuffer::LineBuffer; +/// Signals that can be delivered to the machine from the console. +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum Signal { + /// Asks the machine to stop execution of the currently-running program. + Break, +} + /// Decoded key presses as returned by the console. #[derive(Clone, Copy, Debug, Eq, PartialEq)] pub enum Key { @@ -230,6 +237,11 @@ pub trait Console { // TODO(jmmv): Remove this in favor of write? fn print(&mut self, text: &str) -> io::Result<()>; + /// DO NOT SUBMIT + async fn take_signal(&mut self) -> Option { + None + } + /// Returns the next key press if any is available. async fn poll_key(&mut self) -> io::Result>; diff --git a/std/src/lib.rs b/std/src/lib.rs index 77199f1f..4c0112aa 100644 --- a/std/src/lib.rs +++ b/std/src/lib.rs @@ -22,8 +22,7 @@ #![warn(unused, unused_extern_crates, unused_import_braces, unused_qualifications)] #![warn(unsafe_code)] -use async_channel::{Receiver, Sender}; -use endbasic_core::exec::{Machine, Result, Signal, YieldNowFn}; +use endbasic_core::exec::{Machine, Result}; use std::cell::RefCell; use std::rc::Rc; @@ -49,8 +48,6 @@ pub struct MachineBuilder { console: Option>>, gpio_pins: Option>>, sleep_fn: Option, - yield_now_fn: Option, - signals_chan: Option<(Sender, Receiver)>, } impl MachineBuilder { @@ -72,18 +69,6 @@ impl MachineBuilder { self } - /// Overrides the default yielding function with the given one. - pub fn with_yield_now_fn(mut self, yield_now_fn: YieldNowFn) -> Self { - self.yield_now_fn = Some(yield_now_fn); - self - } - - /// Overrides the default signals channel with the given one. - pub fn with_signals_chan(mut self, chan: (Sender, Receiver)) -> Self { - self.signals_chan = Some(chan); - self - } - /// Lazily initializes the `console` field with a default value and returns it. pub fn get_console(&mut self) -> Rc> { if self.console.is_none() { @@ -105,13 +90,7 @@ impl MachineBuilder { let console = self.get_console(); let gpio_pins = self.get_gpio_pins(); - let signals_chan = match self.signals_chan { - Some(pair) => pair, - None => async_channel::unbounded(), - }; - - let mut machine = - Machine::with_signals_chan_and_yield_now_fn(signals_chan, self.yield_now_fn); + let mut machine = Machine::default(); arrays::add_all(&mut machine); console::add_all(&mut machine, console.clone()); data::add_all(&mut machine); diff --git a/terminal/src/lib.rs b/terminal/src/lib.rs index 1b1db836..cc02a882 100644 --- a/terminal/src/lib.rs +++ b/terminal/src/lib.rs @@ -27,9 +27,9 @@ use async_trait::async_trait; use crossterm::event::{self, KeyEventKind}; use crossterm::tty::IsTty; use crossterm::{cursor, style, terminal, QueueableCommand}; -use endbasic_core::exec::Signal; use endbasic_std::console::{ - get_env_var_as_u16, read_key_from_stdin, remove_control_chars, CharsXY, ClearType, Console, Key, + get_env_var_as_u16, read_key_from_stdin, remove_control_chars, CharsXY, ClearType, Console, + Key, Signal, }; use std::cmp::Ordering; use std::collections::VecDeque; @@ -58,6 +58,8 @@ pub struct TerminalConsole { /// Channel to receive key presses from the terminal. on_key_rx: Receiver, + + pending_signal: Option, } impl Drop for TerminalConsole { @@ -73,47 +75,33 @@ impl TerminalConsole { /// /// This spawns a background task to handle console input so this must be run in the context of /// an Tokio runtime. - pub fn from_stdio(signals_tx: Sender) -> io::Result { - let (terminal, _on_key_tx) = Self::from_stdio_with_injector(signals_tx)?; - Ok(terminal) - } - - /// Creates a new console based on the properties of stdin/stdout. - /// - /// This spawns a background task to handle console input so this must be run in the context of - /// an Tokio runtime. - /// - /// Compared to `from_stdio`, this also returns a key sender to inject extra events into the - /// queue maintained by the terminal. - pub fn from_stdio_with_injector(signals_tx: Sender) -> io::Result<(Self, Sender)> { + pub fn from_stdio() -> io::Result { let (on_key_tx, on_key_rx) = async_channel::unbounded(); let is_tty = io::stdin().is_tty() && io::stdout().is_tty(); if is_tty { terminal::enable_raw_mode()?; - tokio::task::spawn(TerminalConsole::raw_key_handler(on_key_tx.clone(), signals_tx)); + tokio::task::spawn(TerminalConsole::raw_key_handler(on_key_tx.clone())); } else { tokio::task::spawn(TerminalConsole::stdio_key_handler(on_key_tx.clone())); } - Ok(( - Self { - is_tty, - fg_color: None, - bg_color: None, - cursor_visible: true, - alt_active: false, - sync_enabled: true, - on_key_rx, - }, - on_key_tx, - )) + Ok(Self { + is_tty, + fg_color: None, + bg_color: None, + cursor_visible: true, + alt_active: false, + sync_enabled: true, + on_key_rx, + pending_signal: None, + }) } /// Async task to wait for key events on a raw terminal and translate them into events for the /// console or the machine. - async fn raw_key_handler(on_key_tx: Sender, signals_tx: Sender) { + async fn raw_key_handler(on_key_tx: Sender) { use event::{KeyCode, KeyModifiers}; let mut done = false; @@ -170,17 +158,6 @@ impl TerminalConsole { }; done = key == Key::Eof; - if key == Key::Interrupt { - // Handling CTRL+C in this way isn't great because this is not the same as handling - // SIGINT on Unix builds. First, we are unable to stop long-running operations like - // sleeps; and second, a real SIGINT will kill the interpreter completely instead of - // coming this way. We need a real signal handler and we probably should not be - // running in raw mode all the time. - signals_tx - .send(Signal::Break) - .await - .expect("Send to unbounded channel should not have failed") - } // This should never fail but can if the receiver outruns the console because we // don't await for the handler to terminate (which we cannot do safely because @@ -188,7 +165,6 @@ impl TerminalConsole { let _ = on_key_tx.send(key).await; } - signals_tx.close(); on_key_tx.close(); } @@ -366,9 +342,21 @@ impl Console for TerminalConsole { Ok(()) } + async fn take_signal(&mut self) -> Option { + if self.pending_signal.is_none() { + let _ = self.poll_key().await; + } + self.pending_signal.take() + } + async fn poll_key(&mut self) -> io::Result> { match self.on_key_rx.try_recv() { - Ok(k) => Ok(Some(k)), + Ok(k) => { + if k == Key::Interrupt { + self.pending_signal = Some(Signal::Break); + } + Ok(Some(k)) + } Err(TryRecvError::Empty) => Ok(None), Err(TryRecvError::Closed) => Ok(Some(Key::Eof)), } @@ -376,7 +364,12 @@ impl Console for TerminalConsole { async fn read_key(&mut self) -> io::Result { match self.on_key_rx.recv().await { - Ok(k) => Ok(k), + Ok(k) => { + if k == Key::Interrupt { + self.pending_signal = Some(Signal::Break); + } + Ok(k) + } Err(_) => Ok(Key::Eof), } }