Finish up function call implementation, it appears to be working

Functions are downcasted to a `Fun` trait, which will construct the
appropriate stack frame.

A few other things have been shifted around that affect internal APIs
while things are still under construction.

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

src/bin/not.rs

@@ -1,4 +1,4 @@
-use not_python::{syn::ast, compile::Compile};
+use not_python::{syn::ast, compile::Compile, vm::{Vm, signal::*}};
 
 use std::{fs, path::PathBuf};
 use structopt::StructOpt;
@@ -55,5 +55,11 @@ fn main() -> Result<()> {
         return Ok(());
     }
 
+    if action == "run" {
+        let mut vm = Vm::new(&const_pool);
+        vm.handle_signal(Signal::Call(main, vec![]));
+        vm.resume();
+    }
+
     Ok(())
 }

src/compile/mod.rs

@@ -23,10 +23,14 @@ impl Compile {
     /// Compiles the given AST body.
     pub fn compile(mut self, body: &Body) -> error::Result<(ConstPool, UserFunRef)> {
         self.push_scope_layer();
-        let main = thunk::CompileBody::new(&mut self)
+        let mut main = thunk::CompileBody::new(&mut self)
             .compile(body)?
             .flatten()
             .to_vec();
+        // XXX TODO(compile)
+        // remove this when we get returns implemented
+        main.push(crate::vm::inst::Inst::PushSym(crate::obj::reserved::NIL_NAME.sym));
+        main.push(crate::vm::inst::Inst::Return);
         let globals_syms: std::collections::BTreeMap<_, _> = self.pop_scope_layer().unwrap()
             .into_iter()
             .map(|(sym, name)| (name, sym))

src/compile/scope.rs

@@ -1,17 +1,10 @@
 use crate::obj::prelude::*;
 use std::collections::BTreeMap;
 
-// current TODO/problem: How should locals work?
-//
-// * I think we should have a global/local dichotomy in the VM. You can only look up the local
-//   scope in the VM; inner functions that have captures should be created on the fly and detected
-//   by the compiler. I.E. any lookups that are not local or global should be captured dynamically.
-// * Need to differentiate between locals collected on the compilation side vs. locals on the
-//   object/vm side
-
 pub type ScopeLocalSyms = BTreeMap<Sym, Name>;
 pub type ScopeLocals = Vec<Sym>;
 
+#[derive(Debug, Default)]
 pub struct Scope {
     scope: Vec<ScopeLocalSyms>,
 }
@@ -77,10 +70,3 @@ impl Scope {
         self.scope.first_mut()
     }
 }
-
-impl Default for Scope {
-    fn default() -> Self {
-        // empty global scope
-        Self { scope: vec![Default::default()] }
-    }
-}

src/compile/thunk.rs

@@ -382,9 +382,13 @@ impl Visit for CompileBody<'_> {
             })
             .collect();
 
-        let code = self.visit_body(&expr.body)?
+        let mut code = self.visit_body(&expr.body)?
             .flatten()
             .to_vec();
+        // XXX TODO(compile)
+        // remove this when we get returns implemented
+        code.push(Inst::PushSym(crate::obj::reserved::NIL_NAME.sym));
+        code.push(Inst::Return);
         let (hdl, _fun) = self.compile.push_const(UserFun::new_obj(code, locals));
 
         // TODO(compile) : determine return value at the end of the body (preferably at parse-time)
