diff --git a/src/eval/chaining.rs b/src/eval/chaining.rs index be16346ab..d54913c0a 100644 --- a/src/eval/chaining.rs +++ b/src/eval/chaining.rs @@ -11,10 +11,7 @@ use crate::{ }; #[cfg(feature = "no_std")] use std::prelude::v1::*; -use std::{ - convert::{TryFrom, TryInto}, - hash::Hash, -}; +use std::{convert::TryInto, hash::Hash}; /// Function call hashes to index getters and setters. static INDEXER_HASHES: OnceCell<(u64, u64)> = OnceCell::new(); @@ -121,6 +118,9 @@ impl Engine { _add_if_not_found: bool, use_indexers: bool, ) -> RhaiResultOf> { + #[cfg(not(feature = "no_index"))] + use std::convert::TryFrom; + self.track_operation(global, Position::NONE)?; match target { diff --git a/src/func/call.rs b/src/func/call.rs index b4cfc2267..ca6668772 100644 --- a/src/func/call.rs +++ b/src/func/call.rs @@ -479,64 +479,95 @@ impl Engine { } // Error handling + // + // Note: we cannot use `assert!` here (e.g. to check for number of arguments) because + // this function may be called from the VM running a corrupted bytecodes stream! match name { // index getter function not found? #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] - crate::engine::FN_IDX_GET => { - debug_assert_eq!(args.len(), 2); - + crate::engine::FN_IDX_GET => Err(if args.len() != 2 { + ERR::ErrorParsing( + crate::ParseErrorType::MalformedIndexExpr(format!( + "System error: {} argument(s) found for indexer (should be 2)", + args.len() + )), + pos, + ) + .into() + } else { let t0 = self.map_type_name(args[0].type_name()); let t1 = self.map_type_name(args[1].type_name()); - - Err(ERR::ErrorIndexingType(format!("{t0} [{t1}]"), pos).into()) - } + ERR::ErrorIndexingType(format!("{t0} [{t1}]"), pos).into() + }), // index setter function not found? #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] - crate::engine::FN_IDX_SET => { - debug_assert_eq!(args.len(), 3); - + crate::engine::FN_IDX_SET => Err(if args.len() != 3 { + ERR::ErrorParsing( + crate::ParseErrorType::MalformedIndexExpr(format!( + "System error: {} argument(s) found for index setter (should be 3)", + args.len() + )), + pos, + ) + .into() + } else { let t0 = self.map_type_name(args[0].type_name()); let t1 = self.map_type_name(args[1].type_name()); let t2 = self.map_type_name(args[2].type_name()); - - Err(ERR::ErrorIndexingType(format!("{t0} [{t1}] = {t2}"), pos).into()) - } + ERR::ErrorIndexingType(format!("{t0} [{t1}] = {t2}"), pos).into() + }), // Getter function not found? #[cfg(not(feature = "no_object"))] _ if name.starts_with(crate::engine::FN_GET) => { - debug_assert_eq!(args.len(), 1); - - let prop = &name[crate::engine::FN_GET.len()..]; - let t0 = self.map_type_name(args[0].type_name()); - - Err(ERR::ErrorDotExpr( - format!( - "Unknown property '{prop}' - a getter is not registered for type '{t0}'" - ), + let prop = &name[crate::engine::FN_SET.len()..]; + Err(if args.len() != 1 { + ERR::ErrorParsing( + crate::ParseErrorType::MalformedIndexExpr(format!( + "System error: {} argument(s) found for property getter '{prop}' (should be 1)", + args.len() + )), pos, ) - .into()) + .into() + } else { + let t0 = self.map_type_name(args[0].type_name()); + ERR::ErrorDotExpr( + format!( + "Unknown property '{prop}' - a getter is not registered for type '{t0}'" + ), + pos, + ) + .into() + }) } // Setter function not found? #[cfg(not(feature = "no_object"))] _ if name.starts_with(crate::engine::FN_SET) => { - debug_assert_eq!(args.len(), 2); - let prop = &name[crate::engine::FN_SET.len()..]; - let t0 = self.map_type_name(args[0].type_name()); - let t1 = self.map_type_name(args[1].type_name()); - - Err(ERR::ErrorDotExpr( + Err(if args.len() != 2 { + ERR::ErrorParsing( + crate::ParseErrorType::MalformedIndexExpr(format!( + "System error: {} argument(s) found for property setter '{prop}' (should be 2)", + args.len() + )), + pos, + ) + .into() + } else { + let t0 = self.map_type_name(args[0].type_name()); + let t1 = self.map_type_name(args[1].type_name()); + ERR::ErrorDotExpr( format!( "No writable property '{prop}' - a setter is not registered for type '{t0}' to handle '{t1}'" ), pos, ) - .into()) + .into() + }) } // Raise error diff --git a/src/grain/compile/mod.rs b/src/grain/compile/mod.rs index 358f3e609..c243f3bf7 100644 --- a/src/grain/compile/mod.rs +++ b/src/grain/compile/mod.rs @@ -405,6 +405,39 @@ impl Lowering { _ => return false, }; + // Evaluate the assignment value first, so the chain can read it back + // after the lvalue steps have been resolved. + + // The chain is a single expression, so the value is evaluated before + // the root and steps. + + let rewind_mark = self.mark(); + let unwind_depth = self.slots.depth(); + + let value_slot = if let Some(value) = value { + if self.slots.is_full() { + return false; + } + + let value_name = ImmutableString::from("$CHAIN_SET_VALUE$"); + let value_name_index = self.push_name(value_name.clone()); + let value_slot = self.slots.declare(value_name); + + // First evaluate the assigned value first, stash it so the chain + // can read it back after the lvalue steps have been resolved. + self.emit(Op::Unit); + self.emit(Op::DeclareLocal { + name: value_name_index, + is_const: false, + }); + self.expression(value); + self.emit(Op::StoreLocal(value_slot)); + + Some(value_slot) + } else { + None + }; + // Index values and method arguments are evaluated first, in step // order, exactly as Rhai collects them before walking // (`eval/chaining.rs:568`). Evaluating one partway down would need the @@ -434,9 +467,17 @@ impl Lowering { } ChainStep::Method(call, pos) => { if !self.is_lowerable_call(call) { + if value_slot.is_some() { + self.rewind(rewind_mark); + self.slots.unwind_to(unwind_depth); + } return false; } let Ok(argc) = u8::try_from(call.args.len()) else { + if value_slot.is_some() { + self.rewind(rewind_mark); + self.slots.unwind_to(unwind_depth); + } return false; }; let first = operands; @@ -461,10 +502,8 @@ impl Lowering { self.expression(root); } - // The value being assigned goes on last, above everything, so the - // walk can take what it needs before it borrows the container. - if let Some(value) = value { - self.expression(value); + if let Some(value_slot) = value_slot { + self.emit(Op::LoadLocal(value_slot)); } let index = self.push_chain(Chain { @@ -474,6 +513,7 @@ impl Lowering { operands, }); self.emit_at(Op::Chain(index), expr.position()); + self.unwind_to(unwind_depth); true }