From 5a7547911980a8ef5d35bb78e5c9828c9c88d3c4 Mon Sep 17 00:00:00 2001
From: Stephen Chung <schungx@live.com>
Date: Sat, 23 Apr 2022 11:53:17 +0800
Subject: [PATCH] Fix bug in Scope cloning.

---
 CHANGELOG.md       |  1 +
 src/types/scope.rs | 80 ++++++++++++++++++++++++++++++++++++----------
 tests/var_scope.rs | 11 +++++++
 3 files changed, 75 insertions(+), 17 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index a03e5fda..8002c5a7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@ Bug fixes
 ---------
 
 * Compound assignments now work properly with indexers.
+* Cloning a `Scope` no longer turns all constants to mutable.
 
 Script-breaking changes
 -----------------------
diff --git a/src/types/scope.rs b/src/types/scope.rs
index bae3f2a8..6a169bec 100644
--- a/src/types/scope.rs
+++ b/src/types/scope.rs
@@ -6,6 +6,7 @@ use smallvec::SmallVec;
 #[cfg(feature = "no_std")]
 use std::prelude::v1::*;
 use std::{
+    fmt,
     iter::{Extend, FromIterator},
     marker::PhantomData,
 };
@@ -59,7 +60,7 @@ const SCOPE_ENTRIES_INLINED: usize = 8;
 // direct indexing, by-passing the name altogether.
 //
 // [`Dynamic`] is reasonably small so packing it tightly improves cache performance.
-#[derive(Debug, Clone, Hash, Default)]
+#[derive(Debug, Hash, Default)]
 pub struct Scope<'a> {
     /// Current value of the entry.
     values: SmallVec<[Dynamic; SCOPE_ENTRIES_INLINED]>,
@@ -71,6 +72,51 @@ pub struct Scope<'a> {
     phantom: PhantomData<&'a ()>,
 }
 
+impl fmt::Display for Scope<'_> {
+    #[inline]
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        for (i, (name, constant, value)) in self.iter_raw().enumerate() {
+            #[cfg(not(feature = "no_closure"))]
+            let value_is_shared = if value.is_shared() { " (shared)" } else { "" };
+            #[cfg(feature = "no_closure")]
+            let value_is_shared = "";
+
+            write!(
+                f,
+                "[{}] {}{}{} = {:?}\n",
+                i + 1,
+                if constant { "const " } else { "" },
+                name,
+                value_is_shared,
+                *value.read_lock::<Dynamic>().unwrap(),
+            )?;
+        }
+
+        Ok(())
+    }
+}
+
+impl Clone for Scope<'_> {
+    #[inline]
+    fn clone(&self) -> Self {
+        Self {
+            values: self
+                .values
+                .iter()
+                .map(|v| {
+                    // Also copy the value's access mode (otherwise will turn to read-write)
+                    let mut v2 = v.clone();
+                    v2.set_access_mode(v.access_mode());
+                    v2
+                })
+                .collect(),
+            names: self.names.clone(),
+            aliases: self.aliases.clone(),
+            phantom: self.phantom.clone(),
+        }
+    }
+}
+
 impl IntoIterator for Scope<'_> {
     type Item = (String, Dynamic, Vec<Identifier>);
     type IntoIter = Box<dyn Iterator<Item = Self::Item>>;
@@ -551,24 +597,24 @@ impl Scope<'_> {
     #[must_use]
     pub fn clone_visible(&self) -> Self {
         let len = self.len();
+        let mut scope = Self::new();
 
-        self.names
-            .iter()
-            .rev()
-            .enumerate()
-            .fold(Self::new(), |mut entries, (index, name)| {
-                if entries.names.is_empty() || !entries.names.contains(name) {
-                    let orig_value = &self.values[len - 1 - index];
-                    let alias = &self.aliases[len - 1 - index];
-                    let mut value = orig_value.clone();
-                    value.set_access_mode(orig_value.access_mode());
+        self.names.iter().rev().enumerate().for_each(|(i, name)| {
+            if scope.names.contains(name) {
+                return;
+            }
 
-                    entries.names.push(name.clone());
-                    entries.values.push(value);
-                    entries.aliases.push(alias.clone());
-                }
-                entries
-            })
+            let v1 = &self.values[len - 1 - i];
+            let alias = &self.aliases[len - 1 - i];
+            let mut v2 = v1.clone();
+            v2.set_access_mode(v1.access_mode());
+
+            scope.names.push(name.clone());
+            scope.values.push(v2);
+            scope.aliases.push(alias.clone());
+        });
+
+        scope
     }
     /// Get an iterator to entries in the [`Scope`].
     #[inline]
diff --git a/tests/var_scope.rs b/tests/var_scope.rs
index 2f50c457..014ec42b 100644
--- a/tests/var_scope.rs
+++ b/tests/var_scope.rs
@@ -79,6 +79,17 @@ fn test_var_scope() -> Result<(), Box<EvalAltResult>> {
         );
     }
 
+    scope.clear();
+
+    scope.push("x", 42 as INT);
+    scope.push_constant("x", 42 as INT);
+
+    let scope2 = scope.clone();
+    let scope3 = scope.clone_visible();
+
+    assert_eq!(scope2.is_constant("x"), Some(true));
+    assert_eq!(scope3.is_constant("x"), Some(true));
+
     Ok(())
 }