@@ -397,6 +401,7 @@ impl Visit for CompileBody<'_> {
         let thunk = match atom {
             Atom::Ident(ident) => {
                 let sym = global_sym(ident.to_string());
+                // TODO : use "LOAD_GLOBAL" instead of "LOAD_LOCAL" when inside a user function
                 if let Some(local) = self.compile.lookup_local(sym) {
                     // get local
                     Inst::LoadLocal(local).into()

src/obj/fun.rs

@@ -1,10 +1,16 @@
-use crate::{obj::{reserved::*, prelude::*}, vm::{consts::ConstPool, inst::Inst, signal::*, Vm}};
+use crate::{obj::{reserved::*, prelude::*}, vm::{consts::ConstPool, frame::*, inst::Inst, signal::*, Vm}};
 use once_cell::sync::Lazy;
 use shredder::{GcSafeWrapper, Scan};
-use std::{fmt::{Debug, Formatter, self}, io::{self, Write}};
+use std::{fmt::{Debug, Formatter, self}, io::{self, Write}, sync::Arc};
 
 pub type FunLocals = Vec<Sym>;
 
+/// A function object which can create a new stack frame.
+pub trait Fun {
+    /// Creates a new `vm::Frame` from this function object.
+    fn create_frame(&self, callee: ObjRef, vm: &Vm, _args: Vec<ObjRef>) -> Frame;
+}
+
 //
 // struct UserFun
 //
@@ -17,7 +23,7 @@ pub struct UserFun {
     attrs: Attrs,
     // Safe because Vec<Inst> doesn't need to be scanned
     #[shredder(unsafe_skip)]
-    code: Vec<Inst>,
+    code: Arc<Vec<Inst>>,
     // Safe because this is just an interner that points to symbols, which aren't GC'd
     #[shredder(unsafe_skip)]
     locals: FunLocals,
@@ -28,7 +34,7 @@ impl UserFun {
         let obj_ref = ObjRef::new(UserFun {
             vtable: Default::default(), // this is a placeholder for the real vtable
             attrs: Default::default(),
-            code,
+            code: Arc::new(code),
             locals,
         });
 
@@ -81,6 +87,10 @@ impl UserFun {
                     local.index().to_string(),
                     global_sym_lookup(self.locals()[local.index()]).unwrap().to_string(),
                 ),
+                Inst::LoadGlobal(global) => (
+                    global.index().to_string(),
+                    global_sym_lookup(globals[global.index()]).unwrap().to_string(),
+                ),
                 Inst::PopLocal(local) => {
                     if let Some(local) = local {
                         let index = local.index();
@@ -133,7 +143,38 @@ impl UserFun {
     }
 }
 
-impl_obj!(UserFun);
+impl Obj for UserFun {
+    fn vtable(&self) -> &Vtable {
+        &self.vtable
+    }
+
+    fn attrs(&self) -> &Attrs {
+        &self.attrs
+    }
+
+    fn attrs_mut(&mut self) -> Option<&mut Attrs> {
+        Some(&mut self.attrs)
+    }
+
+    fn as_any(&self) -> &dyn std::any::Any {
+        self
+    }
+
+    fn as_fun(&self) -> Option<&dyn Fun> {
+        Some(self)
+    }
+}
+
+impl Fun for UserFun {
+    fn create_frame(&self, callee: ObjRef, vm: &Vm, args: Vec<ObjRef>) -> Frame {
+        let bindings: FrameBindings = args.into_iter()
+            .enumerate()
+            .collect();
+        Frame::User(
+            UserFrame::new(callee, bindings, vm.pc(), Arc::clone(&self.code))
+        )
+    }
+}
 
 impl Debug for UserFun {
     fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
@@ -151,7 +192,7 @@ pub static USER_FUN_TY: Lazy<ObjRef<Ty>> = Lazy::new(|| Ty::new_obj(USER_FUN_NAM
 // struct NativeFun
 //
 
-pub type NativeFunPtr = Box<dyn (Fn(ObjRef, &mut Vm, Vec<ObjRef>) -> Signal) + Send + Sync>;
+pub type NativeFunPtr = fn(ObjRef, &mut Vm, Vec<ObjRef>) -> Signal;
 pub type NativeFunRef = ObjRef<NativeFun>;
 
 #[derive(Scan)]
@@ -207,24 +248,34 @@ impl Debug for NativeFun {
     }
 }
 
-impl_obj!(NativeFun);
+impl Obj for NativeFun {
+    fn vtable(&self) -> &Vtable {
+        &self.vtable
+    }
+
+    fn attrs(&self) -> &Attrs {
+        &self.attrs
+    }
+
+    fn attrs_mut(&mut self) -> Option<&mut Attrs> {
+        None
+    }
+
+    fn as_any(&self) -> &dyn std::any::Any {
+        self
+    }
+
+    fn as_fun(&self) -> Option<&dyn Fun> {
+        Some(self)
+    }
+}
+
+impl Fun for NativeFun {
+    fn create_frame(&self, callee: ObjRef, _vm: &Vm, args: Vec<ObjRef>) -> Frame {
+        Frame::Native(
+            NativeFrame::new(callee, *self.fun, args)
+        )
+    }
+}
 
 pub static NATIVE_FUN_TY: Lazy<ObjRef<Ty>> = Lazy::new(|| Ty::new_obj(NATIVE_FUN_NAME.sym_ref()));
-
-//
-// Native function defs
-//
-
-// __access__ is what the "dot" operator calls
-// __get_attr__ *should* always bypass the __access__ function and get an attribute directly
-
-pub static GET_ATTR_MEMBER_FUN: Lazy<ObjRef<NativeFun>> = Lazy::new(|| {
-    NativeFun::new_obj(Box::new(|_caller, _vm, _args| {
-        /*
-        let sym_ref = vm.pop();
-        let obj_ref = vm.pop();
-        obj_ref.access()
-        */
-        todo!("__get_attr__ function")
-    }))
-});

src/obj/mod.rs

@@ -25,6 +25,7 @@ use std::{
 };
 
 use attrs::*;
+use fun::Fun;
 use reserved::*;
 use sym::Sym;
 
@@ -54,6 +55,9 @@ pub trait Obj: Scan + std::fmt::Debug {
 
     fn as_any(&self) -> &dyn std::any::Any;
 
+    /// Gets this object as a `dyn Fun` reference.
+    fn as_fun(&self) -> Option<&dyn Fun> { None }
+
     obj_attr!(get_ty, TY_MEMBER_NAME);
     obj_attr!(get_call, CALL_MEMBER_NAME);
     obj_attr!(get_name, NAME_MEMBER_NAME);

src/vm/frame.rs

@@ -1,51 +1,107 @@
-use crate::obj::prelude::*;
+use crate::{obj::prelude::*, vm::inst::Inst};
-use shredder::{GcSafe, Scan, Scanner};
+use std::{collections::BTreeMap, sync::Arc};
-use std::collections::BTreeMap;
+use std::fmt::{self, Formatter, Debug};
 
-/// A stack call frame.
+pub type FrameBindings = BTreeMap<usize, ObjRef>;
-#[derive(Debug, Clone)]
+
-pub enum FrameKind {
+#[derive(Debug)]
-    Native(NativeFunRef, Vec<ObjRef>),
+pub enum Frame {
-    User {
+    User(UserFrame),
-        last_pc: usize,
+    Native(NativeFrame),
-        stack_base: usize,
-        fun: UserFunRef,
-    },
 }
 
-unsafe impl Scan for FrameKind {
+impl Frame {
-    fn scan(&self, scanner: &mut Scanner<'_>) {
+    pub fn user_frame(&self) -> Option<&UserFrame> {
-        match self {
+        if let Frame::User(frame) = self {
-            FrameKind::User { fun, .. } => scanner.scan(fun),
+            Some(frame)
-            _ => { /* no-op */ }
+        } else {
+            None
+        }
+    }
+
+    pub fn user_frame_mut(&mut self) -> Option<&mut UserFrame> {
+        if let Frame::User(frame) = self {
+            Some(frame)
+        } else {
+            None
+        }
+    }
+
+    pub fn native_frame(&self) -> Option<&NativeFrame> {
+        if let Frame::Native(frame) = self {
+            Some(frame)
+        } else {
+            None
+        }
+    }
+
+    pub fn native_frame_mut(&mut self) -> Option<&mut NativeFrame> {
+        if let Frame::Native(frame) = self {
+            Some(frame)
+        } else {
+            None
         }
     }
 }
 
-unsafe impl GcSafe for FrameKind {}
+#[derive(Debug, Clone)]
-
+pub struct UserFrame {
-#[derive(Scan, Debug, Clone)]
+    callee: ObjRef,
-pub struct Frame {
+    bindings: FrameBindings,
-    locals: FrameLocals,
+    last_pc: usize,
-    kind: FrameKind,
+    code: Arc<Vec<Inst>>,
 }
 
-impl Frame {
+impl UserFrame {
-    pub fn new(locals: FrameLocals, kind: FrameKind) -> Self {
+    pub fn new(callee: ObjRef, bindings: FrameBindings, last_pc: usize, code: Arc<Vec<Inst>>) -> Self {
-        Self { locals, kind, }
+        Self { callee, bindings, last_pc, code, }
     }
 
-    pub fn locals(&self) -> &FrameLocals {
+    pub fn bindings(&self) -> &FrameBindings {
-        &self.locals
+        &self.bindings
     }
 
-    pub fn locals_mut(&mut self) -> &mut FrameLocals {
+    pub fn bindings_mut(&mut self) -> &mut FrameBindings {
-        &mut self.locals
+        &mut self.bindings
     }
 
-    pub fn kind(&self) -> &FrameKind {
+    pub fn last_pc(&self) -> usize {
-        &self.kind
+        self.last_pc
+    }
+
+    pub fn code(&self) -> &Arc<Vec<Inst>> {
+        &self.code
+    }
+
+    pub fn callee(&self) -> &ObjRef {
+        &self.callee
     }
 }
 
-pub type FrameLocals = BTreeMap<usize, ObjRef>;
+pub struct NativeFrame {
+    callee: ObjRef,
+    fun_ptr: NativeFunPtr,
+    args: Vec<ObjRef>,
+}
+
+impl NativeFrame {
+    pub fn new(callee: ObjRef, fun_ptr: NativeFunPtr, args: Vec<ObjRef>) -> Self {
+        Self { callee, fun_ptr, args }
+    }
+
+    pub fn fun_ptr(&self) -> &NativeFunPtr {
+        &self.fun_ptr
+    }
+
+    pub fn callee(&self) -> &ObjRef {
+        &self.callee
+    }
+}
+
+impl Debug for NativeFrame {
+    fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
+        fmt.debug_struct("NativeFrame")
+            .field("fun_ptr", &format!("{:#x}", &self.fun_ptr as *const _ as usize))
+            .finish()
+    }
+}

src/vm/mod.rs

@@ -2,7 +2,7 @@ pub mod consts;
 pub mod error; // TODO : not needed?
 pub mod frame;
 pub mod inst;
-pub(crate) mod signal;
+pub mod signal;
 
 use crate::{
     obj::{reserved::*, prelude::*},
@@ -44,6 +44,11 @@ impl<'c> Vm<'c> {
         &self.frames
     }
 
+    /// Mutably gets the list of stack frames.
+    pub(crate) fn frames_mut(&mut self) -> &mut Vec<Frame> {
+        &mut self.frames
+    }
+
     /// Gets the stack.
     pub fn stack(&self) -> &Vec<ObjRef> {
         &self.stack
@@ -88,92 +93,85 @@ impl<'c> Vm<'c> {
 
     /// Resumes execution of the current program.
     pub fn resume(&mut self) {
-        while let Some(frame) = self.frame().cloned() {
+        while !self.frames().is_empty() {
-            let signal = match frame.kind() {
+            self.resume_until_return();
-                FrameKind::Native(fun, args) => {
+        }
-                    read_obj!(let fun_obj = fun);
+    }
-                    (fun_obj.fun())(fun.clone(), self, args.clone())
+
+    /// Resume execution until the currently executing function returns.
+    pub fn resume_until_return(&mut self) {
+        // resume execution until control is returned to the originally calling function
+        let start_frames = self.frames().len();
+        if start_frames == 0 {
+            return;
+        }
+
+        while self.frames().len() >= start_frames {
+            let frame = self.frame().unwrap();
+            let signal = match frame {
+                Frame::Native(fun) => {
+                    let callee = fun.callee().clone();
+                    (*fun.fun_ptr())(callee, self, vec![])
                 }
-                FrameKind::User { .. } => {
+                Frame::User(_) => {
-                    // run the user function until it returns control to the VM
+                    self.resume_user_fun()
-                    loop {
-                        if let Some(signal) = self.tick() {
-                            break signal;
-                        }
-                    }
                 }
             };
             self.handle_signal(signal);
         }
     }
 
+    fn resume_user_fun(&mut self) -> Signal {
+        while let Some(Frame::User(_)) = self.frame() {
+            if let Some(signal) = self.tick() {
+                return signal;
+            }
+        }
+        Signal::Return
+    }
+
     /// Handles a signal targeting the VM.
     ///
     /// The signal may originate from the VM itself, or from an external location. Signals
     /// generally interrupt control flow of a program.
     pub fn handle_signal(&mut self, signal: Signal) {
         match signal {
-            Signal::Call(caller, args) => {
+            Signal::Call(callee, args) => {
-                self.begin_call(caller, args);
+                read_obj!(let callee_obj = callee);
+                let frame = callee_obj.as_fun()
+                    .expect("callable function")
+                    .create_frame(callee.clone(), self, args);
+                self.frames_mut().push(frame);
+                // Jump to the first address of the new function call
+                self.set_pc(0);
             }
             Signal::Return => {
-                let retval = self.stack.pop()
+                // 1. pop return value
-                    .expect("return value");
+                let ret_val = self.pop().expect("return value");
-                match self.frames.pop().unwrap().kind() {
+
-                    // no-op; return value should be the TOS
+                // 2. pop stack frame
-                    FrameKind::Native(_, _) => {  },
+                let frame = self.frames_mut().pop().expect("stack frame");
-                    FrameKind::User { last_pc, stack_base, fun: _, } => {
+
-                        // pop return value, reset PC, clean stack, and push return value
+                // 3. reset PC
-                        self.set_pc(*last_pc);
+                if let Frame::User(frame) = frame {
-                        self.stack.truncate(*stack_base);
+                    self.set_pc(frame.last_pc());
-                    }
                 }
-                self.stack.push(retval);
+
+                // 4. push return value
+                self.push(ret_val);
             }
         }
     }
 
-    /// Sets up the stack for a function call.
-    fn begin_call(&mut self, caller: ObjRef, args: Vec<ObjRef>) {
-        // create stack frame
-        let stack_frame = if let Some(user_fun) = std::any::Any::downcast_ref::<UserFunRef>(&caller) {
-            let names: FrameLocals = {
-                read_obj!(let fun_ref = user_fun);
-                fun_ref.locals()
-                    .iter()
-                    .enumerate()
-                    .zip(args.into_iter())
-                    .map(|((index, _), arg)| (index, arg.clone()))
-                    .collect()
-            };
-            // TODO : check function arity vs argument count
-
-            Frame::new(names, FrameKind::User {
-                last_pc: self.pc(),
-                stack_base: self.stack().len(),
-                fun: user_fun.clone(),
-            })
-        } else if let Some(native_fun) = std::any::Any::downcast_ref::<NativeFunRef>(&caller) {
-            Frame::new(Default::default(), FrameKind::Native(native_fun.clone(), args))
-        } else {
-            // TODO : throw an error when error handling is figured out
-            panic!("can't call object {:?}", caller);
-        };
-
-        // push a new stack frame
-        self.frames.push(stack_frame);
-    }
-
     /// Gets the current instruction.
     #[inline(always)]
     fn load_inst(&self) -> Inst {
-        let user_fun = if let FrameKind::User { fun, .. } = self.frame().expect("frame").kind() {
+        let frame = if let Frame::User(frame) = self.frame().expect("frame") {
-            fun
+            frame
         } else {
             panic!("expected user function frame")
         };
-        read_obj!(let user_fun = user_fun);
+        frame.code()[self.pc()]
-        user_fun.code()[self.pc()]
     }
 
     /// Run a single instruction - inlined version.
@@ -196,28 +194,14 @@ impl<'c> Vm<'c> {
                 self.push(obj_ref);
             }
             Inst::LoadLocal(local) => {
-                let obj_ref = self.frames()
+                let value = self.get_local(local)
-                    .iter()
+                    .expect("TODO: throw error for missing local");
-                    .rev()
+                self.push(value);
-                    .filter_map(|frame| frame.locals().get(&local.index()))
-                    .cloned()
-                    .next();
-                if let Some(obj_ref) = obj_ref {
-                    self.push(obj_ref);
-                } else {
-                    todo!("TODO: implement \"local value not found\" lookup")
-                }
             }
             Inst::LoadGlobal(global) => {
-                let obj_ref = self.frames()
+                let value = self.get_global(global)
-                    .first()
+                    .expect("TODO: throw error for missing global");
-                    .and_then(|frame| frame.locals().get(&global.index()))
+                self.push(value);
-                    .cloned();
-                if let Some(obj_ref) = obj_ref {
-                    self.push(obj_ref);
-                } else {
-                    todo!("TODO: implement \"local value not found\" lookup")
-                }
             }
             Inst::PopLocal(name) => {
                 let tos = self.pop()
@@ -329,15 +313,40 @@ impl<'c> Vm<'c> {
         signal
     }
 
+    fn get_local(&self, name: Name) -> Option<ObjRef> {
+        self.frame()?
+            .user_frame()?
+            .bindings()
+            .get(&name.index())
+            .cloned()
+    }
+    
     fn set_local(&mut self, name: Name, value: ObjRef) {
-        let frame = self.frame_mut().unwrap();
+        let frame = self
-        let locals = frame.locals_mut();
+            .frame_mut()
-        locals.insert(name.index(), value);
+            .expect("user stack frame")
+            .user_frame_mut()
+            .unwrap();
+        let bindings = frame.bindings_mut();
+        bindings.insert(name.index(), value);
+    }
+
+    fn get_global(&self, name: Name) -> Option<ObjRef> {
+        self.frames
+            .first()?
+            .user_frame()?
+            .bindings()
+            .get(&name.index())
+            .cloned()
     }
 
     fn set_global(&mut self, name: Name, value: ObjRef) {
-        let frame = self.frames.first_mut().unwrap();
+        let frame = self.frames_mut()
-        let locals = frame.locals_mut();
+            .first_mut()
-        locals.insert(name.index(), value);
+            .expect("global stack frame")
+            .user_frame_mut()
+            .expect("user-defined function stack frame");
+        let bindings = frame.bindings_mut();
+        bindings.insert(name.index(), value);
     }
 }

src/vm/signal.rs

@@ -1,6 +1,7 @@
 use crate::obj::ObjRef;
 
 /// A signal from executing code in the VM.
+#[derive(Debug, Clone)]
 pub enum Signal {
     Call(ObjRef, Vec<ObjRef>),
     Return,