Reduce indirections.

CHANGELOG.md

@@ -4,21 +4,23 @@ Rhai Release Notes
 Version 0.19.15
 ===============
 
-This version replaces internal usage of `HashMap` with `BTreeMap` in many cases, which should result
+This version replaces all internal usage of `HashMap` with `BTreeMap`, which should result
-in speed improvements because a `BTreeMap` is faster when the number of items held is small.
+in some speed improvement because a `BTreeMap` is leaner when the number of items held is small.
+Most, if not all, collections in Rhai hold very few data items, so this is a typical scenario of
+many tiny-sized collections.
 
-This also translates to the Rhai object map type, `Map`, which used to be an alias to `HashMap` and
+The Rhai object map type, `Map`, used to be an alias to `HashMap` and is now aliased to `BTreeMap`
-is now aliased to `BTreeMap` instead.  This change is due to the fact that, in the vast majority of
+instead. This is also because, in the vast majority of usage cases, the number of properties held by
-object map usages, the number of properties held is small.
+an object map is small.
 
-`HashMap` and `BTreeMap` have almost identical public API's so this change is unlikely to break a
+`HashMap` and `BTreeMap` have almost identical public API's so this change is unlikely to break
-lot of existing code.
+existing code.
 
 
 Breaking changes
 ----------------
 
-* `Map` is now an alias to `BTreeMap` instead of `HashMap`.  This is because most object maps used have few properties.
+* `Map` is now an alias to `BTreeMap` instead of `HashMap` because most object maps hold few properties.
 * The traits `RegisterFn` and `RegisterResultFn` are removed.  `Engine::register_fn` and `Engine::register_result_fn` are now implemented directly on `Engine`.
 * `FnPtr::call_dynamic` now takes `&NativeCallContext` instead of consuming it.
 * All `Module::set_fn_XXX` methods are removed, in favor of `Module::set_native_fn`.
@@ -28,7 +30,7 @@ Breaking changes
 Enhancements
 ------------
 
-* Replaced most `HashMap` usage with `BTreeMap` for better performance when the number of items is small.
+* Replaced all `HashMap` usage with `BTreeMap` for better performance because collections in Rhai are tiny.
 * `Engine::register_result_fn` no longer requires the successful return type to be `Dynamic`.  It can now be any clonable type.
 * `#[rhai_fn(return_raw)]` can now return `Result<T, Box<EvalAltResult>>` where `T` is any clonable type instead of `Result<Dynamic, Box<EvalAltResult>>`.
 

src/ast.rs

