Pure functions!

4 files changed, 125 insertions, 133 deletions

diff --git a/src/eval/capture.rs b/src/eval/capture.rs
index 24fc7abc..1192eaa7 100644
--- a/src/eval/capture.rs
+++ b/src/eval/capture.rs
@@ -1,5 +1,3 @@
-use std::sync::Arc;
-
 use super::{Scope, Scopes, Value};
 use crate::syntax::ast::{ClosureParam, Expr, Ident, Imports, TypedNode};
 use crate::syntax::RedRef;
@@ -28,14 +26,14 @@ impl<'a> CapturesVisitor<'a> {
 
     /// Bind a new internal variable.
     pub fn bind(&mut self, ident: Ident) {
-        self.internal.top.def_mut(ident.take(), Value::None);
+        self.internal.top.define(ident.take(), Value::None);
     }
 
     /// Capture a variable if it isn't internal.
     pub fn capture(&mut self, ident: Ident) {
-        if self.internal.get(&ident).is_none() {
-            if let Some(slot) = self.external.get(&ident) {
-                self.captures.def_slot(ident.take(), Arc::clone(slot));
+        if self.internal.get(&ident).is_err() {
+            if let Ok(value) = self.external.get(&ident) {
+                self.captures.define_captured(ident.take(), value.clone());
             }
         }
     }
@@ -145,9 +143,9 @@ mod tests {
         let red = RedNode::from_root(green, SourceId::from_raw(0));
 
         let mut scopes = Scopes::new(None);
-        scopes.top.def_const("x", 0);
-        scopes.top.def_const("y", 0);
-        scopes.top.def_const("z", 0);
+        scopes.top.define("x", 0);
+        scopes.top.define("y", 0);
+        scopes.top.define("z", 0);
 
         let mut visitor = CapturesVisitor::new(&scopes);
         visitor.visit(red.as_ref());
diff --git a/src/eval/func.rs b/src/eval/func.rs
index f15549dc..12dbfb2e 100644
--- a/src/eval/func.rs
+++ b/src/eval/func.rs
@@ -206,7 +206,7 @@ impl Closure {
 
         // Parse the arguments according to the parameter list.
         for (param, default) in &self.params {
-            scopes.top.def_mut(param, match default {
+            scopes.top.define(param, match default {
                 None => args.expect::<Value>(param)?,
                 Some(default) => {
                     args.named::<Value>(param)?.unwrap_or_else(|| default.clone())
@@ -216,7 +216,7 @@ impl Closure {
 
         // Put the remaining arguments into the sink.
         if let Some(sink) = &self.sink {
-            scopes.top.def_mut(sink, args.take());
+            scopes.top.define(sink, args.take());
         }
 
         // Determine the route inside the closure.
diff --git a/src/eval/mod.rs b/src/eval/mod.rs
index d76257fa..db7595f9 100644
--- a/src/eval/mod.rs
+++ b/src/eval/mod.rs
@@ -31,7 +31,6 @@ pub use value::*;
 
 use std::collections::BTreeMap;
 
-use parking_lot::{MappedRwLockWriteGuard, RwLockWriteGuard};
 use unicode_segmentation::UnicodeSegmentation;
 
 use crate::diag::{At, StrResult, Trace, Tracepoint, TypResult};
@@ -165,7 +164,7 @@ fn eval_markup(
             }
             MarkupNode::Expr(Expr::Wrap(wrap)) => {
                 let tail = eval_markup(vm, nodes)?;
-                vm.scopes.top.def_mut(wrap.binding().take(), tail);
+                vm.scopes.top.define(wrap.binding().take(), tail);
                 wrap.body().eval(vm)?.display()
             }
 
@@ -354,10 +353,7 @@ impl Eval for Ident {
     type Output = Value;
 
     fn eval(&self, vm: &mut Machine) -> TypResult<Self::Output> {
-        match vm.scopes.get(self) {
-            Some(slot) => Ok(slot.read().clone()),
-            None => bail!(self.span(), "unknown variable"),
-        }
+        vm.scopes.get(self).cloned().at(self.span())
     }
 }
 
@@ -404,7 +400,7 @@ fn eval_code(
             }
             Expr::Wrap(wrap) => {
                 let tail = eval_code(vm, exprs)?;
-                vm.scopes.top.def_mut(wrap.binding().take(), tail);
+                vm.scopes.top.define(wrap.binding().take(), tail);
                 wrap.body().eval(vm)?
             }
 
@@ -565,8 +561,7 @@ impl BinaryExpr {
         op: fn(Value, Value) -> StrResult<Value>,
     ) -> TypResult<Value> {
         let rhs = self.rhs().eval(vm)?;
-        let lhs = self.lhs();
-        let mut location = lhs.access(vm)?;
+        let location = self.lhs().access(vm)?;
         let lhs = std::mem::take(&mut *location);
         *location = op(lhs, rhs).at(self.span())?;
         Ok(Value::None)
@@ -748,7 +743,7 @@ impl Eval for LetExpr {
             Some(expr) => expr.eval(vm)?,
             None => Value::None,
         };
-        vm.scopes.top.def_mut(self.binding().take(), value);
+        vm.scopes.top.define(self.binding().take(), value);
         Ok(Value::None)
     }
 }
@@ -860,7 +855,7 @@ impl Eval for ForExpr {
             (for ($($binding:ident => $value:ident),*) in $iter:expr) => {{
                 #[allow(unused_parens)]
                 for ($($value),*) in $iter {
-                    $(vm.scopes.top.def_mut(&$binding, $value);)*
+                    $(vm.scopes.top.define(&$binding, $value);)*
 
                     let body = self.body();
                     let value = body.eval(vm)?;
@@ -937,14 +932,14 @@ impl Eval for ImportExpr {
 
         match self.imports() {
             Imports::Wildcard => {
-                for (var, slot) in module.scope.iter() {
-                    vm.scopes.top.def_mut(var, slot.read().clone());
+                for (var, value) in module.scope.iter() {
+                    vm.scopes.top.define(var, value.clone());
                 }
             }
             Imports::Items(idents) => {
                 for ident in idents {
-                    if let Some(slot) = module.scope.get(&ident) {
-                        vm.scopes.top.def_mut(ident.take(), slot.read().clone());
+                    if let Some(value) = module.scope.get(&ident) {
+                        vm.scopes.top.define(ident.take(), value.clone());
                     } else {
                         bail!(ident.span(), "unresolved import");
                     }
@@ -1028,11 +1023,11 @@ impl Eval for ReturnExpr {
 /// Access an expression mutably.
 pub trait Access {
     /// Access the value.
-    fn access<'a>(&self, vm: &'a mut Machine) -> TypResult<Location<'a>>;
+    fn access<'a>(&self, vm: &'a mut Machine) -> TypResult<&'a mut Value>;
 }
 
 impl Access for Expr {
-    fn access<'a>(&self, vm: &'a mut Machine) -> TypResult<Location<'a>> {
+    fn access<'a>(&self, vm: &'a mut Machine) -> TypResult<&'a mut Value> {
         match self {
             Expr::Ident(v) => v.access(vm),
             Expr::FieldAccess(v) => v.access(vm),
@@ -1043,68 +1038,35 @@ impl Access for Expr {
 }
 
 impl Access for Ident {
-    fn access<'a>(&self, vm: &'a mut Machine) -> TypResult<Location<'a>> {
-        match vm.scopes.get(self) {
-            Some(slot) => match slot.try_write() {
-                Some(guard) => Ok(RwLockWriteGuard::map(guard, |v| v)),
-                None => bail!(self.span(), "cannot mutate a constant"),
-            },
-            None => bail!(self.span(), "unknown variable"),
-        }
+    fn access<'a>(&self, vm: &'a mut Machine) -> TypResult<&'a mut Value> {
+        vm.scopes.get_mut(self).at(self.span())
     }
 }
 
 impl Access for FieldAccess {
-    fn access<'a>(&self, vm: &'a mut Machine) -> TypResult<Location<'a>> {
-        let guard = self.object().access(vm)?;
-        try_map(guard, |value| {
-            Ok(match value {
-                Value::Dict(dict) => dict.get_mut(self.field().take()),
-                v => bail!(
-                    self.object().span(),
-                    "expected dictionary, found {}",
-                    v.type_name(),
-                ),
-            })
+    fn access<'a>(&self, vm: &'a mut Machine) -> TypResult<&'a mut Value> {
+        Ok(match self.object().access(vm)? {
+            Value::Dict(dict) => dict.get_mut(self.field().take()),
+            v => bail!(
+                self.object().span(),
+                "expected dictionary, found {}",
+                v.type_name(),
+            ),
         })
     }
 }
 
 impl Access for FuncCall {
-    fn access<'a>(&self, vm: &'a mut Machine) -> TypResult<Location<'a>> {
+    fn access<'a>(&self, vm: &'a mut Machine) -> TypResult<&'a mut Value> {
         let args = self.args().eval(vm)?;
-        let guard = self.callee().access(vm)?;
-        try_map(guard, |value| {
-            Ok(match value {
-                Value::Array(array) => {
-                    array.get_mut(args.into_index()?).at(self.span())?
-                }
-                Value::Dict(dict) => dict.get_mut(args.into_key()?),
-                v => bail!(
-                    self.callee().span(),
-                    "expected collection, found {}",
-                    v.type_name(),
-                ),
-            })
+        Ok(match self.callee().access(vm)? {
+            Value::Array(array) => array.get_mut(args.into_index()?).at(self.span())?,
+            Value::Dict(dict) => dict.get_mut(args.into_key()?),
+            v => bail!(
+                self.callee().span(),
+                "expected collection, found {}",
+                v.type_name(),
+            ),
         })
     }
 }
-
-/// A mutable location.
-type Location<'a> = MappedRwLockWriteGuard<'a, Value>;
-
-/// Map a reader-writer lock with a function.
-fn try_map<F>(location: Location, f: F) -> TypResult<Location>
-where
-    F: FnOnce(&mut Value) -> TypResult<&mut Value>,
-{
-    let mut error = None;
-    MappedRwLockWriteGuard::try_map(location, |value| match f(value) {
-        Ok(value) => Some(value),
-        Err(err) => {
-            error = Some(err);
-            None
-        }
-    })
-    .map_err(|_| error.unwrap())
-}
diff --git a/src/eval/scope.rs b/src/eval/scope.rs
index 8a0b8165..29778a90 100644
--- a/src/eval/scope.rs
+++ b/src/eval/scope.rs
@@ -1,18 +1,11 @@
 use std::collections::BTreeMap;
 use std::fmt::{self, Debug, Formatter};
-use std::hash::{Hash, Hasher};
-use std::iter;
-use std::sync::Arc;
-
-use parking_lot::RwLock;
+use std::hash::Hash;
 
 use super::{Args, Func, Machine, Node, Value};
-use crate::diag::TypResult;
+use crate::diag::{StrResult, TypResult};
 use crate::util::EcoString;
 
-/// A slot where a variable is stored.
-pub type Slot = Arc<RwLock<Value>>;
-
 /// A stack of scopes.
 #[derive(Debug, Default, Clone)]
 pub struct Scopes<'a> {
@@ -42,46 +35,43 @@ impl<'a> Scopes<'a> {
         self.top = self.scopes.pop().expect("no pushed scope");
     }
 
-    /// Look up the slot of a variable.
-    pub fn get(&self, var: &str) -> Option<&Slot> {
-        iter::once(&self.top)
+    /// Try to access a variable immutably.
+    pub fn get(&self, var: &str) -> StrResult<&Value> {
+        Ok(std::iter::once(&self.top)
             .chain(self.scopes.iter().rev())
             .chain(self.base.into_iter())
             .find_map(|scope| scope.get(var))
+            .ok_or("unknown variable")?)
     }
-}
 
-/// A map from variable names to variable slots.
-#[derive(Default, Clone)]
-pub struct Scope {
-    /// The mapping from names to slots.
-    values: BTreeMap<EcoString, Slot>,
+    /// Try to access a variable mutably.
+    pub fn get_mut(&mut self, var: &str) -> StrResult<&mut Value> {
+        std::iter::once(&mut self.top)
+            .chain(&mut self.scopes.iter_mut().rev())
+            .find_map(|scope| scope.get_mut(var))
+            .ok_or_else(|| {
+                if self.base.map_or(false, |base| base.get(var).is_some()) {
+                    "cannot mutate a constant"
+                } else {
+                    "unknown variable"
+                }
+            })?
+    }
 }
 
+/// A map from binding names to values.
+#[derive(Default, Clone, Hash)]
+pub struct Scope(BTreeMap<EcoString, Slot>);
+
 impl Scope {
     /// Create a new empty scope.
     pub fn new() -> Self {
         Self::default()
     }
 
-    /// Define a constant variable with a value.
-    pub fn def_const(&mut self, var: impl Into<EcoString>, value: impl Into<Value>) {
-        let cell = RwLock::new(value.into());
-
-        // Make it impossible to write to this value again.
-        std::mem::forget(cell.read());
-
-        self.values.insert(var.into(), Arc::new(cell));
-    }
-
-    /// Define a mutable variable with a value.
-    pub fn def_mut(&mut self, var: impl Into<EcoString>, value: impl Into<Value>) {
-        self.values.insert(var.into(), Arc::new(RwLock::new(value.into())));
-    }
-
-    /// Define a variable with a slot.
-    pub fn def_slot(&mut self, var: impl Into<EcoString>, slot: Slot) {
-        self.values.insert(var.into(), slot);
+    /// Bind a value to a name.
+    pub fn define(&mut self, name: impl Into<EcoString>, value: impl Into<Value>) {
+        self.0.insert(name.into(), Slot::new(value.into(), Kind::Normal));
     }
 
     /// Define a function through a native rust function.
@@ -90,32 +80,36 @@ impl Scope {
         name: &'static str,
         func: fn(&mut Machine, &mut Args) -> TypResult<Value>,
     ) {
-        self.def_const(name, Func::from_fn(name, func));
+        self.define(name, Func::from_fn(name, func));
     }
 
     /// Define a function through a native rust node.
     pub fn def_node<T: Node>(&mut self, name: &'static str) {
-        self.def_const(name, Func::from_node::<T>(name));
+        self.define(name, Func::from_node::<T>(name));
     }
 
-    /// Look up the value of a variable.
-    pub fn get(&self, var: &str) -> Option<&Slot> {
-        self.values.get(var)
+    /// Define a captured, immutable binding.
+    pub fn define_captured(
+        &mut self,
+        var: impl Into<EcoString>,
+        value: impl Into<Value>,
+    ) {
+        self.0.insert(var.into(), Slot::new(value.into(), Kind::Captured));
     }
 
-    /// Iterate over all definitions.
-    pub fn iter(&self) -> impl Iterator<Item = (&str, &Slot)> {
-        self.values.iter().map(|(k, v)| (k.as_str(), v))
+    /// Try to access a variable immutably.
+    pub fn get(&self, var: &str) -> Option<&Value> {
+        self.0.get(var).map(Slot::read)
     }
-}
 
-impl Hash for Scope {
-    fn hash<H: Hasher>(&self, state: &mut H) {
-        self.values.len().hash(state);
-        for (name, value) in self.values.iter() {
-            name.hash(state);
-            value.read().hash(state);
-        }
+    /// Try to access a variable mutably.
+    pub fn get_mut(&mut self, var: &str) -> Option<StrResult<&mut Value>> {
+        self.0.get_mut(var).map(Slot::write)
+    }
+
+    /// Iterate over all definitions.
+    pub fn iter(&self) -> impl Iterator<Item = (&str, &Value)> {
+        self.0.iter().map(|(k, v)| (k.as_str(), v.read()))
     }
 }
 
@@ -123,7 +117,45 @@ impl Debug for Scope {
     fn fmt(&self, f: &mut Formatter) -> fmt::Result {
         f.write_str("Scope ")?;
         f.debug_map()
-            .entries(self.values.iter().map(|(k, v)| (k, v.read())))
+            .entries(self.0.iter().map(|(k, v)| (k, v.read())))
             .finish()
     }
 }
+
+/// A slot where a variable is stored.
+#[derive(Clone, Hash)]
+struct Slot {
+    /// The stored value.
+    value: Value,
+    /// The kind of slot, determines how the value can be accessed.
+    kind: Kind,
+}
+
+/// The different kinds of slots.
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
+enum Kind {
+    /// A normal, mutable binding.
+    Normal,
+    /// A captured copy of another variable.
+    Captured,
+}
+
+impl Slot {
+    /// Create a new constant slot.
+    fn new(value: Value, kind: Kind) -> Self {
+        Self { value, kind }
+    }
+
+    /// Read the variable.
+    fn read(&self) -> &Value {
+        &self.value
+    }
+
+    /// Try to write to the variable.
+    fn write(&mut self) -> StrResult<&mut Value> {
+        match self.kind {
+            Kind::Normal => Ok(&mut self.value),
+            Kind::Captured => Err("cannot mutate a captured variable")?,
+        }
+    }
+}