Remove separate let scopes (#51)

Let scopes originally used their own scoping system. Instead, I've
merged their behavior to act like scoped functions. I have also added a
note in the docs to not use functions directly as a let block, as it
will lead to unexpected behavior.

docs/src/reference/builtins.md

@@ -721,12 +721,17 @@ Pops `b.len()` items off of the stack, assigning each item the corresponding sym
 
 If list `b` was `(first second)`, then they would be popped from the stack in order, following this signature: `([first] [second] --)`.
 
+**Important Note:** Functions **cannot be used** as the block of a let (`a`). To use functions within lets, wrap them within the let block: `0 '((fn a)) '(a) let`. Lets create create their own scopes, so any `def` will be isolated to that `let`.
+
 **Examples:**
 ```clj
 10 2 '(a b -) '(a b) let
 ;; 8
 
-10 2 '(fn a b -) '(a b) let
+10 2
+'(
+  (fn a b -)
+) '(a b) let
 ;; 8
 
 10 2

stack-core/src/context.rs

@@ -15,7 +15,6 @@ use crate::{
 #[derive(Debug, Clone, PartialEq, Default)]
 pub struct Context {
   stack: Vec<Expr>,
-  lets: Vec<HashMap<Symbol, Expr>>,
   scopes: VecOne<Scope>,
   journal: Option<Journal>,
   sources: HashMap<Symbol, Source>,
@@ -26,7 +25,6 @@ impl Context {
   pub fn new() -> Self {
     Self {
       stack: Vec::new(),
-      lets: Vec::new(),
       scopes: VecOne::new(Scope::new()),
       journal: None,
       sources: HashMap::new(),
@@ -91,24 +89,26 @@ impl Context {
     &mut self.journal
   }
 
-  pub fn stack_push(&mut self, expr: Expr) -> Result<(), RunError> {
-    let expr = if expr.kind.is_function() {
+  pub fn scan_expr(&mut self, expr: Expr) -> Result<Expr, RunError> {
+    if expr.kind.is_function() {
       let mut duplicate = self.scopes.last().duplicate();
       let mut scanner = Scanner::new(&mut duplicate);
 
       match scanner.scan(expr) {
-        Ok(expr) => expr,
-        Err((expr, reason)) => {
-          return Err(RunError {
-            reason,
-            context: self.clone(),
-            expr,
-          })
-        }
+        Ok(expr) => Ok(expr),
+        Err((expr, reason)) => Err(RunError {
+          reason,
+          context: self.clone(),
+          expr,
+        }),
       }
     } else {
-      expr
-    };
+      Ok(expr)
+    }
+  }
+
+  pub fn stack_push(&mut self, expr: Expr) -> Result<(), RunError> {
+    let expr = self.scan_expr(expr)?;
 
     if let Some(journal) = self.journal_mut() {
       journal.op(JournalOp::Push(expr.clone()));
@@ -164,8 +164,6 @@ impl Context {
     let layer = self.scopes.last_mut();
 
     layer.define(symbol, value);
-    // Remove the item from our let block since we've defined our own
-    self.let_remove(symbol);
   }
 
   pub fn set_scope_item(
@@ -199,29 +197,4 @@ impl Context {
   pub fn pop_scope(&mut self) {
     self.scopes.try_pop();
   }
-
-  #[inline]
-  pub fn let_push(&mut self) {
-    self.lets.push(HashMap::new());
-  }
-
-  #[inline]
-  pub fn let_pop(&mut self) -> Option<HashMap<Symbol, Expr>> {
-    self.lets.pop()
-  }
-
-  #[inline]
-  pub fn let_get(&self, name: Symbol) -> Option<&Expr> {
-    self.lets.iter().rev().find_map(|x| x.get(&name))
-  }
-
-  #[inline]
-  pub fn let_set(&mut self, name: Symbol, expr: Expr) -> Option<Expr> {
-    self.lets.last_mut().and_then(|x| x.insert(name, expr))
-  }
-
-  #[inline]
-  pub fn let_remove(&mut self, name: Symbol) -> Option<Expr> {
-    self.lets.last_mut().and_then(|x| x.remove(&name))
-  }
 }

stack-core/src/engine.rs

@@ -13,7 +13,6 @@ use crate::{
   intrinsic::Intrinsic,
   journal::JournalOp,
   module::Module,
-  scope::Scanner,
   symbol::Symbol,
 };
 
@@ -118,13 +117,7 @@ impl Engine {
       }
     }
 
-    let expr = Scanner::new(context.scope_mut()).scan(expr).map_err(
-      |(expr, reason)| RunError {
-        expr,
-        context: context.clone(),
-        reason,
-      },
-    )?;
+    let expr = context.scan_expr(expr)?;
     match expr.kind {
       ExprKind::Nil
       | ExprKind::Boolean(_)
@@ -173,9 +166,6 @@ impl Engine {
               reason: RunErrorReason::UnknownCall,
             })
           }
-        } else if let Some(r#let) = context.let_get(x).cloned() {
-          context.stack_push(r#let)?;
-          Ok(context)
         } else if let Some(item) = context.scope_item(x) {
           if item.kind.is_function() {
             let fn_ident = item.kind.fn_symbol().unwrap();
@@ -457,7 +447,7 @@ mod tests {
   }
 
   #[test]
-  fn lets_act_as_overlays() {
+  fn lets_do_not_act_as_overlays() {
     let source = Source::new("", "0 'a def 1 '(a 2 'a def a) '(a) let a");
     let mut lexer = Lexer::new(source);
     let exprs = crate::parser::parse(&mut lexer).unwrap();
@@ -475,14 +465,14 @@ mod tests {
       vec![
         &ExprKind::Integer(1),
         &ExprKind::Integer(2),
-        &ExprKind::Integer(2),
+        &ExprKind::Integer(0),
       ]
     );
   }
 
   #[test]
   fn functions_work_in_lets() {
-    let source = Source::new("", "0 'a def 1 '(fn a 2 'a def a) '(a) let a");
+    let source = Source::new("", "0 'a def 1 '((fn a 2 'a def a)) '(a) let a");
     let mut lexer = Lexer::new(source);
     let exprs = crate::parser::parse(&mut lexer).unwrap();
 
@@ -504,38 +494,14 @@ mod tests {
     );
   }
 
-  #[test]
-  fn scopeless_functions_work_in_lets() {
-    let source = Source::new("", "0 'a def 1 '(fn! a 2 'a def a) '(a) let a");
-    let mut lexer = Lexer::new(source);
-    let exprs = crate::parser::parse(&mut lexer).unwrap();
-
-    let engine = Engine::new();
-    let mut context = Context::new().with_stack_capacity(32);
-    context = engine.run(context, exprs).unwrap();
-
-    assert_eq!(
-      context
-        .stack()
-        .iter()
-        .map(|expr| &expr.kind)
-        .collect::<Vec<_>>(),
-      vec![
-        &ExprKind::Integer(1),
-        &ExprKind::Integer(2),
-        &ExprKind::Integer(2),
-      ]
-    );
-  }
-
   #[test]
   fn lets_dont_leak() {
     let source = Source::new(
       "",
       "0 'a def
       1 '(a) '(a) let
-      1 '(fn! a) '(a) let
-      1 '(fn a) '(a) let
+      1 '((fn! a)) '(a) let
+      1 '((fn a)) '(a) let
       a",
     );
     let mut lexer = Lexer::new(source);
@@ -561,16 +527,22 @@ mod tests {
   }
 
   #[test]
-  fn lets_cant_set() {
-    let source = Source::new("", "1 '(2 'a set) '(a) let");
+  fn lets_can_set() {
+    let source = Source::new("", "1 '(a 2 'a set a) '(a) let");
     let mut lexer = Lexer::new(source);
     let exprs = crate::parser::parse(&mut lexer).unwrap();
 
     let engine = Engine::new();
-    let context = Context::new().with_stack_capacity(32);
+    let mut context = Context::new().with_stack_capacity(32);
+    context = engine.run(context, exprs).unwrap();
+
     assert_eq!(
-      engine.run(context, exprs).map_err(|err| err.reason),
-      Err(RunErrorReason::CannotSetBeforeDef)
+      context
+        .stack()
+        .iter()
+        .map(|expr| &expr.kind)
+        .collect::<Vec<_>>(),
+      vec![&ExprKind::Integer(1), &ExprKind::Integer(2),]
     );
   }
 }

stack-core/src/intrinsic.rs

@@ -832,20 +832,20 @@ impl Intrinsic {
               },
             )?;
 
-            context.let_push();
-
+            let mut scope = context.scope().duplicate();
             for name in n.into_iter().rev() {
               let expr = context.stack_pop(&expr)?;
-              context.let_set(name, expr);
+              scope.define(name, expr);
             }
 
             if let Some(journal) = context.journal_mut() {
               journal.commit();
               journal.op(JournalOp::FnStart(false));
             }
 
+            context.push_scope(scope);
             context = engine.run_expr(context, body)?;
-            context.let_pop().unwrap();
+            context.pop_scope();
 
             if let Some(journal) = context.journal_mut() {
               journal.commit();
@@ -905,11 +905,7 @@ impl Intrinsic {
         match name.kind {
           ExprKind::Symbol(symbol) => {
             // Lets take precedence over scoped vars
-            let item = if let Some(let_item) = context.let_get(symbol) {
-              Some(let_item.clone())
-            } else {
-              context.scope_item(symbol)
-            };
+            let item = context.scope_item(symbol);
 
             context
               .stack_push(item.ok_or_else(|| RunError {

stack-core/src/scope.rs

@@ -18,6 +18,7 @@ impl fmt::Debug for Scope {
 }
 
 impl Clone for Scope {
+  /// Clones the scope, using the same Arc's as self
   fn clone(&self) -> Self {
     let mut items = HashMap::new();
 
@@ -103,6 +104,7 @@ impl Scope {
     }
   }
 
+  /// Creates a new scope, linking the new symbols to that of self (such as for a function call)
   pub fn duplicate(&self) -> Self {
     let mut items = HashMap::new();
 

stack-std/src/scope.rs

@@ -16,8 +16,6 @@ pub fn module() -> Module {
           ExprKind::Symbol(ref x) => {
             if Intrinsic::from_str(x.as_str()).is_ok() {
               context.stack_push(ExprKind::String("intrinsic".into()).into())
-            } else if context.let_get(*x).is_some() {
-              context.stack_push(ExprKind::String("let".into()).into())
             } else if context.scope_item(*x).is_some() {
               context.stack_push(ExprKind::String("scope".into()).into())
             } else if engine.module(x).is_some() {