From dc9b4d7f4d3832f087c10a53a02f0f4f83cc9ff7 Mon Sep 17 00:00:00 2001 From: Stephen Chung Date: Tue, 18 May 2021 20:12:30 +0800 Subject: [PATCH] Indexer as fallback to property. --- CHANGELOG.md | 3 + src/ast.rs | 14 +++-- src/engine.rs | 137 ++++++++++++++++++++++++++++++++++------------ src/engine_api.rs | 24 ++++---- src/fn_call.rs | 16 ++---- src/module/mod.rs | 9 +-- src/optimize.rs | 2 +- src/parser.rs | 10 +--- tests/get_set.rs | 19 +++++-- 9 files changed, 155 insertions(+), 79 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1082da15..9e975ac1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ Breaking changes * `Engine::disable_doc_comments` is removed because doc-comments are now placed under the `metadata` feature flag. * Registering a custom syntax now only requires specifying whether the `Scope` is adjusted (i.e. whether variables are added or removed). There is no need to specify the number of variables added/removed. * Assigning to a property of a constant is now allowed and no longer raise an `EvalAltResult::ErrorAssignmentToConstant` error. This is to facilitate the Singleton pattern. Registered setter functions are automatically guarded against setters calling on constants and will continue to raise errors unless the `pure` attribute is present (for plugins). +* If a property getter/setter is not found, an indexer with string index, if any, is tried. +* The indexers API (`Engine::register_indexer_XXX` and `Module::set_indexer_XXX`) are now also exposed under `no_index`. New features ------------ @@ -23,6 +25,7 @@ New features * A new internal feature `no_smartstring` to turn off `SmartString` for those rare cases that it is needed. * `DynamicReadLock` and `DynamicWriteLoc` are exposed under `internals`. * `From>>` is added for `Dynamic` mapping directly to a shared value, together with support for `Dynamic::from`. +* An indexer with string index acts as a _fallback_ to a property getter/setter. Enhancements ------------ diff --git a/src/ast.rs b/src/ast.rs index aa1a1a4a..d90be201 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -1715,7 +1715,13 @@ pub enum Expr { )>, ), /// Property access - ((getter, hash), (setter, hash), prop) - Property(Box<((Identifier, u64), (Identifier, u64), Ident)>), + Property( + Box<( + (Identifier, u64), + (Identifier, u64), + (ImmutableString, Position), + )>, + ), /// { [statement][Stmt] ... } Stmt(Box), /// func `(` expr `,` ... `)` @@ -1780,7 +1786,7 @@ impl fmt::Debug for Expr { } f.write_str(")") } - Self::Property(x) => write!(f, "Property({})", x.2.name), + Self::Property(x) => write!(f, "Property({})", (x.2).0), Self::Stmt(x) => { f.write_str("Stmt")?; f.debug_list().entries(x.0.iter()).finish() @@ -1896,7 +1902,7 @@ impl Expr { Self::InterpolatedString(x) => x.first().unwrap().position(), - Self::Property(x) => (x.2).pos, + Self::Property(x) => (x.2).1, Self::Stmt(x) => x.1, Self::And(x, _) | Self::Or(x, _) | Self::Dot(x, _) | Self::Index(x, _) => { @@ -1931,7 +1937,7 @@ impl Expr { x.first_mut().unwrap().set_position(new_pos); } - Self::Property(x) => (x.2).pos = new_pos, + Self::Property(x) => (x.2).1 = new_pos, Self::Stmt(x) => x.1 = new_pos, } diff --git a/src/engine.rs b/src/engine.rs index 948278c7..09314457 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -224,9 +224,9 @@ pub const KEYWORD_GLOBAL: &str = "global"; pub const FN_GET: &str = "get$"; #[cfg(not(feature = "no_object"))] pub const FN_SET: &str = "set$"; -#[cfg(not(feature = "no_index"))] +#[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] pub const FN_IDX_GET: &str = "index$get$"; -#[cfg(not(feature = "no_index"))] +#[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] pub const FN_IDX_SET: &str = "index$set$"; #[cfg(not(feature = "no_function"))] pub const FN_ANONYMOUS: &str = "anon$"; @@ -1256,7 +1256,7 @@ impl Engine { } // {xxx:map}.id op= ??? Expr::Property(x) if target.is::() && new_val.is_some() => { - let Ident { name, pos, .. } = &x.2; + let (name, pos) = &x.2; let index = name.into(); let val = self.get_indexed_mut( mods, state, lib, target, index, *pos, true, is_ref, false, level, @@ -1269,7 +1269,7 @@ impl Engine { } // {xxx:map}.id Expr::Property(x) if target.is::() => { - let Ident { name, pos, .. } = &x.2; + let (name, pos) = &x.2; let index = name.into(); let val = self.get_indexed_mut( mods, state, lib, target, index, *pos, false, is_ref, false, level, @@ -1279,8 +1279,7 @@ impl Engine { } // xxx.id op= ??? Expr::Property(x) if new_val.is_some() => { - let ((getter, hash_get), (setter, hash_set), Ident { pos, .. }) = - x.as_ref(); + let ((getter, hash_get), (setter, hash_set), (name, pos)) = x.as_ref(); let ((mut new_val, new_pos), (op_info, op_pos)) = new_val.unwrap(); if op_info.is_some() { @@ -1303,24 +1302,66 @@ impl Engine { mods, state, lib, setter, hash, &mut args, is_ref, true, *pos, None, level, ) - .map(|(v, _)| (v, true)) + .or_else(|err| match *err { + // Try an indexer if property does not exist + EvalAltResult::ErrorDotExpr(_, _) => { + let mut prop = name.into(); + let args = &mut [target, &mut prop, &mut new_val]; + let hash_set = FnCallHashes::from_native(crate::calc_fn_hash( + std::iter::empty(), + FN_IDX_SET, + 3, + )); + self.exec_fn_call( + mods, state, lib, FN_IDX_SET, hash_set, args, is_ref, true, + *pos, None, level, + ) + .map_err( + |idx_err| match *idx_err { + EvalAltResult::ErrorIndexingType(_, _) => err, + _ => idx_err, + }, + ) + } + _ => Err(err), + }) } // xxx.id Expr::Property(x) => { - let ((getter, hash_get), _, Ident { pos, .. }) = x.as_ref(); + let ((getter, hash_get), _, (name, pos)) = x.as_ref(); let hash = FnCallHashes::from_native(*hash_get); let mut args = [target.as_mut()]; self.exec_fn_call( mods, state, lib, getter, hash, &mut args, is_ref, true, *pos, None, level, ) - .map(|(v, _)| (v, false)) + .map_or_else( + |err| match *err { + // Try an indexer if property does not exist + EvalAltResult::ErrorDotExpr(_, _) => { + let prop = name.into(); + self.get_indexed_mut( + mods, state, lib, target, prop, *pos, false, is_ref, true, + level, + ) + .map(|v| (v.take_or_clone(), false)) + .map_err(|idx_err| { + match *idx_err { + EvalAltResult::ErrorIndexingType(_, _) => err, + _ => idx_err, + } + }) + } + _ => Err(err), + }, + |(v, _)| Ok((v, false)), + ) } // {xxx:map}.sub_lhs[expr] | {xxx:map}.sub_lhs.expr Expr::Index(x, x_pos) | Expr::Dot(x, x_pos) if target.is::() => { let mut val = match &x.lhs { Expr::Property(p) => { - let Ident { name, pos, .. } = &p.2; + let (name, pos) = &p.2; let index = name.into(); self.get_indexed_mut( mods, state, lib, target, index, *pos, false, is_ref, true, @@ -1356,17 +1397,36 @@ impl Engine { match &x.lhs { // xxx.prop[expr] | xxx.prop.expr Expr::Property(p) => { - let ((getter, hash_get), (setter, hash_set), Ident { pos, .. }) = + let ((getter, hash_get), (setter, hash_set), (name, pos)) = p.as_ref(); let rhs_chain = rhs_chain.unwrap(); let hash_get = FnCallHashes::from_native(*hash_get); let hash_set = FnCallHashes::from_native(*hash_set); - let arg_values = &mut [target.as_mut(), &mut Default::default()]; + let mut arg_values = [target.as_mut(), &mut Default::default()]; let args = &mut arg_values[..1]; - let (mut val, updated) = self.exec_fn_call( - mods, state, lib, getter, hash_get, args, is_ref, true, *pos, - None, level, - )?; + let (mut val, updated) = self + .exec_fn_call( + mods, state, lib, getter, hash_get, args, is_ref, true, + *pos, None, level, + ) + .or_else(|err| match *err { + // Try an indexer if property does not exist + EvalAltResult::ErrorDotExpr(_, _) => { + let prop = name.into(); + self.get_indexed_mut( + mods, state, lib, target, prop, *pos, false, + is_ref, true, level, + ) + .map(|v| (v.take_or_clone(), false)) + .map_err( + |idx_err| match *idx_err { + EvalAltResult::ErrorIndexingType(_, _) => err, + _ => idx_err, + }, + ) + } + _ => Err(err), + })?; let val = &mut val; @@ -1389,17 +1449,36 @@ impl Engine { // Feed the value back via a setter just in case it has been updated if updated || may_be_changed { // Re-use args because the first &mut parameter will not be consumed - arg_values[1] = val; + let mut arg_values = [target.as_mut(), val]; + let args = &mut arg_values; self.exec_fn_call( - mods, state, lib, setter, hash_set, arg_values, is_ref, - true, *pos, None, level, + mods, state, lib, setter, hash_set, args, is_ref, true, + *pos, None, level, ) .or_else( |err| match *err { - // If there is no setter, no need to feed it back because - // the property is read-only + // Try an indexer if property does not exist EvalAltResult::ErrorDotExpr(_, _) => { - Ok((Dynamic::UNIT, false)) + let mut prop = name.into(); + let args = &mut [target.as_mut(), &mut prop, val]; + let hash_set = + FnCallHashes::from_native(crate::calc_fn_hash( + std::iter::empty(), + FN_IDX_SET, + 3, + )); + self.exec_fn_call( + mods, state, lib, FN_IDX_SET, hash_set, args, + is_ref, true, *pos, None, level, + ) + .or_else(|idx_err| match *idx_err { + EvalAltResult::ErrorIndexingType(_, _) => { + // If there is no setter, no need to feed it back because + // the property is read-only + Ok((Dynamic::UNIT, false)) + } + _ => Err(idx_err), + }) } _ => Err(err), }, @@ -1550,7 +1629,7 @@ impl Engine { #[cfg(not(feature = "no_object"))] Expr::Property(x) if _parent_chain_type == ChainType::Dot => { - idx_values.push(ChainArgument::Property(x.2.pos)) + idx_values.push(ChainArgument::Property((x.2).1)) } Expr::Property(_) => unreachable!("unexpected Expr::Property for indexing"), @@ -1561,7 +1640,7 @@ impl Engine { let lhs_val = match lhs { #[cfg(not(feature = "no_object"))] Expr::Property(x) if _parent_chain_type == ChainType::Dot => { - ChainArgument::Property(x.2.pos) + ChainArgument::Property((x.2).1) } Expr::Property(_) => unreachable!("unexpected Expr::Property for indexing"), @@ -1748,7 +1827,6 @@ impl Engine { #[cfg(not(feature = "no_index"))] _ if _indexers => { - let type_name = target.type_name(); let args = &mut [target, &mut _idx]; let hash_get = FnCallHashes::from_native(calc_fn_hash(std::iter::empty(), FN_IDX_GET, 2)); @@ -1758,15 +1836,6 @@ impl Engine { _level, ) .map(|(v, _)| v.into()) - .map_err(|err| match *err { - EvalAltResult::ErrorFunctionNotFound(fn_sig, _) if fn_sig.ends_with(']') => { - Box::new(EvalAltResult::ErrorIndexingType( - type_name.into(), - Position::NONE, - )) - } - _ => err, - }) } _ => EvalAltResult::ErrorIndexingType( diff --git a/src/engine_api.rs b/src/engine_api.rs index 56f58522..0b1a0784 100644 --- a/src/engine_api.rs +++ b/src/engine_api.rs @@ -534,7 +534,7 @@ impl Engine { /// /// The function signature must start with `&mut self` and not `&self`. /// - /// Not available under `no_index`. + /// Not available under both `no_index` and `no_object`. /// /// # Panics /// @@ -574,12 +574,13 @@ impl Engine { /// # Ok(()) /// # } /// ``` - #[cfg(not(feature = "no_index"))] + #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] #[inline(always)] pub fn register_indexer_get( &mut self, get_fn: impl Fn(&mut T, X) -> V + SendSync + 'static, ) -> &mut Self { + #[cfg(not(feature = "no_index"))] if TypeId::of::() == TypeId::of::() { panic!("Cannot register indexer for arrays."); } @@ -600,7 +601,7 @@ impl Engine { /// /// The function signature must start with `&mut self` and not `&self`. /// - /// Not available under `no_index`. + /// Not available under both `no_index` and `no_object`. /// /// # Panics /// @@ -642,7 +643,7 @@ impl Engine { /// # Ok(()) /// # } /// ``` - #[cfg(not(feature = "no_index"))] + #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] #[inline(always)] pub fn register_indexer_get_result< T: Variant + Clone, @@ -652,6 +653,7 @@ impl Engine { &mut self, get_fn: impl Fn(&mut T, X) -> Result> + SendSync + 'static, ) -> &mut Self { + #[cfg(not(feature = "no_index"))] if TypeId::of::() == TypeId::of::() { panic!("Cannot register indexer for arrays."); } @@ -670,7 +672,7 @@ impl Engine { } /// Register an index setter for a custom type with the [`Engine`]. /// - /// Not available under `no_index`. + /// Not available under both `no_index` and `no_object`. /// /// # Panics /// @@ -712,12 +714,13 @@ impl Engine { /// # Ok(()) /// # } /// ``` - #[cfg(not(feature = "no_index"))] + #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] #[inline(always)] pub fn register_indexer_set( &mut self, set_fn: impl Fn(&mut T, X, V) + SendSync + 'static, ) -> &mut Self { + #[cfg(not(feature = "no_index"))] if TypeId::of::() == TypeId::of::() { panic!("Cannot register indexer for arrays."); } @@ -736,7 +739,7 @@ impl Engine { } /// Register an index setter for a custom type with the [`Engine`]. /// - /// Not available under `no_index`. + /// Not available under both `no_index` and `no_object`. /// /// # Panics /// @@ -781,7 +784,7 @@ impl Engine { /// # Ok(()) /// # } /// ``` - #[cfg(not(feature = "no_index"))] + #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] #[inline(always)] pub fn register_indexer_set_result< T: Variant + Clone, @@ -791,6 +794,7 @@ impl Engine { &mut self, set_fn: impl Fn(&mut T, X, V) -> Result<(), Box> + SendSync + 'static, ) -> &mut Self { + #[cfg(not(feature = "no_index"))] if TypeId::of::() == TypeId::of::() { panic!("Cannot register indexer for arrays."); } @@ -809,7 +813,7 @@ impl Engine { } /// Short-hand for registering both index getter and setter functions for a custom type with the [`Engine`]. /// - /// Not available under `no_index`. + /// Not available under both `no_index` and `no_object`. /// /// # Panics /// @@ -851,7 +855,7 @@ impl Engine { /// # Ok(()) /// # } /// ``` - #[cfg(not(feature = "no_index"))] + #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] #[inline(always)] pub fn register_indexer_get_set( &mut self, diff --git a/src/fn_call.rs b/src/fn_call.rs index 0c087b2e..c10f4b0d 100644 --- a/src/fn_call.rs +++ b/src/fn_call.rs @@ -375,12 +375,8 @@ impl Engine { crate::engine::FN_IDX_GET => { assert!(args.len() == 2); - EvalAltResult::ErrorFunctionNotFound( - format!( - "{} [{}]", - self.map_type_name(args[0].type_name()), - self.map_type_name(args[1].type_name()), - ), + EvalAltResult::ErrorIndexingType( + self.map_type_name(args[0].type_name()).to_string(), pos, ) .into() @@ -391,12 +387,8 @@ impl Engine { crate::engine::FN_IDX_SET => { assert!(args.len() == 3); - EvalAltResult::ErrorFunctionNotFound( - format!( - "{} [{}]=", - self.map_type_name(args[0].type_name()), - self.map_type_name(args[1].type_name()), - ), + EvalAltResult::ErrorIndexingType( + self.map_type_name(args[0].type_name()).to_string(), pos, ) .into() diff --git a/src/module/mod.rs b/src/module/mod.rs index 25830f17..2b531e9e 100644 --- a/src/module/mod.rs +++ b/src/module/mod.rs @@ -24,7 +24,6 @@ use std::{ #[cfg(not(feature = "no_index"))] use crate::Array; -#[cfg(not(feature = "no_index"))] #[cfg(not(feature = "no_object"))] use crate::Map; @@ -956,7 +955,7 @@ impl Module { /// }); /// assert!(module.contains_fn(hash)); /// ``` - #[cfg(not(feature = "no_index"))] + #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] #[inline(always)] pub fn set_indexer_get_fn(&mut self, func: F) -> u64 where @@ -966,6 +965,7 @@ impl Module { F: RegisterNativeFunction>>, F: Fn(&mut A, B) -> Result> + SendSync + 'static, { + #[cfg(not(feature = "no_index"))] if TypeId::of::() == TypeId::of::() { panic!("Cannot register indexer for arrays."); } @@ -1016,7 +1016,7 @@ impl Module { /// }); /// assert!(module.contains_fn(hash)); /// ``` - #[cfg(not(feature = "no_index"))] + #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] #[inline(always)] pub fn set_indexer_set_fn(&mut self, func: F) -> u64 where @@ -1026,6 +1026,7 @@ impl Module { F: RegisterNativeFunction>>, F: Fn(&mut A, B, C) -> Result<(), Box> + SendSync + 'static, { + #[cfg(not(feature = "no_index"))] if TypeId::of::() == TypeId::of::() { panic!("Cannot register indexer for arrays."); } @@ -1082,7 +1083,7 @@ impl Module { /// assert!(module.contains_fn(hash_get)); /// assert!(module.contains_fn(hash_set)); /// ``` - #[cfg(not(feature = "no_index"))] + #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] #[inline(always)] pub fn set_indexer_get_set_fn( &mut self, diff --git a/src/optimize.rs b/src/optimize.rs index 74a4c7ef..20b1cc4a 100644 --- a/src/optimize.rs +++ b/src/optimize.rs @@ -696,7 +696,7 @@ fn optimize_expr(expr: &mut Expr, state: &mut State) { Expr::Dot(x, _) => match (&mut x.lhs, &mut x.rhs) { // map.string (Expr::Map(m, pos), Expr::Property(p)) if m.0.iter().all(|(_, x)| x.is_pure()) => { - let prop = &p.2.name; + let prop = p.2.0.as_str(); // Map literal where everything is pure - promote the indexed item. // All other items can be thrown away. state.set_dirty(); diff --git a/src/parser.rs b/src/parser.rs index 97b61d24..8539960c 100644 --- a/src/parser.rs +++ b/src/parser.rs @@ -232,10 +232,7 @@ impl Expr { Self::Property(Box::new(( (getter, hash_get), (setter, hash_set), - Ident { - name: state.get_identifier(ident), - pos, - }, + (state.get_identifier(ident).into(), pos), ))) } _ => self, @@ -1541,10 +1538,7 @@ fn make_dot_expr( let rhs = Expr::Property(Box::new(( (getter, hash_get), (setter, hash_set), - Ident { - name: state.get_identifier(ident), - pos: var_pos, - }, + (state.get_identifier(ident).into(), var_pos), ))); Expr::Dot(Box::new(BinaryExpr { lhs, rhs }), op_pos) diff --git a/tests/get_set.rs b/tests/get_set.rs index a55f5854..aa702be1 100644 --- a/tests/get_set.rs +++ b/tests/get_set.rs @@ -1,6 +1,6 @@ #![cfg(not(feature = "no_object"))] -use rhai::{Engine, EvalAltResult, ImmutableString, INT}; +use rhai::{Engine, EvalAltResult, INT}; #[test] fn test_get_set() -> Result<(), Box> { @@ -46,20 +46,27 @@ fn test_get_set() -> Result<(), Box> { assert_eq!(engine.eval::("let a = new_ts(); a.x.add(); a.x")?, 42); assert_eq!(engine.eval::("let a = new_ts(); a.y.add(); a.y")?, 0); - #[cfg(not(feature = "no_index"))] + #[cfg(any(not(feature = "no_index"), not(feature = "no_object")))] { engine.register_indexer_get_set( - |value: &mut TestStruct, index: ImmutableString| value.array[index.len()], - |value: &mut TestStruct, index: ImmutableString, new_val: INT| { - value.array[index.len()] = new_val - }, + |value: &mut TestStruct, index: &str| value.array[index.len()], + |value: &mut TestStruct, index: &str, new_val: INT| value.array[index.len()] = new_val, ); + #[cfg(not(feature = "no_index"))] assert_eq!(engine.eval::(r#"let a = new_ts(); a["abc"]"#)?, 4); + + #[cfg(not(feature = "no_index"))] assert_eq!( engine.eval::(r#"let a = new_ts(); a["abc"] = 42; a["abc"]"#)?, 42 ); + + assert_eq!(engine.eval::(r"let a = new_ts(); a.abc")?, 4); + assert_eq!( + engine.eval::(r"let a = new_ts(); a.abc = 42; a.abc")?, + 42 + ); } Ok(()) }