@@ -1285,7 +1285,7 @@ pub struct BinaryExpr {
 pub struct OpAssignment {
     pub hash_op_assign: u64,
     pub hash_op: u64,
-    pub op: Cow<'static, str>,
+    pub op: &'static str,
 }
 
 /// _(INTERNALS)_ An set of function call hashes.

src/engine.rs

@@ -342,13 +342,13 @@ impl<'a> Target<'a> {
     #[inline(always)]
     pub fn is<T: Variant + Clone>(&self) -> bool {
         match self {
-            Target::Ref(r) => r.is::<T>(),
+            Self::Ref(r) => r.is::<T>(),
             #[cfg(not(feature = "no_closure"))]
             #[cfg(not(feature = "no_object"))]
-            Target::LockGuard((r, _)) => r.is::<T>(),
+            Self::LockGuard((r, _)) => r.is::<T>(),
-            Target::Value(r) => r.is::<T>(),
+            Self::Value(r) => r.is::<T>(),
             #[cfg(not(feature = "no_index"))]
-            Target::StringChar(_, _, _) => TypeId::of::<T>() == TypeId::of::<char>(),
+            Self::StringChar(_, _, _) => TypeId::of::<T>() == TypeId::of::<char>(),
         }
     }
     /// Get the value of the `Target` as a `Dynamic`, cloning a referenced value if necessary.
@@ -1056,7 +1056,7 @@ impl Engine {
         idx_values: &mut StaticVec<ChainArgument>,
         chain_type: ChainType,
         level: usize,
-        new_val: Option<((Dynamic, Position), (&Option<OpAssignment>, Position))>,
+        new_val: Option<((Dynamic, Position), (Option<OpAssignment>, Position))>,
     ) -> Result<(Dynamic, bool), Box<EvalAltResult>> {
         assert!(chain_type != ChainType::NonChaining);
 
@@ -1097,6 +1097,8 @@ impl Engine {
                     // xxx[rhs] op= new_val
                     _ if new_val.is_some() => {
                         let idx_val = idx_val.as_index_value();
+
+                        #[cfg(not(feature = "no_index"))]
                         let mut idx_val2 = idx_val.clone();
 
                         // `call_setter` is introduced to bypass double mutable borrowing of target
@@ -1357,7 +1359,7 @@ impl Engine {
         this_ptr: &mut Option<&mut Dynamic>,
         expr: &Expr,
         level: usize,
-        new_val: Option<((Dynamic, Position), (&Option<OpAssignment>, Position))>,
+        new_val: Option<((Dynamic, Position), (Option<OpAssignment>, Position))>,
     ) -> RhaiResult {
         let (crate::ast::BinaryExpr { lhs, rhs }, chain_type, op_pos) = match expr {
             Expr::Index(x, pos) => (x.as_ref(), ChainType::Index, *pos),
@@ -1524,7 +1526,7 @@ impl Engine {
         state: &mut State,
         _lib: &[&Module],
         target: &'t mut Dynamic,
-        idx: Dynamic,
+        mut idx: Dynamic,
         idx_pos: Position,
         _create: bool,
         _is_ref: bool,
@@ -1557,21 +1559,18 @@ impl Engine {
             #[cfg(not(feature = "no_object"))]
             Dynamic(Union::Map(map, _)) => {
                 // val_map[idx]
-                Ok(if _create {
+                let index = &*idx.read_lock::<ImmutableString>().ok_or_else(|| {
-                    let index = idx.take_immutable_string().map_err(|err| {
+                    self.make_type_mismatch_err::<ImmutableString>(idx.type_name(), idx_pos)
-                        self.make_type_mismatch_err::<ImmutableString>(err, idx_pos)
                 })?;
 
-                    map.entry(index).or_insert_with(Default::default).into()
+                if _create && !map.contains_key(index) {
-                } else {
+                    map.insert(index.clone(), Default::default());
-                    let index = idx.read_lock::<ImmutableString>().ok_or_else(|| {
+                }
-                        self.make_type_mismatch_err::<ImmutableString>("", idx_pos)
-                    })?;
 
-                    map.get_mut(&*index)
+                Ok(map
+                    .get_mut(index)
                     .map(Target::from)
-                        .unwrap_or_else(|| Target::from(()))
+                    .unwrap_or_else(|| Target::from(())))
-                })
             }
 
             #[cfg(not(feature = "no_index"))]
@@ -1596,7 +1595,6 @@ impl Engine {
             #[cfg(not(feature = "no_index"))]
             _ if _indexers => {
                 let type_name = target.type_name();
-                let mut idx = idx;
                 let args = &mut [target, &mut idx];
                 let hash_get = FnCallHash::from_native(calc_fn_hash(empty(), FN_IDX_GET, 2));
                 self.exec_fn_call(
@@ -1863,7 +1861,7 @@ impl Engine {
         mods: &mut Imports,
         state: &mut State,
         lib: &[&Module],
-        op_info: &Option<OpAssignment>,
+        op_info: Option<OpAssignment>,
         op_pos: Position,
         mut target: Target,
         mut new_value: Dynamic,
@@ -1889,20 +1887,19 @@ impl Engine {
                 lhs_ptr_inner = target.as_mut();
             }
 
-            let hash = *hash_op_assign;
+            let hash = hash_op_assign;
             let args = &mut [lhs_ptr_inner, &mut new_value];
 
             match self.call_native_fn(mods, state, lib, op, hash, args, true, true, op_pos) {
                 Ok(_) => (),
-                Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op.as_ref())) =>
+                Err(err) if matches!(err.as_ref(), EvalAltResult::ErrorFunctionNotFound(f, _) if f.starts_with(op)) =>
                 {
                     // Expand to `var = var op rhs`
                     let op = &op[..op.len() - 1]; // extract operator without =
 
                     // Run function
-                    let (value, _) = self.call_native_fn(
+                    let (value, _) = self
-                        mods, state, lib, op, *hash_op, args, true, false, op_pos,
+                        .call_native_fn(mods, state, lib, op, hash_op, args, true, false, op_pos)?;
-                    )?;
 
                     *args[0] = value.flatten();
                 }
@@ -1975,7 +1972,7 @@ impl Engine {
                         mods,
                         state,
                         lib,
-                        op_info,
+                        op_info.clone(),
                         *op_pos,
                         lhs_ptr,
                         rhs_val,
@@ -1991,7 +1988,7 @@ impl Engine {
                 let rhs_val = self
                     .eval_expr(scope, mods, state, lib, this_ptr, rhs_expr, level)?
                     .flatten();
-                let _new_val = Some(((rhs_val, rhs_expr.position()), (op_info, *op_pos)));
+                let _new_val = Some(((rhs_val, rhs_expr.position()), (op_info.clone(), *op_pos)));
 
                 // Must be either `var[index] op= val` or `var.prop op= val`
                 match lhs_expr {

src/engine_api.rs

@@ -1278,9 +1278,9 @@ impl Engine {
 
         // Trims the JSON string and add a '#' in front
         let json_text = json.trim_start();
-        let scripts = if json_text.starts_with(Token::MapStart.syntax().as_ref()) {
+        let scripts = if json_text.starts_with(Token::MapStart.keyword_syntax()) {
             [json_text, ""]
-        } else if json_text.starts_with(Token::LeftBrace.syntax().as_ref()) {
+        } else if json_text.starts_with(Token::LeftBrace.keyword_syntax()) {
             ["#", json_text]
         } else {
             return Err(crate::ParseErrorType::MissingToken(

src/parser.rs

@@ -10,7 +10,6 @@ use crate::module::NamespaceRef;
 use crate::optimize::optimize_into_ast;
 use crate::optimize::OptimizationLevel;
 use crate::stdlib::{
-    borrow::Cow,
     boxed::Box,
     collections::BTreeMap,
     format,
@@ -1355,7 +1354,7 @@ fn parse_unary(
 
 /// Make an assignment statement.
 fn make_assignment_stmt<'a>(
-    op: Cow<'static, str>,
+    op: &'static str,
     state: &mut ParseState,
     lhs: Expr,
     rhs: Expr,
@@ -1479,7 +1478,7 @@ fn parse_op_assignment_stmt(
         | Token::PowerOfAssign
         | Token::AndAssign
         | Token::OrAssign
-        | Token::XOrAssign => token.syntax(),
+        | Token::XOrAssign => token.keyword_syntax(),
 
         _ => return Ok(Stmt::Expr(lhs)),
     };

src/token.rs

@@ -381,25 +381,15 @@ pub enum Token {
 }
 
 impl Token {
-    /// Get the syntax of the token.
+    /// Get the syntax of the token if it is a keyword.
-    pub fn syntax(&self) -> Cow<'static, str> {
+    ///
+    /// # Panics
+    ///
+    /// Panics if the token is not a keyword.
+    pub fn keyword_syntax(&self) -> &'static str {
         use Token::*;
 
         match self {
-            IntegerConstant(i) => i.to_string().into(),
-            #[cfg(not(feature = "no_float"))]
-            FloatConstant(f) => f.to_string().into(),
-            #[cfg(feature = "decimal")]
-            DecimalConstant(d) => d.to_string().into(),
-            StringConstant(_) => "string".into(),
-            CharConstant(c) => c.to_string().into(),
-            Identifier(s) => s.clone().into(),
-            Reserved(s) => s.clone().into(),
-            Custom(s) => s.clone().into(),
-            LexError(err) => err.to_string().into(),
-            Comment(s) => s.clone().into(),
-
-            token => match token {
             LeftBrace => "{",
             RightBrace => "}",
             LeftParen => "(",
@@ -479,10 +469,32 @@ impl Token {
             Export => "export",
             #[cfg(not(feature = "no_module"))]
             As => "as",
-                EOF => "{EOF}",
+
-                t => unreachable!("operator should be matched in outer scope: {:?}", t),
+            t => unreachable!("{:?} is not a keyword", t),
         }
-            .into(),
+    }
+
+    /// Get the syntax of the token.
+    pub fn syntax(&self) -> Cow<'static, str> {
+        use Token::*;
+
+        match self {
+            IntegerConstant(i) => i.to_string().into(),
+            #[cfg(not(feature = "no_float"))]
+            FloatConstant(f) => f.to_string().into(),
+            #[cfg(feature = "decimal")]
+            DecimalConstant(d) => d.to_string().into(),
+            StringConstant(_) => "string".into(),
+            CharConstant(c) => c.to_string().into(),
+            Identifier(s) => s.clone().into(),
+            Reserved(s) => s.clone().into(),
+            Custom(s) => s.clone().into(),
+            LexError(err) => err.to_string().into(),
+            Comment(s) => s.clone().into(),
+
+            EOF => "{EOF}".into(),
+
+            token => token.keyword_syntax().into(),
         }
     }