Update how visitors work, add NameId type

* Visitors are now defined on a per-type level, allowing for greater
  flexibility in combining and re-using behavior
* NameId is used for namespaces, which are used to index locally scoped
  variables. Syms are used for free namespaces, specifically in objects.
  All NameIDs are symbols, while not all symbols are NameIDs.

Signed-off-by: Alek Ratzloff <alekratz@gmail.com>

src/compile/block.rs

@@ -1,6 +1,69 @@
-use crate::compile::ir::Body;
+use crate::{
+    syn::{ast::*, op::BinOp, span::*},
+    compile::{
+        Ctx,
+        error::*,
+        ir,
+        visit::*,
+    },
+};
 
 // basic block
 pub enum Block {
-    Body(Body),
+    Body(ir::Body),
+}
+
+pub struct TranslateAst<'c, 't> {
+    ctx: &'c mut Ctx,
+    text: &'t str,
+}
+
+impl<'c, 't> TranslateAst<'c, 't> {
+    pub fn new(ctx: &'c mut Ctx, text: &'t str) -> Self {
+        TranslateAst {
+            ctx,
+            text,
+        }
+    }
+
+    pub fn translate(&mut self, ast: &Vec<Stmt>) -> Result<ir::Body> {
+        todo!()
+    }
+
+    fn visit_lhs_expr(&mut self, expr: &Expr) -> Result<ir::Lhs> {
+        match expr {
+            Expr::Bin(b) if b.op == BinOp::Dot => todo!(),
+            Expr::Base(BaseExpr { kind: BaseExprKind::Ident, .. }) => {
+                let name = expr.text_at(self.text);
+                //let name_id = self.ctx.
+                    todo!()
+                //Ok(ir::Lhs::Name(
+            }
+            _ => todo!()
+        }
+    }
+}
+
+impl Visit<Stmt> for TranslateAst<'_, '_> {
+    type Out = Result<ir::Stmt>;
+
+    fn visit(&mut self, stmt: &Stmt) -> Self::Out {
+        todo!()
+    }
+}
+
+impl Visit<AssignStmt> for TranslateAst<'_, '_> {
+    type Out = Result<ir::Stmt>;
+
+    fn visit(&mut self, stmt: &AssignStmt) -> Self::Out {
+        todo!()
+    }
+}
+
+impl Visit<Expr> for TranslateAst<'_, '_> {
+    type Out = Result<ir::Expr>;
+
+    fn visit(&mut self, expr: &Expr) -> Self::Out {
+        todo!()
+    }
 }

src/compile/error.rs

@@ -1,9 +1,12 @@
+use crate::syn::span::*;
 use snafu::Snafu;
 
 #[derive(Debug, Snafu)]
 pub enum Error {
-    //#[snafu(display("illegal attr key"))]
-
+    #[snafu(display("invalid assignment target"))]
+    InvalidLhs {
+        span: Span,
+    }
 }
 
 pub type Result<T, E = Error> = std::result::Result<T, E>;

src/compile/ir.rs

@@ -1,5 +1,4 @@
 use crate::{
-    compile::Ctx,
     obj::Sym,
     syn::span::*,
 };
