From a32e90a2210e845376bab09ecdd004e4c4fb0a0c Mon Sep 17 00:00:00 2001 From: John-John Tedro Date: Sat, 27 Jul 2024 11:05:05 +0200 Subject: [PATCH] vm: Change operations to free temporaries --- crates/rune/src/compile/v1/assemble.rs | 162 ++++++++++++++++--------- crates/rune/src/compile/v1/linear.rs | 9 ++ crates/rune/src/compile/v1/needs.rs | 16 ++- crates/rune/src/compile/v1/scopes.rs | 5 +- crates/rune/src/function/mod.rs | 34 +++--- crates/rune/src/runtime/inst.rs | 141 ++++++++++----------- crates/rune/src/runtime/stack.rs | 66 ++++++++-- crates/rune/src/runtime/value.rs | 2 +- crates/rune/src/runtime/vm.rs | 114 ++++++++++------- crates/rune/src/shared/mod.rs | 7 +- 10 files changed, 341 insertions(+), 215 deletions(-) diff --git a/crates/rune/src/compile/v1/assemble.rs b/crates/rune/src/compile/v1/assemble.rs index 8730b4f8d..d50222dd8 100644 --- a/crates/rune/src/compile/v1/assemble.rs +++ b/crates/rune/src/compile/v1/assemble.rs @@ -1170,7 +1170,7 @@ fn const_<'a, 'hir>( span, )?; - linear.free()?; + linear.free_non_dangling()?; } ConstValue::Tuple(ref tuple) => { let mut linear = cx.scopes.linear(span, tuple.len())?; @@ -1188,7 +1188,7 @@ fn const_<'a, 'hir>( span, )?; - linear.free()?; + linear.free_non_dangling()?; } ConstValue::Object(ref object) => { let mut linear = cx.scopes.linear(span, object.len())?; @@ -1213,7 +1213,7 @@ fn const_<'a, 'hir>( span, )?; - linear.free()?; + linear.free_non_dangling()?; } } @@ -1644,7 +1644,7 @@ fn expr_async_block<'a, 'hir>( &"async block", )?; - linear.free()?; + linear.free_non_dangling()?; Ok(Asm::new(span, ())) } @@ -1744,7 +1744,7 @@ fn expr_call<'a, 'hir>( span, )?; - linear.free()?; + linear.free_non_dangling()?; } hir::Call::Associated { target, hash } => { let linear = converge!(exprs_2(cx, span, slice::from_ref(target), hir.args)?); @@ -1759,7 +1759,7 @@ fn expr_call<'a, 'hir>( span, )?; - linear.free()?; + linear.free_non_dangling()?; } hir::Call::Meta { hash } => { let linear = converge!(exprs(cx, span, hir.args)?); @@ -1774,7 +1774,7 @@ fn expr_call<'a, 'hir>( span, )?; - linear.free()?; + linear.free_non_dangling()?; } hir::Call::Expr { expr: e } => { let mut function = cx.scopes.defer(span); @@ -1791,7 +1791,7 @@ fn expr_call<'a, 'hir>( span, )?; - linear.free()?; + linear.free_non_dangling()?; function.free()?; } hir::Call::ConstFn { @@ -1871,9 +1871,9 @@ fn exprs_2_with<'a, 'hir, T>( } ([e], []) | ([], [e]) => { let e = map(e); - let mut needs = cx.scopes.defer(e); + let mut needs = cx.scopes.alloc(e)?; converge!(expr(cx, e, &mut needs)?, free(needs)); - linear = Linear::single(needs.into_addr()?); + linear = Linear::single(needs); } _ => { let len = a.len() + b.len(); @@ -2066,9 +2066,19 @@ fn expr_for<'a, 'hir>( let into_iter = cx.scopes.alloc(span)?.with_name("into_iter"); let binding = cx.scopes.alloc(&hir.binding)?.with_name("binding"); + // Copy the iterator, since CallAssociated will consume it. cx.asm.push_with_comment( - Inst::CallAssociated { + Inst::Copy { addr: iter.addr(), + out: into_iter.output(), + }, + span, + &"Protocol::INTO_ITER", + )?; + + cx.asm.push_with_comment( + Inst::CallAssociated { + addr: into_iter.addr(), hash: *Protocol::INTO_ITER, args: 1, out: into_iter.output(), @@ -2106,12 +2116,22 @@ fn expr_for<'a, 'hir>( drop: Some(into_iter.addr()), })?; + let into_iter_copy = cx.scopes.alloc(span)?.with_name("into_iter_copy"); + + cx.asm.push( + Inst::Copy { + addr: into_iter.addr(), + out: into_iter_copy.output(), + }, + span, + )?; + // Use the memoized loop variable. if let Some(next_offset) = &next_offset { cx.asm.push( Inst::CallFn { function: next_offset.addr(), - addr: into_iter.addr(), + addr: into_iter_copy.addr(), args: 1, out: binding.output(), }, @@ -2120,7 +2140,7 @@ fn expr_for<'a, 'hir>( } else { cx.asm.push_with_comment( Inst::CallAssociated { - addr: into_iter.addr(), + addr: into_iter_copy.addr(), hash: *Protocol::NEXT, args: 1, out: binding.output(), @@ -2130,6 +2150,8 @@ fn expr_for<'a, 'hir>( )?; } + into_iter_copy.free()?; + // Test loop condition and unwrap the option, or jump to `end_label` if the current value is `None`. cx.asm .iter_next(binding.addr(), &end_label, &hir.binding, binding.output())?; @@ -2534,9 +2556,9 @@ fn expr_object<'a, 'hir>( span, )?; } - } + }; - linear.free()?; + linear.free_non_dangling()?; } Ok(Asm::new(span, ())) @@ -2601,58 +2623,83 @@ fn expr_range<'a, 'hir>( span: &'hir dyn Spanned, needs: &mut dyn Needs<'a, 'hir>, ) -> compile::Result> { - let a: Option<&hir::Expr<'hir>>; - let b: Option<&hir::Expr<'hir>>; + let range; + let vars; - let range = match hir { + match hir { hir::ExprRange::RangeFrom { start } => { - a = Some(start); - b = None; - InstRange::RangeFrom + let mut s = cx.scopes.defer(start); + converge!(expr(cx, start, &mut s)?, free(s)); + + let start = s.into_addr()?; + + range = InstRange::RangeFrom { + start: start.addr(), + }; + vars = [Some(start), None]; } hir::ExprRange::RangeFull => { - a = None; - b = None; - InstRange::RangeFull + range = InstRange::RangeFull; + vars = [None, None]; } hir::ExprRange::RangeInclusive { start, end } => { - a = Some(start); - b = Some(end); - InstRange::RangeInclusive + let mut s = cx.scopes.defer(start); + converge!(expr(cx, start, &mut s)?, free(s)); + + let mut e = cx.scopes.defer(end); + converge!(expr(cx, end, &mut e)?, free(s, e)); + + let start = s.into_addr()?; + let end = e.into_addr()?; + + range = InstRange::RangeInclusive { + start: start.addr(), + end: end.addr(), + }; + vars = [Some(start), Some(end)]; } hir::ExprRange::RangeToInclusive { end } => { - a = Some(end); - b = None; - InstRange::RangeToInclusive + let mut e = cx.scopes.defer(end); + converge!(expr(cx, end, &mut e)?, free(e)); + + let end = e.into_addr()?; + + range = InstRange::RangeToInclusive { end: end.addr() }; + vars = [Some(end), None]; } hir::ExprRange::RangeTo { end } => { - a = Some(end); - b = None; - InstRange::RangeTo + let mut e = cx.scopes.defer(end); + converge!(expr(cx, end, &mut e)?, free(e)); + + let end = e.into_addr()?; + + range = InstRange::RangeTo { end: end.addr() }; + vars = [Some(end), None]; } hir::ExprRange::Range { start, end } => { - a = Some(start); - b = Some(end); - InstRange::Range - } - }; + let mut s = cx.scopes.defer(start); + converge!(expr(cx, start, &mut s)?, free(s)); - let a = a.map(slice::from_ref).unwrap_or_default(); - let b = b.map(slice::from_ref).unwrap_or_default(); + let mut e = cx.scopes.defer(end); + converge!(expr(cx, end, &mut e)?, free(s, e)); - if let Some(linear) = exprs_2(cx, span, a, b)?.into_converging() { - if let Some(out) = needs.try_alloc_output()? { - cx.asm.push( - Inst::Range { - addr: linear.addr(), - range, - out, - }, - span, - )?; + let start = s.into_addr()?; + let end = e.into_addr()?; + + range = InstRange::Range { + start: start.addr(), + end: end.addr(), + }; + vars = [Some(start), Some(end)]; } + }; - linear.free()?; + if let Some(out) = needs.try_alloc_output()? { + cx.asm.push(Inst::Range { range, out }, span)?; + } + + for var in vars.into_iter().flatten() { + var.free()?; } Ok(Asm::new(span, ())) @@ -2829,7 +2876,7 @@ fn expr_tuple<'a, 'hir>( cx.asm.push( Inst::$variant { - args: [$($expr.addr(),)*], + addr: [$($expr.addr(),)*], out: needs.alloc_output()?, }, span, @@ -2859,9 +2906,11 @@ fn expr_tuple<'a, 'hir>( }, span, )?; - } - linear.free()?; + linear.free_non_dangling()?; + } else { + linear.free()?; + } } } @@ -2936,9 +2985,12 @@ fn expr_vec<'a, 'hir>( }, span, )?; + + linear.free_non_dangling()?; + } else { + linear.free()?; } - linear.free()?; Ok(Asm::new(span, ())) } diff --git a/crates/rune/src/compile/v1/linear.rs b/crates/rune/src/compile/v1/linear.rs index 66ca5797e..e06036639 100644 --- a/crates/rune/src/compile/v1/linear.rs +++ b/crates/rune/src/compile/v1/linear.rs @@ -68,6 +68,15 @@ impl<'a, 'hir> Linear<'a, 'hir> { Ok(()) } + #[inline] + pub(super) fn free_non_dangling(self) -> compile::Result<()> { + for addr in self.into_iter().rev() { + addr.free_non_dangling()?; + } + + Ok(()) + } + #[inline] pub(super) fn forget(self) -> compile::Result<()> { for var in self { diff --git a/crates/rune/src/compile/v1/needs.rs b/crates/rune/src/compile/v1/needs.rs index 47bfe52a9..5be0bf19f 100644 --- a/crates/rune/src/compile/v1/needs.rs +++ b/crates/rune/src/compile/v1/needs.rs @@ -290,15 +290,25 @@ impl<'a, 'hir> Address<'a, 'hir> { Ok(()) } + pub(super) fn free(self) -> compile::Result<()> { + self.free_inner(true) + } + + pub(super) fn free_non_dangling(self) -> compile::Result<()> { + self.free_inner(false) + } + /// Free the current address. - pub(super) fn free(mut self) -> compile::Result<()> { + fn free_inner(mut self, dangling: bool) -> compile::Result<()> { match replace(&mut self.kind, AddressKind::Freed) { AddressKind::Local | AddressKind::Dangling => { - self.scopes.free_addr(self.span, self.address, self.name)?; + self.scopes + .free_addr(self.span, self.address, self.name, dangling)?; } AddressKind::Scope(scope) => { if self.scopes.top_id() == scope { - self.scopes.free_addr(self.span, self.address, self.name)?; + self.scopes + .free_addr(self.span, self.address, self.name, dangling)?; } } AddressKind::Freed => { diff --git a/crates/rune/src/compile/v1/scopes.rs b/crates/rune/src/compile/v1/scopes.rs index f7dd1200e..a6dd1840b 100644 --- a/crates/rune/src/compile/v1/scopes.rs +++ b/crates/rune/src/compile/v1/scopes.rs @@ -421,6 +421,7 @@ impl<'hir> Scopes<'hir> { span: &dyn Spanned, addr: InstAddress, name: Option<&'static str>, + dangling: bool, ) -> compile::Result<()> { let mut scopes = self.scopes.borrow_mut(); @@ -447,7 +448,9 @@ impl<'hir> Scopes<'hir> { )); } - self.dangling.borrow_mut().insert(addr).with_span(span)?; + if dangling { + self.dangling.borrow_mut().insert(addr).with_span(span)?; + } let mut slots = self.slots.borrow_mut(); diff --git a/crates/rune/src/function/mod.rs b/crates/rune/src/function/mod.rs index 51749a81a..7efecf642 100644 --- a/crates/rune/src/function/mod.rs +++ b/crates/rune/src/function/mod.rs @@ -2,6 +2,7 @@ mod macros; use core::future::Future; +use core::mem::replace; use crate::alloc; use crate::compile::meta; @@ -12,8 +13,8 @@ use crate::runtime::{ }; // Expand to function variable bindings. -macro_rules! access_stack { - ($count:expr, $add:expr, $stack:ident, $addr:ident, $args:ident, $($from_fn:path, $var:ident, $num:expr),* $(,)?) => { +macro_rules! access_memory { + ($count:expr, $add:expr, $memory:ident, $addr:ident, $args:ident, $($from_fn:path, $var:ident, $num:expr),* $(,)?) => { if $args != $count + $add { return VmResult::err(VmErrorKind::BadArgumentCount { actual: $args, @@ -21,8 +22,11 @@ macro_rules! access_stack { }); } - let [$($var,)*] = vm_try!($stack.array_at($addr)); - $(let $var = Clone::clone($var);)* + let [$($var,)*] = vm_try!($memory.slice_at_mut($addr, $args)) else { + unreachable!(); + }; + + $(let $var = replace($var, Value::empty());)* $( let $var = vm_try!($from_fn($var).with_error(|| VmErrorKind::BadArgument { @@ -79,7 +83,7 @@ pub trait Function: 'static + Send + Sync { #[doc(hidden)] fn fn_call( &self, - stack: &mut dyn Memory, + memory: &mut dyn Memory, addr: InstAddress, args: usize, out: Output, @@ -109,7 +113,7 @@ pub trait InstanceFunction: 'static + Send + Sync { #[doc(hidden)] fn fn_call( &self, - stack: &mut dyn Memory, + memory: &mut dyn Memory, addr: InstAddress, args: usize, out: Output, @@ -132,8 +136,8 @@ macro_rules! impl_instance_function_traits { } #[inline] - fn fn_call(&self, stack: &mut dyn Memory, addr: InstAddress, args: usize, out: Output) -> VmResult<()> { - Function::fn_call(self, stack, addr, args, out) + fn fn_call(&self, memory: &mut dyn Memory, addr: InstAddress, args: usize, out: Output) -> VmResult<()> { + Function::fn_call(self, memory, addr, args, out) } } }; @@ -260,16 +264,16 @@ macro_rules! impl_function_traits { } #[allow(clippy::drop_non_drop)] - fn fn_call(&self, stack: &mut dyn Memory, addr: InstAddress, args: usize, out: Output) -> VmResult<()> { - access_stack!($count, 0, stack, addr, args, $($from_fn, $var, $num,)*); + fn fn_call(&self, memory: &mut dyn Memory, addr: InstAddress, args: usize, out: Output) -> VmResult<()> { + access_memory!($count, 0, memory, addr, args, $($from_fn, $var, $num,)*); - // Safety: We hold a reference to the stack, so we can guarantee + // Safety: We hold a reference to memory, so we can guarantee // that it won't be modified. let ret = self($($var.0),*); $(drop($var.1);)* let value = vm_try!(ToValue::to_value(ret)); - vm_try!(out.store(stack, value)); + vm_try!(out.store(memory, value)); VmResult::Ok(()) } } @@ -288,8 +292,8 @@ macro_rules! impl_function_traits { } #[allow(clippy::drop_non_drop)] - fn fn_call(&self, stack: &mut dyn Memory, addr: InstAddress, args: usize, out: Output) -> VmResult<()> { - access_stack!($count, 0, stack, addr, args, $($from_fn, $var, $num,)*); + fn fn_call(&self, memory: &mut dyn Memory, addr: InstAddress, args: usize, out: Output) -> VmResult<()> { + access_memory!($count, 0, memory, addr, args, $($from_fn, $var, $num,)*); let fut = self($($var.0),*); // Note: we may drop any produced reference guard here since the @@ -304,7 +308,7 @@ macro_rules! impl_function_traits { })); let value = vm_try!(Value::try_from(ret)); - vm_try!(out.store(stack, value)); + vm_try!(out.store(memory, value)); VmResult::Ok(()) } } diff --git a/crates/rune/src/runtime/inst.rs b/crates/rune/src/runtime/inst.rs index 101379c3e..7bf5225fa 100644 --- a/crates/rune/src/runtime/inst.rs +++ b/crates/rune/src/runtime/inst.rs @@ -172,7 +172,7 @@ pub enum Inst { /// /// The function will be looked up in the unit and context. The arguments /// passed to the function call are stored at `addr`, where `size` - /// determines the number of arguments. + /// determines the number of arguments. The arguments will be dropped. /// /// The return value of the function call will be written to `out`. #[musli(packed)] @@ -510,15 +510,10 @@ pub enum Inst { /// The offset to jump if the condition is true. jump: usize, }, - /// Construct a push a vector value onto the stack. The number of elements - /// in the vector are determined by `count` and are popped from the stack. + /// Construct a vector at `out`, populating it with `count` elements from + /// `addr`. /// - /// # Operation - /// - /// ```text - /// - /// => - /// ``` + /// The values at `addr` are dropped. #[musli(packed)] Vec { /// Where the arguments to the vector are stored. @@ -528,75 +523,58 @@ pub enum Inst { /// Where to store the produced vector. out: Output, }, - /// Construct a push a one-tuple value onto the stack. - /// - /// # Operation + /// Construct a one element tuple at `out`, populating it with `count` + /// elements from `addr`. /// - /// ```text - /// => - /// ``` + /// The values at `addr` are not dropped. #[musli(packed)] Tuple1 { - /// First element of the tuple. + /// Tuple arguments. #[inst_display(display_with = DisplayArray::new)] - args: [InstAddress; 1], + addr: [InstAddress; 1], /// Where to store the produced tuple. out: Output, }, - /// Construct a push a two-tuple value onto the stack. - /// - /// # Operation + /// Construct a two element tuple at `out`, populating it with `count` + /// elements from `addr`. /// - /// ```text - /// => - /// ``` + /// The values at `addr` are not dropped. #[musli(packed)] Tuple2 { /// Tuple arguments. #[inst_display(display_with = DisplayArray::new)] - args: [InstAddress; 2], + addr: [InstAddress; 2], /// Where to store the produced tuple. out: Output, }, - /// Construct a push a three-tuple value onto the stack. - /// - /// # Operation + /// Construct a three element tuple at `out`, populating it with `count` + /// elements from `addr`. /// - /// ```text - /// => - /// ``` + /// The values at `addr` are not dropped. #[musli(packed)] Tuple3 { /// Tuple arguments. #[inst_display(display_with = DisplayArray::new)] - args: [InstAddress; 3], + addr: [InstAddress; 3], /// Where to store the produced tuple. out: Output, }, - /// Construct a push a four-tuple value onto the stack. + /// Construct a four element tuple at `out`, populating it with `count` + /// elements from `addr`. /// - /// # Operation - /// - /// ```text - /// => - /// ``` + /// The values at `addr` are not dropped. #[musli(packed)] Tuple4 { /// Tuple arguments. #[inst_display(display_with = DisplayArray::new)] - args: [InstAddress; 4], + addr: [InstAddress; 4], /// Where to store the produced tuple. out: Output, }, - /// Construct a push a tuple value onto the stack. The number of elements - /// in the tuple are determined by `count` and are popped from the stack. - /// - /// # Operation + /// Construct a tuple at `out`, populating it with `count` elements from + /// `addr`. /// - /// ```text - /// - /// => - /// ``` + /// Unlike `TupleN` variants, values at `addr` are dropped. #[musli(packed)] Tuple { /// Where the arguments to the tuple are stored. @@ -647,22 +625,14 @@ pub enum Inst { /// Where to store the produced tuple. out: Output, }, - /// Construct a range. This will pop the start and end of the range from the - /// stack. - /// - /// # Operation + /// Construct a range. /// - /// ```text - /// [start] - /// [end] - /// => - /// ``` + /// The arguments loaded are determined by the range being constructed. #[musli(packed)] Range { - /// The kind of the range, which determines the number of arguments on the stack. + /// The kind of the range, which determines the number arguments on the + /// stack. range: InstRange, - /// Where the arguments of the range are stored. - addr: InstAddress, /// Where to store the produced range. out: Output, }, @@ -681,18 +651,11 @@ pub enum Inst { /// Where to write the empty struct. out: Output, }, - /// Construct a push an object of the given type onto the stack. The number - /// of elements in the object are determined the slot of the object keys - /// `slot` and are popped from the stack. - /// - /// For each element, a value is popped corresponding to the object key. - /// - /// # Operation + /// Construct a struct of type `hash` at `out`, populating it with fields + /// from `addr`. The number of fields and their names is determined by the + /// `slot` being referenced. /// - /// ```text - /// - /// => - /// ``` + /// The values at `addr` are dropped. #[musli(packed)] Struct { /// The address to load fields from. @@ -1423,30 +1386,50 @@ impl fmt::Debug for InstAddress { /// Range limits of a range expression. #[derive(Debug, TryClone, Clone, Copy, Serialize, Deserialize, Decode, Encode)] #[try_clone(copy)] +#[non_exhaustive] pub enum InstRange { /// `start..`. - RangeFrom, + RangeFrom { + /// The start address of the range. + start: InstAddress, + }, /// `..`. RangeFull, /// `start..=end`. - RangeInclusive, + RangeInclusive { + /// The start address of the range. + start: InstAddress, + /// The end address of the range. + end: InstAddress, + }, /// `..=end`. - RangeToInclusive, + RangeToInclusive { + /// The end address of the range. + end: InstAddress, + }, /// `..end`. - RangeTo, + RangeTo { + /// The end address of the range. + end: InstAddress, + }, /// `start..end`. - Range, + Range { + /// The start address of the range. + start: InstAddress, + /// The end address of the range. + end: InstAddress, + }, } impl fmt::Display for InstRange { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - InstRange::RangeFrom => write!(f, "start.."), + InstRange::RangeFrom { start } => write!(f, "{start}.."), InstRange::RangeFull => write!(f, ".."), - InstRange::RangeInclusive => write!(f, "start..=end"), - InstRange::RangeToInclusive => write!(f, "..=end"), - InstRange::RangeTo => write!(f, "..end"), - InstRange::Range => write!(f, "start..end"), + InstRange::RangeInclusive { start, end } => write!(f, "{start}..={end}"), + InstRange::RangeToInclusive { end } => write!(f, "..={end}"), + InstRange::RangeTo { end } => write!(f, "..{end}"), + InstRange::Range { start, end } => write!(f, "{start}..{end}"), } } } diff --git a/crates/rune/src/runtime/stack.rs b/crates/rune/src/runtime/stack.rs index 63c18b6fa..a66696dfd 100644 --- a/crates/rune/src/runtime/stack.rs +++ b/crates/rune/src/runtime/stack.rs @@ -57,7 +57,6 @@ pub trait Memory { /// /// ``` /// use rune::vm_try; - /// use rune::Module; /// use rune::runtime::{Output, Memory, ToValue, VmResult, InstAddress}; /// /// fn sum(stack: &mut dyn Memory, addr: InstAddress, args: usize, out: Output) -> VmResult<()> { @@ -73,6 +72,25 @@ pub trait Memory { /// ``` fn slice_at(&self, addr: InstAddress, len: usize) -> Result<&[Value], SliceError>; + /// Access the given slice mutably. + /// + /// # Examples + /// + /// ``` + /// use rune::vm_try; + /// use rune::runtime::{Output, Memory, InstAddress, Value, VmResult}; + /// + /// fn drop_values(stack: &mut dyn Memory, addr: InstAddress, args: usize, out: Output) -> VmResult<()> { + /// for value in vm_try!(stack.slice_at_mut(addr, args)) { + /// *value = Value::empty(); + /// } + /// + /// out.store(stack, ()); + /// VmResult::Ok(()) + /// } + /// ``` + fn slice_at_mut(&mut self, addr: InstAddress, len: usize) -> Result<&mut [Value], SliceError>; + /// Get a value mutable at the given index from the stack bottom. /// /// # Examples @@ -111,6 +129,11 @@ where (**self).slice_at(addr, len) } + #[inline] + fn slice_at_mut(&mut self, addr: InstAddress, len: usize) -> Result<&mut [Value], SliceError> { + (**self).slice_at_mut(addr, len) + } + #[inline] fn at_mut(&mut self, addr: InstAddress) -> Result<&mut Value, StackError> { (**self).at_mut(addr) @@ -136,6 +159,27 @@ impl Memory for [Value; N] { Ok(values) } + fn slice_at_mut(&mut self, addr: InstAddress, len: usize) -> Result<&mut [Value], SliceError> { + if len == 0 { + return Ok(&mut []); + } + + let start = addr.offset(); + + let Some(values) = start + .checked_add(len) + .and_then(|end| self.get_mut(start..end)) + else { + return Err(SliceError { + addr, + len, + stack: N, + }); + }; + + Ok(values) + } + #[inline] fn at_mut(&mut self, addr: InstAddress) -> Result<&mut Value, StackError> { let Some(value) = self.get_mut(addr.offset()) else { @@ -334,15 +378,6 @@ impl Stack { self.stack.drain(self.top..) } - /// Get the slice at the given address with the given static length. - pub(crate) fn array_at( - &self, - addr: InstAddress, - ) -> Result<[&Value; N], SliceError> { - let slice = self.slice_at(addr, N)?; - Ok(array::from_fn(|i| &slice[i])) - } - /// Clear the current stack. pub(crate) fn clear(&mut self) { self.stack.clear(); @@ -423,10 +458,10 @@ impl Stack { // values. It is also guaranteed to be non-overlapping. unsafe { let ptr = self.stack.as_mut_ptr(); - let from = slice::from_raw_parts(ptr.add(start), len); + let from = slice::from_raw_parts_mut(ptr.add(start), len); - for (value, n) in from.iter().zip(old_len..) { - ptr.add(n).write(value.clone()); + for (value, n) in from.iter_mut().zip(old_len..) { + ptr.add(n).write(replace(value, Value::empty())); } self.stack.set_len(new_len); @@ -454,6 +489,11 @@ impl Memory for Stack { Stack::slice_at(self, addr, len) } + #[inline] + fn slice_at_mut(&mut self, addr: InstAddress, len: usize) -> Result<&mut [Value], SliceError> { + Stack::slice_at_mut(self, addr, len) + } + #[inline] fn at_mut(&mut self, addr: InstAddress) -> Result<&mut Value, StackError> { Stack::at_mut(self, addr) diff --git a/crates/rune/src/runtime/value.rs b/crates/rune/src/runtime/value.rs index 57965e39f..8ccfa7040 100644 --- a/crates/rune/src/runtime/value.rs +++ b/crates/rune/src/runtime/value.rs @@ -607,7 +607,7 @@ impl Value { } /// Construct an empty value. - pub(crate) const fn empty() -> Self { + pub const fn empty() -> Self { Self { repr: ValueRepr::Empty, } diff --git a/crates/rune/src/runtime/vm.rs b/crates/rune/src/runtime/vm.rs index 012d5e98a..b67c61678 100644 --- a/crates/rune/src/runtime/vm.rs +++ b/crates/rune/src/runtime/vm.rs @@ -26,6 +26,16 @@ use crate::runtime::{budget, ProtocolCaller}; use super::{VmDiagnostics, VmDiagnosticsObj}; +/// Helper to take a value, replacing the old one with empty. +#[inline(always)] +fn take(value: &mut Value) -> Value { + replace(value, Value::empty()) +} + +fn consume(value: &mut Value) { + *value = Value::empty(); +} + /// Indicating the kind of isolation that is present for a frame. #[derive(Debug, Clone, Copy)] pub enum Isolated { @@ -1247,13 +1257,15 @@ impl Vm { args: usize, out: Output, ) -> Result<(), VmErrorKind> { - let iter = self.stack.slice_at(addr, args)?; + let values = self.stack.slice_at_mut(addr, args)?; if let Some(at) = out.as_addr() { - let stack = iter.iter().cloned().try_collect::()?; + let stack = values.iter_mut().map(take).try_collect::()?; let mut vm = Self::with_stack(self.context.clone(), self.unit.clone(), stack); vm.ip = offset; *self.stack.at_mut(at)? = Value::try_from(Generator::new(vm))?; + } else { + values.iter_mut().for_each(consume); } Ok(()) @@ -1267,13 +1279,15 @@ impl Vm { args: usize, out: Output, ) -> Result<(), VmErrorKind> { - let stack = self.stack.slice_at(addr, args)?; + let values = self.stack.slice_at_mut(addr, args)?; if let Some(at) = out.as_addr() { - let stack = stack.iter().cloned().try_collect::()?; + let stack = values.iter_mut().map(take).try_collect::()?; let mut vm = Self::with_stack(self.context.clone(), self.unit.clone(), stack); vm.ip = offset; *self.stack.at_mut(at)? = Value::try_from(Stream::new(vm))?; + } else { + values.iter_mut().for_each(consume); } Ok(()) @@ -1287,15 +1301,17 @@ impl Vm { args: usize, out: Output, ) -> Result<(), VmErrorKind> { - let stack = self.stack.slice_at(addr, args)?; + let values = self.stack.slice_at_mut(addr, args)?; if let Some(at) = out.as_addr() { - let stack = stack.iter().cloned().try_collect::()?; + let stack = values.iter_mut().map(take).try_collect::()?; let mut vm = Self::with_stack(self.context.clone(), self.unit.clone(), stack); vm.ip = offset; let mut execution = vm.into_execution(); let future = Future::new(async move { execution.async_complete().await })?; *self.stack.at_mut(at)? = Value::try_from(future)?; + } else { + values.iter_mut().for_each(consume); } Ok(()) @@ -1756,8 +1772,8 @@ impl Vm { /// Construct a new vec. #[cfg_attr(feature = "bench", inline(never))] fn op_vec(&mut self, addr: InstAddress, count: usize, out: Output) -> VmResult<()> { - let vec = vm_try!(self.stack.slice_at(addr, count)); - let vec = vm_try!(vec.iter().cloned().try_collect::>()); + let vec = vm_try!(self.stack.slice_at_mut(addr, count)); + let vec = vm_try!(vec.iter_mut().map(take).try_collect::>()); vm_try!(out.store(&mut self.stack, || ValueKind::Vec(Vec::from(vec)))); VmResult::Ok(()) } @@ -1765,8 +1781,12 @@ impl Vm { /// Construct a new tuple. #[cfg_attr(feature = "bench", inline(never))] fn op_tuple(&mut self, addr: InstAddress, count: usize, out: Output) -> VmResult<()> { - let tuple = vm_try!(self.stack.slice_at(addr, count)); - let tuple = vm_try!(tuple.iter().cloned().try_collect::>()); + let tuple = vm_try!(self.stack.slice_at_mut(addr, count)); + + let tuple = vm_try!(tuple + .iter_mut() + .map(take) + .try_collect::>()); vm_try!( out.store(&mut self.stack, || VmResult::Ok(ValueKind::Tuple(vm_try!( @@ -1779,10 +1799,10 @@ impl Vm { /// Construct a new tuple with a fixed number of arguments. #[cfg_attr(feature = "bench", inline(never))] - fn op_tuple_n(&mut self, args: &[InstAddress], out: Output) -> VmResult<()> { - let mut tuple = vm_try!(alloc::Vec::::try_with_capacity(args.len())); + fn op_tuple_n(&mut self, addr: &[InstAddress], out: Output) -> VmResult<()> { + let mut tuple = vm_try!(alloc::Vec::::try_with_capacity(addr.len())); - for &arg in args { + for &arg in addr { let value = vm_try!(self.stack.at(arg)).clone(); vm_try!(tuple.try_push(value)); } @@ -2501,11 +2521,11 @@ impl Vm { .ok_or(VmErrorKind::MissingStaticObjectKeys { slot })); let mut object = vm_try!(Object::with_capacity(keys.len())); - let values = vm_try!(self.stack.slice_at(addr, keys.len())); + let values = vm_try!(self.stack.slice_at_mut(addr, keys.len())); for (key, value) in keys.iter().zip(values) { let key = vm_try!(String::try_from(key.as_str())); - vm_try!(object.insert(key, value.clone())); + vm_try!(object.insert(key, take(value))); } vm_try!(out.store(&mut self.stack, ValueKind::Object(object))); @@ -2514,29 +2534,31 @@ impl Vm { /// Operation to allocate an object. #[cfg_attr(feature = "bench", inline(never))] - fn op_range(&mut self, addr: InstAddress, range: InstRange, out: Output) -> VmResult<()> { + fn op_range(&mut self, range: InstRange, out: Output) -> VmResult<()> { let value = match range { - InstRange::RangeFrom => { - let [s] = vm_try!(self.stack.array_at(addr)); + InstRange::RangeFrom { start } => { + let s = vm_try!(self.stack.at(start)).clone(); vm_try!(Value::try_from(RangeFrom::new(s.clone()))) } InstRange::RangeFull => { vm_try!(Value::try_from(RangeFull::new())) } - InstRange::RangeInclusive => { - let [s, e] = vm_try!(self.stack.array_at(addr)); + InstRange::RangeInclusive { start, end } => { + let s = vm_try!(self.stack.at(start)).clone(); + let e = vm_try!(self.stack.at(end)).clone(); vm_try!(Value::try_from(RangeInclusive::new(s.clone(), e.clone()))) } - InstRange::RangeToInclusive => { - let [e] = vm_try!(self.stack.array_at(addr)); + InstRange::RangeToInclusive { end } => { + let e = vm_try!(self.stack.at(end)).clone(); vm_try!(Value::try_from(RangeToInclusive::new(e.clone()))) } - InstRange::RangeTo => { - let [e] = vm_try!(self.stack.array_at(addr)); + InstRange::RangeTo { end } => { + let e = vm_try!(self.stack.at(end)).clone(); vm_try!(Value::try_from(RangeTo::new(e.clone()))) } - InstRange::Range => { - let [s, e] = vm_try!(self.stack.array_at(addr)); + InstRange::Range { start, end } => { + let s = vm_try!(self.stack.at(start)).clone(); + let e = vm_try!(self.stack.at(end)).clone(); vm_try!(Value::try_from(Range::new(s.clone(), e.clone()))) } }; @@ -2581,12 +2603,12 @@ impl Vm { .lookup_rtti(hash) .ok_or(VmErrorKind::MissingRtti { hash })); - let values = vm_try!(self.stack.slice_at(addr, keys.len())); let mut data = vm_try!(Object::with_capacity(keys.len())); + let values = vm_try!(self.stack.slice_at_mut(addr, keys.len())); for (key, value) in keys.iter().zip(values) { let key = vm_try!(String::try_from(key.as_str())); - vm_try!(data.insert(key, value.clone())); + vm_try!(data.insert(key, take(value))); } vm_try!(out.store(&mut self.stack, || ValueKind::Struct(Struct { @@ -2599,7 +2621,7 @@ impl Vm { /// Operation to allocate an object variant. #[cfg_attr(feature = "bench", inline(never))] - fn op_object_variant( + fn op_struct_variant( &mut self, addr: InstAddress, hash: Hash, @@ -2617,11 +2639,11 @@ impl Vm { .ok_or(VmErrorKind::MissingVariantRtti { hash })); let mut data = vm_try!(Object::with_capacity(keys.len())); - let values = vm_try!(self.stack.slice_at(addr, keys.len())); + let values = vm_try!(self.stack.slice_at_mut(addr, keys.len())); for (key, value) in keys.iter().zip(values) { let key = vm_try!(String::try_from(key.as_str())); - vm_try!(data.insert(key, value.clone())); + vm_try!(data.insert(key, take(value))); } vm_try!( @@ -3093,8 +3115,8 @@ impl Vm { .lookup_rtti(hash) .ok_or(VmErrorKind::MissingRtti { hash })); - let tuple = vm_try!(self.stack.slice_at(addr, args)); - let tuple = vm_try!(tuple.iter().cloned().try_collect()); + let tuple = vm_try!(self.stack.slice_at_mut(addr, args)); + let tuple = vm_try!(tuple.iter_mut().map(take).try_collect()); vm_try!(out.store(&mut self.stack, || { Value::tuple_struct(rtti.clone(), tuple) @@ -3111,8 +3133,8 @@ impl Vm { .lookup_variant_rtti(hash) .ok_or(VmErrorKind::MissingVariantRtti { hash })); - let tuple = vm_try!(self.stack.slice_at(addr, args)); - let tuple = vm_try!(tuple.iter().cloned().try_collect()); + let tuple = vm_try!(self.stack.slice_at_mut(addr, args)); + let tuple = vm_try!(tuple.iter_mut().map(take).try_collect()); vm_try!(out.store(&mut self.stack, || Value::tuple_variant( rtti.clone(), @@ -3454,17 +3476,17 @@ impl Vm { Inst::Tuple { addr, count, out } => { vm_try!(self.op_tuple(addr, count, out)); } - Inst::Tuple1 { args, out } => { - vm_try!(self.op_tuple_n(&args[..], out)); + Inst::Tuple1 { addr, out } => { + vm_try!(self.op_tuple_n(&addr[..], out)); } - Inst::Tuple2 { args, out } => { - vm_try!(self.op_tuple_n(&args[..], out)); + Inst::Tuple2 { addr, out } => { + vm_try!(self.op_tuple_n(&addr[..], out)); } - Inst::Tuple3 { args, out } => { - vm_try!(self.op_tuple_n(&args[..], out)); + Inst::Tuple3 { addr, out } => { + vm_try!(self.op_tuple_n(&addr[..], out)); } - Inst::Tuple4 { args, out } => { - vm_try!(self.op_tuple_n(&args[..], out)); + Inst::Tuple4 { addr, out } => { + vm_try!(self.op_tuple_n(&addr[..], out)); } Inst::Environment { addr, count, out } => { vm_try!(self.op_environment(addr, count, out)); @@ -3472,8 +3494,8 @@ impl Vm { Inst::Object { addr, slot, out } => { vm_try!(self.op_object(addr, slot, out)); } - Inst::Range { addr, range, out } => { - vm_try!(self.op_range(addr, range, out)); + Inst::Range { range, out } => { + vm_try!(self.op_range(range, out)); } Inst::EmptyStruct { hash, out } => { vm_try!(self.op_empty_struct(hash, out)); @@ -3492,7 +3514,7 @@ impl Vm { slot, out, } => { - vm_try!(self.op_object_variant(addr, hash, slot, out)); + vm_try!(self.op_struct_variant(addr, hash, slot, out)); } Inst::String { slot, out } => { vm_try!(self.op_string(slot, out)); diff --git a/crates/rune/src/shared/mod.rs b/crates/rune/src/shared/mod.rs index d3c34699e..d0f50e997 100644 --- a/crates/rune/src/shared/mod.rs +++ b/crates/rune/src/shared/mod.rs @@ -35,6 +35,7 @@ macro_rules! _rune_diagnose { #[doc(inline)] pub(crate) use _rune_diagnose as rune_diagnose; +#[cfg(debug_assertions)] pub(crate) enum RuneAssert { /// Assert should panic. Panic, @@ -42,6 +43,7 @@ pub(crate) enum RuneAssert { Error, } +#[cfg(debug_assertions)] impl RuneAssert { /// Test if the assert is a panic. pub(crate) fn is_panic(&self) -> bool { @@ -49,7 +51,7 @@ impl RuneAssert { } } -#[cfg(not(feature = "std"))] +#[cfg(all(debug_assertions, not(feature = "std")))] mod r#impl { use core::fmt; @@ -77,7 +79,7 @@ mod r#impl { } /// Test whether current assertions model should panic. -#[cfg(feature = "std")] +#[cfg(all(debug_assertions, feature = "std"))] mod r#impl { use core::sync::atomic::{AtomicU8, Ordering}; @@ -111,4 +113,5 @@ mod r#impl { } } +#[cfg(debug_assertions)] pub(crate) use self::r#impl::*;