Avoid cloning on optimization.

CHANGELOG.md

@@ -25,6 +25,7 @@ Deprecated API's
 Enhancements
 ------------
 
+* Avoid cloning values unless needed when performing constants propagation in optimization.
 * Added `to_int` method for characters.
 * `Token::FloatConstant` and `Token::DecimalConstant` now carry the original text representation for use in, say, a _token mapper_.
 * `Dynamic::is_fnptr` is made a public API.

src/optimizer.rs

@@ -22,6 +22,7 @@ use crate::{
 use std::prelude::v1::*;
 use std::{
     any::TypeId,
+    borrow::Cow,
     convert::TryFrom,
     hash::{Hash, Hasher},
     mem,
@@ -49,12 +50,14 @@ pub enum OptimizationLevel {
 struct OptimizerState<'a> {
     /// Has the [`AST`] been changed during this pass?
     is_dirty: bool,
-    /// Stack of variables/constants for constants propagation.
-    variables: Vec<(ImmutableString, Option<Dynamic>)>,
+    /// Stack of variables/constants for constants propagation and strict variables checking.
+    variables: Vec<(ImmutableString, Option<Cow<'a, Dynamic>>)>,
     /// Activate constants propagation?
     propagate_constants: bool,
     /// [`Engine`] instance for eager function evaluation.
     engine: &'a Engine,
+    /// Optional [`Scope`].
+    scope: Option<&'a Scope<'a>>,
     /// The global runtime state.
     global: GlobalRuntimeState,
     /// Function resolution caches.
@@ -69,6 +72,7 @@ impl<'a> OptimizerState<'a> {
     pub fn new(
         engine: &'a Engine,
         lib: &'a [crate::SharedModule],
+        scope: Option<&'a Scope<'a>>,
         optimization_level: OptimizationLevel,
     ) -> Self {
         let mut _global = GlobalRuntimeState::new(engine);
@@ -84,6 +88,7 @@ impl<'a> OptimizerState<'a> {
             variables: Vec::new(),
             propagate_constants: true,
             engine,
+            scope,
             global: _global,
             caches: Caches::new(),
             optimization_level,
@@ -113,7 +118,7 @@ impl<'a> OptimizerState<'a> {
     ///
     /// `Some(value)` if literal constant (which can be used for constants propagation), `None` otherwise.
     #[inline(always)]
-    pub fn push_var(&mut self, name: ImmutableString, value: Option<Dynamic>) {
+    pub fn push_var<'x: 'a>(&mut self, name: ImmutableString, value: Option<Cow<'x, Dynamic>>) {
         self.variables.push((name, value));
     }
     /// Look up a literal constant from the variables stack.
@@ -123,7 +128,7 @@ impl<'a> OptimizerState<'a> {
             .iter()
             .rev()
             .find(|(n, _)| n == name)
-            .and_then(|(_, value)| value.as_ref())
+            .and_then(|(_, value)| value.as_deref())
     }
     /// Call a registered function
     #[inline]
@@ -220,7 +225,7 @@ fn optimize_stmt_block(
 
                     let value = if options.contains(ASTFlags::CONSTANT) && x.1.is_constant() {
                         // constant literal
-                        Some(x.1.get_literal_value().unwrap())
+                        Some(Cow::Owned(x.1.get_literal_value().unwrap()))
                     } else {
                         // variable
                         None
@@ -1327,21 +1332,29 @@ impl Engine {
         }
 
         // Set up the state
-        let mut state = OptimizerState::new(self, lib, optimization_level);
+        let mut state = OptimizerState::new(self, lib, scope, optimization_level);
 
         // Add constants from global modules
         self.global_modules
             .iter()
             .rev()
             .flat_map(|m| m.iter_var())
-            .for_each(|(name, value)| state.push_var(name.into(), Some(value.clone())));
+            .for_each(|(name, value)| state.push_var(name.into(), Some(Cow::Borrowed(value))));
 
         // Add constants and variables from the scope
-        scope
+        state
+            .scope
             .into_iter()
-            .flat_map(Scope::iter)
+            .flat_map(Scope::iter_inner)
             .for_each(|(name, constant, value)| {
-                state.push_var(name.into(), if constant { Some(value) } else { None });
+                state.push_var(
+                    name.into(),
+                    if constant {
+                        Some(Cow::Borrowed(value))
+                    } else {
+                        None
+                    },
+                );
             });
 
         optimize_stmt_block(statements, &mut state, true, false, true)

src/parser.rs

@@ -153,9 +153,8 @@ impl<'e, 's> ParseState<'e, 's> {
 
         let index = self
             .stack
-            .iter_rev_raw()
-            .enumerate()
-            .find(|&(.., (n, ..))| {
+            .iter_rev_inner()
+            .position(|(n, ..)| {
                 if n == SCOPE_SEARCH_BARRIER_MARKER {
                     // Do not go beyond the barrier
                     hit_barrier = true;
@@ -164,7 +163,7 @@ impl<'e, 's> ParseState<'e, 's> {
                     n == name
                 }
             })
-            .map_or(0, |(i, ..)| i + 1);
+            .map_or(0, |i| i + 1);
 
         (index, hit_barrier)
     }

src/types/scope.rs

@@ -727,18 +727,15 @@ impl Scope<'_> {
     /// Panics if the index is out of bounds.
     #[inline(always)]
     #[allow(dead_code)]
-    pub(crate) fn get_entry_by_index(
-        &mut self,
-        index: usize,
-    ) -> (&str, &Dynamic, &[ImmutableString]) {
-        if self.aliases.len() <= index {
-            self.aliases.resize(index + 1, <_>::default());
-        }
-
+    pub(crate) fn get_entry_by_index(&self, index: usize) -> (&str, &Dynamic, &[ImmutableString]) {
         (
             &self.names[index],
             &self.values[index],
-            &self.aliases[index],
+            if self.aliases.len() > index {
+                &self.aliases[index]
+            } else {
+                &[]
+            },
         )
     }
     /// Remove the last entry in the [`Scope`] by the specified name and return its value.
@@ -929,15 +926,26 @@ impl Scope<'_> {
             .zip(self.values.iter())
             .map(|(name, value)| (name.as_str(), value.is_read_only(), value))
     }
+    /// Get an iterator to entries in the [`Scope`].
+    /// Shared values are not expanded.
+    #[inline]
+    pub(crate) fn iter_inner(&self) -> impl Iterator<Item = (&ImmutableString, bool, &Dynamic)> {
+        self.names
+            .iter()
+            .zip(self.values.iter())
+            .map(|(name, value)| (name, value.is_read_only(), value))
+    }
     /// Get a reverse iterator to entries in the [`Scope`].
     /// Shared values are not expanded.
     #[inline]
-    pub(crate) fn iter_rev_raw(&self) -> impl Iterator<Item = (&str, bool, &Dynamic)> {
+    pub(crate) fn iter_rev_inner(
+        &self,
+    ) -> impl Iterator<Item = (&ImmutableString, bool, &Dynamic)> {
         self.names
             .iter()
             .rev()
             .zip(self.values.iter().rev())
-            .map(|(name, value)| (name.as_str(), value.is_read_only(), value))
+            .map(|(name, value)| (name, value.is_read_only(), value))
     }
     /// Remove a range of entries within the [`Scope`].
     ///

tests/var_scope.rs

@@ -397,3 +397,25 @@ fn test_var_def_filter() {
     assert!(engine.run("let y = 42; { let x = y + 1; }").is_err());
     engine.run("let y = 42; { let z = y + 1; { let x = z + 1; } }").unwrap();
 }
+
+#[test]
+fn test_var_scope_cloning() {
+    struct Foo {
+        field: INT,
+    }
+
+    impl Clone for Foo {
+        fn clone(&self) -> Self {
+            panic!("forbidden to clone!");
+        }
+    }
+
+    let mut engine = Engine::new();
+    engine.register_get_set("field", |foo: &mut Foo| foo.field, |foo: &mut Foo, value| foo.field = value);
+
+    let mut scope = Scope::new();
+    scope.push("foo", Foo { field: 1 });
+
+    engine.run_with_scope(&mut scope, "let x = 42; print(x + foo.field);").unwrap();
+    assert_eq!(engine.eval_with_scope::<INT>(&mut scope, "let x = 42; x + foo.field").unwrap(), 43);
+}