@@ -14,7 +13,7 @@ pub enum Stmt {
 
 #[derive(Debug, Clone)]
 pub enum Lhs {
-    Sym(Sym),
+    Name(Sym),
     Complex(Expr),
 }
 
@@ -29,7 +28,7 @@ pub enum BaseExprKind {
     Num(i64),
     Str(String),
     Sym(Sym),
-    Ident(String),
+    Ident(Sym),
 }
 
 #[derive(Debug, Clone)]
@@ -37,23 +36,3 @@ pub struct BaseExpr {
     pub kind: BaseExprKind,
     pub span: Span,
 }
-
-// FromAst and IntoAst are just so we can use .into_ast(ctx, text)
-// some impls may not need ctx &mut ref - but probably needed for symbol resolution
-
-pub trait FromAst<A> {
-    fn from_ast(other: A, ctx: &mut Ctx, text: &str) -> Self;
-}
-
-pub trait IntoIr<I> {
-    fn into_ir(self, ctx: &mut Ctx, text: &str) -> I;
-}
-
-impl<A, I> IntoIr<I> for A
-where
-    I: FromAst<A>,
-{
-    fn into_ir(self, ctx: &mut Ctx, text: &str) -> I {
-        FromAst::from_ast(self, ctx, text)
-    }
-}

src/compile/mod.rs

@@ -1,8 +1,8 @@
+#[macro_use] pub mod visit;
 pub mod block;
 pub mod error;
 pub mod ir;
 pub mod name;
-pub mod visit;
 
 use crate::compile::name::NameStack;
 
@@ -10,23 +10,17 @@ use crate::compile::name::NameStack;
 // * Collect names as symbols
 // * Create basic blocks
 
-pub struct Ctx<'t> {
-    text: &'t str,
+pub struct Ctx {
     name_stack: NameStack,
 }
 
-impl<'t> Ctx<'t> {
-    pub fn new(text: &'t str) -> Self {
+impl Ctx {
+    pub fn new() -> Self {
         Ctx {
-            text,
             name_stack: NameStack::new(),
         }
     }
 
-    pub fn text(&self) -> &'t str {
-        self.text
-    }
-
     pub fn name_stack(&self) -> &NameStack {
         &self.name_stack
     }
@@ -35,16 +29,3 @@ impl<'t> Ctx<'t> {
         &mut self.name_stack
     }
 }
-
-// no sugar in the syntax quite yet
-/*
-pub struct Desugar;
-
-impl Pass<Vec<Stmt>> for Desugar {
-    type Out = Vec<Stmt>;
-
-    fn pass(&mut self, input: Vec<Stmt>) -> Vec<Stmt> {
-        input
-    }
-}
-*/

src/compile/name.rs

@@ -1,22 +1,10 @@
 use crate::{
     compile::{visit::*, Ctx},
-    syn::{ast::prelude::*, op::BinOp, span::*},
+    obj::prelude::*,
+    syn::{ast::prelude::*, span::*},
 };
 use std::collections::HashMap;
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
-pub struct NameId(usize);
-
-impl NameId {
-    pub fn new(id: usize) -> Self {
-        NameId(id)
-    }
-
-    pub fn id(&self) -> usize {
-        self.0
-    }
-}
-
 pub struct NameStack {
     next_sym: usize,
     stack: Vec<HashMap<String, NameId>>,
@@ -84,78 +72,70 @@ impl NameStack {
 }
 
 /// Collect local names and push them to the top layer of the name stack.
-pub struct CollectNames<'c, 't: 'c> {
-    ctx: &'c mut Ctx<'t>,
+pub struct CollectNames<'c, 't> {
+    ctx: &'c mut Ctx,
+    text: &'t str,
 }
 
-impl<'c, 't: 'c> CollectNames<'c, 't> {
-    pub fn new(ctx: &'c mut Ctx<'t>) -> Self {
-        CollectNames { ctx }
+default_visitor!(Expr for CollectNames<'_, '_> where Out = ());
+empty_visitor!(FunCallExpr for CollectNames<'_, '_>);
+empty_visitor!(FunExpr for CollectNames<'_, '_>);
+
+impl<'c, 't> CollectNames<'c, 't> {
+    pub fn new(ctx: &'c mut Ctx, text: &'t str) -> Self {
+        CollectNames { ctx, text }
     }
 
     pub fn collect(&mut self, stmts: &Vec<Stmt>) {
+        // Collect all LHS assignments
         for stmt in stmts.iter() {
-            self.visit_stmt(stmt);
-        }
-    }
-
-    fn visit_base_expr(&mut self, expr: &BaseExpr) {
-        match &expr.kind {
-            BaseExprKind::Ident => {
-                let name = expr.text_at(self.ctx.text()).to_string();
-                self.ctx.name_stack_mut().add(name);
+            if let Stmt::Assign(stmt) = stmt {
+                self.visit(stmt);
             }
-            BaseExprKind::List(l) | BaseExprKind::Tuple(l) => {
-                l.iter().for_each(|expr| self.visit_expr(expr));
-            }
-            BaseExprKind::Object(o) => {
-                o.iter().for_each(|(key, value)| {
-                    self.visit_expr(key);
-                    self.visit_expr(value);
-                });
-            }
-            _ => { /* no-op */ }
         }
     }
 }
 
-impl Visit for CollectNames<'_, '_> {
+impl Visit<AssignStmt> for CollectNames<'_, '_> {
     type Out = ();
 
-    fn visit_expr(&mut self, expr: &Expr) -> Self::Out {
-        match expr {
-            Expr::Bin(b) => {
-                self.visit_expr(&b.lhs);
-                // don't descend into dot-expressions - these are not names, but symbols
-                if b.op != BinOp::Dot {
-                    self.visit_expr(&b.rhs);
-                }
-            }
-            Expr::Un(u) => {
-                self.visit_expr(&u.expr);
-            }
-            Expr::FunCall(f) => {
-                self.visit_expr(&f.expr);
-                f.args.iter().for_each(|expr| self.visit_expr(expr));
-            }
-            Expr::Index(i) => {
-                self.visit_expr(&i.expr);
-                self.visit_expr(&i.index);
-            }
-            Expr::Fun(_) => { /* no-op */ }
-            Expr::Base(b) => self.visit_base_expr(b),
-        }
+    fn visit(&mut self, stmt: &AssignStmt) -> Self::Out {
+        self.visit(&stmt.lhs);
     }
+}
 
-    fn visit_stmt(&mut self, stmt: &Stmt) -> Self::Out {
-        match stmt {
-            Stmt::Assign(AssignStmt { lhs, rhs, .. }) => {
-                self.visit_expr(lhs);
-                self.visit_expr(rhs);
-            }
-            Stmt::Expr(expr) => {
-                self.visit_expr(expr);
-            }
+impl Visit<BinExpr> for CollectNames<'_, '_> {
+    type Out = ();
+
+    fn visit(&mut self, expr: &BinExpr) -> Self::Out {
+        self.visit(&expr.lhs);
+    }
+}
+
+impl Visit<UnExpr> for CollectNames<'_, '_> {
+    type Out = ();
+
+    fn visit(&mut self, expr: &UnExpr) -> Self::Out {
+        self.visit(&expr.expr)
+    }
+}
+
+impl Visit<IndexExpr> for CollectNames<'_, '_> {
+    type Out = ();
+
+    fn visit(&mut self, expr: &IndexExpr) -> Self::Out {
+        self.visit(&expr.expr);
+    }
+}
+
+impl Visit<BaseExpr> for CollectNames<'_, '_> {
+    type Out = ();
+
+    fn visit(&mut self, expr: &BaseExpr) -> Self::Out {
+        // This is a LHS standalone expr
+        if let BaseExpr { kind: BaseExprKind::Ident, .. } = expr {
+            let name = expr.text_at(self.text).to_string();
+            self.ctx.name_stack_mut().add(name);
         }
     }
 }

src/compile/visit.rs

@@ -1,29 +1,91 @@
-use crate::syn::ast::{Stmt, Expr};
+use crate::syn::ast::prelude::*;
 
-pub trait Visit {
+pub trait Visit<A: Accept> {
     type Out;
-    fn visit<A: Accept<Self>>(&mut self, acceptor: &A) -> Self::Out
-        where Self: Sized
-    {
-        acceptor.accept(self)
-    }
-
-    fn visit_stmt(&mut self, stmt: &Stmt) -> Self::Out;
-    fn visit_expr(&mut self, expr: &Expr) -> Self::Out;
+    fn visit(&mut self, acceptor: &A) -> Self::Out;
 }
 
-pub trait Accept<V: Visit> {
-    fn accept(&self, visitor: &mut V) -> V::Out;
+pub trait Accept {
+    fn accept<V: Visit<Self>>(&self, visitor: &mut V) -> V::Out
+    where
+        Self: Sized;
 }
 
-impl<V: Visit> Accept<V> for Stmt {
-    fn accept(&self, visitor: &mut V) -> V::Out {
-        visitor.visit_stmt(self)
+pub trait DefaultAccept<V: Visit<Self>>: Accept + Sized
+{
+    fn default_accept(&self, visitor: &mut V) -> V::Out;
+}
+
+#[macro_export]
+macro_rules! default_visitor {
+    ($default:ident for $visitor:ty where Out = $out:ty) => {
+        impl crate::compile::visit::Visit<$default> for $visitor {
+            type Out = $out;
+            fn visit(&mut self, acceptor: &$default) -> Self::Out {
+                acceptor.default_accept(self)
+            }
+        }
+    };
+}
+
+#[macro_export]
+macro_rules! empty_visitor {
+    ($default:ident for $visitor:ty) => {
+        impl crate::compile::visit::Visit<$default> for $visitor {
+            type Out = ();
+            fn visit(&mut self, _: &$default) -> Self::Out {}
+        }
     }
 }
 
-impl<V: Visit> Accept<V> for Expr {
-    fn accept(&self, visitor: &mut V) -> V::Out {
-        visitor.visit_expr(self)
-    }
+macro_rules! impl_accept {
+    ($what:ident) => {
+        impl crate::compile::visit::Accept for $what {
+            fn accept<V: Visit<Self>>(&self, visitor: &mut V) -> V::Out {
+                visitor.visit(self)
+            }
+        }
+    };
 }
+
+macro_rules! impl_default_accept {
+    ($what:ident : $($requires:ident),+ => $($tail:tt)+) => {
+        impl<V, Out> DefaultAccept<V> for $what
+            where V: Visit<Self, Out=Out> $(+ Visit<$requires, Out=Out>)+
+        {
+            $($tail)+
+        }
+    };
+}
+
+impl_accept!(Stmt);
+impl_default_accept!(Stmt : AssignStmt, Expr =>
+    fn default_accept(&self, visitor: &mut V) -> Out {
+        match self {
+            Stmt::Assign(a) => visitor.visit(a),
+            Stmt::Expr(e) => visitor.visit(e),
+        }
+    }
+);
+impl_accept!(AssignStmt);
+impl_accept!(Expr);
+
+impl_default_accept!(Expr: BinExpr, UnExpr, FunCallExpr, IndexExpr, FunExpr, BaseExpr =>
+    fn default_accept(&self, visitor: &mut V) -> Out {
+        match self {
+            Expr::Bin(b) => visitor.visit(b.as_ref()),
+            Expr::Un(u) => visitor.visit(u.as_ref()),
+            Expr::FunCall(f) => visitor.visit(f.as_ref()),
+            Expr::Index(i) => visitor.visit(i.as_ref()),
+            Expr::Fun(f) => visitor.visit(f.as_ref()),
+            Expr::Base(b) => visitor.visit(b),
+        }
+    }
+);
+
+impl_accept!(BinExpr);
+impl_accept!(UnExpr);
+impl_accept!(FunCallExpr);
+impl_accept!(IndexExpr);
+impl_accept!(FunExpr);
+impl_accept!(BaseExpr);

src/main.rs

@@ -43,10 +43,10 @@ fn main() -> Result<()> {
     let ast = parser.next_body()?;
     //println!("{:#?}", ast);
     
-    let mut ctx = compile::Ctx::new(text.as_str());
+    let mut ctx = compile::Ctx::new();
     ctx.name_stack_mut().push_default();
     {
-        let mut collect_names = compile::name::CollectNames::new(&mut ctx);
+        let mut collect_names = compile::name::CollectNames::new(&mut ctx, text.as_str());
         collect_names.collect(&ast);
     }
     let names = ctx.name_stack_mut().pop().unwrap();

src/mem/basic.rs

@@ -65,7 +65,7 @@ impl Intern for BasicIntern {
     fn intern_sym(&mut self, s: &str) -> Sym {
         let len = self.syms.len();
         *self.syms.entry(s.to_string())
-            .or_insert_with(|| len)
+            .or_insert_with(|| Sym::new(len))
     }
 }
 

src/obj/attrs.rs

@@ -11,13 +11,13 @@ use std::{
 pub type AttrsRef = ObjRef<Attrs>;
 
 pub struct Attrs {
-    attrs: Ns,
+    attrs: Ss,
     marked: Cell<bool>,
     this: ObjRef<Attrs>,
 }
 
 impl Attrs {
-    pub fn new<G: Gc>(gc: &mut G, attrs: Ns) -> ObjRef<Self> {
+    pub fn new<G: Gc>(gc: &mut G, attrs: Ss) -> ObjRef<Self> {
         let obj: Box<ObjCell<Attrs>> = unsafe {
             let mut obj: Box<MaybeUninit<ObjCell<Attrs>>> = Box::new_uninit();
             // maybe UB? taking away the uninit right before it's initialized may be bad
@@ -88,7 +88,7 @@ impl Obj for Attrs {
 
 pub struct AttrsBuilder<'i, I: Intern> {
     intern: &'i mut I,
-    attrs: Ns,
+    attrs: Ss,
 }
 
 impl<'i, I: Intern> AttrsBuilder<'i, I> {
@@ -96,7 +96,7 @@ impl<'i, I: Intern> AttrsBuilder<'i, I> {
         Self::with_base(intern, Default::default())
     }
 
-    pub fn with_base(intern: &'i mut I, attrs: Ns) -> Self {
+    pub fn with_base(intern: &'i mut I, attrs: Ss) -> Self {
         AttrsBuilder {
             attrs,
             intern,

src/obj/mod.rs

@@ -3,18 +3,20 @@ pub mod ctx;
 pub mod dict;
 pub mod error;
 pub mod fun;
+pub mod name;
 //pub mod num;
 pub mod ns {
-    use crate::{mem::ptr::DynRef, obj::Sym};
+    use crate::{mem::ptr::DynRef, obj::{NameId, Sym}};
     use std::collections::BTreeMap;
 
-    /// A namespace
-    pub type Ns = BTreeMap<Sym, DynRef>;
+    /// A namespace, indexed by `NameID`. This is used for scopes and local values.
+    pub type Ns = BTreeMap<NameId, DynRef>;
+
+    /// A symbolspace, indexed by `Sym`. This is used for attributes.
+    pub type Ss = BTreeMap<Sym, DynRef>;
 }
 pub mod str;
-pub mod sym {
-    pub type Sym = usize;
-}
+pub mod sym;
 //pub mod ty;
 
 pub mod prelude {
@@ -24,7 +26,10 @@ pub mod prelude {
             intern::Intern,
             ptr::{DynRef, ObjRef},
         },
-        obj::{Attrs, AttrsRef, Dict, DictRef, Fun, NativeFun, Ns, Obj, ObjCtx, Str, StrRef, Sym},
+        obj::{
+            Attrs, AttrsRef, Dict, DictRef, Fun, NativeFun, NameId, Ns, Obj, ObjCtx, Ss, Str,
+            StrRef, Sym,
+        },
     };
 }
 
@@ -33,7 +38,8 @@ pub use self::{
     ctx::ObjCtx,
     dict::{Dict, DictRef},
     fun::{Fun, NativeFun},
-    ns::Ns,
+    name::NameId,
+    ns::{Ns, Ss},
     str::{Str, StrRef},
     sym::Sym,
 };

src/obj/name.rs

@@ -0,0 +1,12 @@
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+pub struct NameId(usize);
+
+impl NameId {
+    pub fn new(id: usize) -> Self {
+        NameId(id)
+    }
+
+    pub fn id(&self) -> usize {
+        self.0
+    }
+}

src/obj/sym.rs

@@ -0,0 +1,18 @@
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+pub struct Sym(usize);
+
+impl Sym {
+    pub fn new(sym: usize) -> Self {
+        Sym(sym)
+    }
+
+    pub fn sym(&self) -> usize {
+        self.0
+    }
+}
+
+impl From<usize> for Sym {
+    fn from(other: usize) -> Self {
+        Sym::new(other)
+    }
+}

src/vm/frame.rs

@@ -2,13 +2,13 @@ use crate::{
     obj::prelude::*,
     vm::op::Op,
 };
-use std::rc::Rc;
+use std::{collections::BTreeMap, rc::Rc};
 
 pub struct Frame {
     stack: Vec<DynRef>,
     ip: usize,
     return_value: Option<DynRef>,
-    locals: Ns,
+    locals: BTreeMap<NameId, DynRef>,
     ops: Rc<Vec<Op>>,
 }
 
@@ -47,11 +47,11 @@ impl Frame {
         self.return_value = Some(obj_ref);
     }
 
-    pub fn get_local(&self, sym: Sym) -> Option<DynRef> {
-        self.locals.get(&sym).copied()
+    pub fn get_local(&self, name: NameId) -> Option<DynRef> {
+        self.locals.get(&name).copied()
     }
 
-    pub fn set_local(&mut self, sym: Sym, obj_ref: DynRef) -> Option<DynRef> {
+    pub fn set_local(&mut self, sym: NameId, obj_ref: DynRef) -> Option<DynRef> {
         self.locals.insert(sym, obj_ref)
     }
 

src/vm/mod.rs

@@ -1,21 +1,23 @@
-pub mod op;
 pub mod frame;
+pub mod op;
 
 use crate::{
-    obj::prelude::*,
     mem::gc::Gc,
-    vm::{
-        op::Op,
-        frame::Frame,
-    }
+    obj::prelude::*,
+    vm::{frame::Frame, op::Op},
 };
 use std::rc::Rc;
 
+/*
 fn root_ns<I: Intern, G: Gc>(intern: &mut I, gc: &mut G) -> Ns {
     let mut ns: Ns = Default::default();
-    ns.insert(intern.intern_sym("Unit"), Attrs::new(gc, Default::default()));
+    ns.insert(
+        intern.intern_sym("Unit"),
+        Attrs::new(gc, Default::default()),
+    );
     ns
 }
+*/
 
 pub struct State<I: Intern, G: Gc> {
     intern: I,
@@ -25,9 +27,14 @@ pub struct State<I: Intern, G: Gc> {
 }
 
 impl<I: Intern, G: Gc> State<I, G> {
-    pub fn new(mut intern: I, mut gc: G) -> Self {
-        let root_ns = root_ns(&mut intern, &mut gc);
-        State { intern, gc, root_ns, frames: Default::default(), }
+    pub fn new(intern: I, gc: G, root_ns: Ns) -> Self {
+        //let root_ns = root_ns(&mut intern, &mut gc);
+        State {
+            intern,
+            gc,
+            root_ns,
+            frames: Default::default(),
+        }
     }
 
     pub fn frames(&self) -> &Vec<Frame> {
@@ -39,25 +46,19 @@ impl<I: Intern, G: Gc> State<I, G> {
     }
 
     pub fn frame(&self) -> &Frame {
-        self.frames
-            .last()
-            .unwrap()
+        self.frames.last().unwrap()
     }
 
     pub fn frame_mut(&mut self) -> &mut Frame {
-        self.frames
-            .last_mut()
-            .unwrap()
+        self.frames.last_mut().unwrap()
     }
 
     pub fn push_stack(&mut self, value: DynRef) {
-        self.frame_mut()
-            .push(value);
+        self.frame_mut().push(value);
     }
 
     pub fn pop_stack(&mut self) -> Option<DynRef> {
-        self.frame_mut()
-            .pop()
+        self.frame_mut().pop()
     }
 
     pub fn ip(&self) -> usize {
@@ -68,12 +69,12 @@ impl<I: Intern, G: Gc> State<I, G> {
         self.frame_mut().set_ip(ip);
     }
 
-    pub fn get_local(&self, sym: Sym) -> Option<DynRef> {
-        self.frame().get_local(sym)
+    pub fn get_local(&self, name: NameId) -> Option<DynRef> {
+        self.frame().get_local(name)
     }
 
-    pub fn set_local(&mut self, sym: Sym, obj_ref: DynRef) -> Option<DynRef> {
-        self.frame_mut().set_local(sym, obj_ref)
+    pub fn set_local(&mut self, name: NameId, obj_ref: DynRef) -> Option<DynRef> {
+        self.frame_mut().set_local(name, obj_ref)
     }
 
     pub fn ops(&self) -> &Rc<Vec<Op>> {
@@ -97,40 +98,36 @@ impl<I: Intern, G: Gc> State<I, G> {
     pub fn step(&mut self) {
         let mut next_ip = self.ip() + 1;
         match self.ops()[self.ip()] {
-            Op::Push(sym) => {
+            Op::Push(name) => {
                 // TODO - local not found
-                let obj_ref = self.get_local(sym)
-                    .expect("TODO - local not found");
+                let obj_ref = self.get_local(name).expect("TODO - local not found");
                 self.push_stack(obj_ref);
             }
-            Op::Pop(sym) => {
+            Op::Pop(name) => {
                 // stack should not underflow
-                let obj_ref = self.pop_stack()
-                    .expect("misaligned stack for pop");
-                if let Some(sym) = sym {
-                    self.set_local(sym, obj_ref);
+                let obj_ref = self.pop_stack().expect("misaligned stack for pop");
+                if let Some(name) = name {
+                    self.set_local(name, obj_ref);
                 }
             }
             Op::GetAttr(sym) => {
-                let top = self.pop_stack()
-                    .expect("misaligned stack for getattrs");
+                let top = self.pop_stack().expect("misaligned stack for getattrs");
                 let obj_ref = {
                     let top_ref = top.borrow();
                     let attrs = top_ref.attrs();
                     let attrs_ref = attrs.borrow();
                     // TODO - local not found
-                    attrs_ref.get(sym)
-                        .expect("TODO - local not found")
+                    attrs_ref.get(sym).expect("TODO - local not found")
                 };
                 self.push_stack(obj_ref);
             }
             Op::Call(argc) => {
-                let fun = self.pop_stack()
+                let fun = self
+                    .pop_stack()
                     .expect("misaligned stack for function call");
                 let mut argv = Vec::with_capacity(argc);
-                for _ in 0 .. argc {
-                    let arg = self.pop_stack()
-                        .expect("misaligned stack for argv");
+                for _ in 0..argc {
+                    let arg = self.pop_stack().expect("misaligned stack for argv");
                     argv.push(arg);
                 }
                 // reverse since arguments are pushed in order of being passed
@@ -147,7 +144,6 @@ impl<I: Intern, G: Gc> State<I, G> {
                 } else {
                     todo!("TODO - not a function");
                 }
-
             }
         }
         self.set_ip(next_ip);

src/vm/op.rs

@@ -1,4 +1,4 @@
-use crate::obj::Sym;
+use crate::obj::{NameId, Sym};
 
 // VM execution model:
 // * every function call has its own stack frame
@@ -8,11 +8,11 @@ use crate::obj::Sym;
 
 #[derive(Clone, Copy)]
 pub enum Op {
-    /// Push a value from a symbol
-    Push(Sym),
+    /// Push a value from a name ID.
+    Push(NameId),
     
-    /// Pop the top stack value into a local (if supplied)
-    Pop(Option<Sym>),
+    /// Pop the top stack value into a local name (if supplied)
+    Pop(Option<NameId>),
 
     /// Pop the top stack value, getting an attribute and replacing pushing that value to the
     /// stack.