Add __call__, __init__, and fix a few stack bugs

* __call__ on a type will construct a new value for a couple of types,
  based on the arguments passed to that constructor. For example,
  `Str(foo)` just ends up calling `foo.to_str`. However, this opens up
  the door for more complex constructors.
*  __init__ is available, but for all objects that currently have it, it
   just does a no-op because they are copy-on-write, and are
   instantiated on creation.
* Builtin functions sometimes call other functions. However, when a VM
  would handle an `Op::Return`, it was expecting the callee function to
  be on top of the stack after discarding the stack items. A lot of the
  .call()s were not pushing the function to the stack beforehand, so
  this was causing stack misalignment when it really mattered. It went
  undetected until now because every function that was using .call() had
  stack items that were safe to discard.

Hopefully we should be in a good place to implement the rest of the
builtins that have not been implemented, and then we can start working
on implementing containers.

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

src/builtins.rs

@@ -45,6 +45,7 @@ pub(crate) fn println(vm: &mut Vm, state: FunctionState) -> FunctionResult {
                 .borrow()
                 .get_vtable_attr(obj.clone(), "to_str")
                 .expect("no to_str");
+            vm.push(method.clone());
             method.borrow().call(vm, 0);
             FunctionResult::Yield(0)
         }
@@ -64,6 +65,7 @@ pub(crate) fn print(vm: &mut Vm, state: FunctionState) -> FunctionResult {
                 .borrow()
                 .get_vtable_attr(obj.clone(), "to_str")
                 .expect("no to_str");
+            vm.push(method.clone());
             method.borrow().call(vm, 0);
             FunctionResult::Yield(0)
         }
@@ -93,6 +95,7 @@ impl BaseObj {
                     .get_vtable_attr(this.clone(), "to_repr")
                     .clone()
                     .expect("no to_repr");
+                vm.push(method.clone());
                 method.borrow().call(vm, 0);
                 FunctionResult::Yield(0)
             }
@@ -167,6 +170,7 @@ impl BaseObj {
                     .borrow()
                     .get_vtable_attr(obj.clone(), "to_bool")
                     .expect("no to_bool");
+                vm.push(method.clone());
                 method.borrow().call(vm, 0);
                 FunctionResult::Yield(0)
             }
@@ -207,6 +211,38 @@ impl BaseObj {
 // Str implementations
 ////////////////////////////////////////////////////////////////////////////////
 impl Str {
+    pub(crate) fn do_call(vm: &mut Vm, state: FunctionState) -> FunctionResult {
+        match state {
+            FunctionState::Begin => {
+                // get the top item off the stack and call to_str on it
+                let arg = vm.peek();
+                let method =
+                    if let Some(method) = arg.borrow().get_vtable_attr(arg.clone(), "to_str") {
+                        method
+                    } else {
+                        // TODO Str::do_call - throw exception when target doesn't have a to_str()
+                        // method
+                        // BLOCKED-ON: exceptions
+                        todo!("{} does not have a to_str() method", arg.borrow().ty_name());
+                        // NOTE - every object should have to_str, right?
+                    };
+                vm.push(method.clone());
+                method.borrow().call(vm, 0);
+
+                // resume execution
+                FunctionResult::Yield(0)
+            }
+            FunctionState::Resume(0) => FunctionResult::Return,
+            _ => unreachable!(),
+        }
+    }
+
+    pub(crate) fn init(_vm: &mut Vm, _state: FunctionState) -> FunctionResult {
+        // This is a no-op. We don't want the user-exposed `__init__` function to do anything,
+        // instantiation is done in the `__call__` function.
+        FunctionResult::ReturnPush(Nil::create())
+    }
+
     pub(crate) fn to_str(_vm: &mut Vm, _state: FunctionState) -> FunctionResult {
         // top item of the stack should just be ourselves, so return immediately
         FunctionResult::Return
@@ -344,6 +380,38 @@ impl Int {
         Float::create(int_value as f64).into()
     }
 
+    pub(crate) fn do_call(vm: &mut Vm, state: FunctionState) -> FunctionResult {
+        match state {
+            FunctionState::Begin => {
+                // Pop the argument and call `to_int` on it
+                let arg = vm.peek();
+
+                let method =
+                    if let Some(method) = arg.borrow().get_vtable_attr(arg.clone(), "to_int") {
+                        method
+                    } else {
+                        // TODO Int::do_call - throw exception when arg doesn't have to_int()
+                        // method
+                        // BLOCKED-ON: exceptions
+                        todo!("{} does not have a to_int() method", arg.borrow().ty_name());
+                    };
+
+                vm.push(method.clone());
+                method.borrow().call(vm, 0);
+
+                FunctionResult::Yield(0)
+            }
+            FunctionState::Resume(0) => FunctionResult::Return,
+            _ => unreachable!(),
+        }
+    }
+
+    pub(crate) fn init(_vm: &mut Vm, _state: FunctionState) -> FunctionResult {
+        // This is a no-op. We don't want the user-exposed `__init__` function to do anything,
+        // instantiation is done in the `__call__` function.
+        FunctionResult::ReturnPush(Nil::create())
+    }
+
     int_bin_op_math!(add, +);
     int_bin_op_math!(sub, -);
 
@@ -454,6 +522,40 @@ macro_rules! float_bin_op_logical {
 }
 
 impl Float {
+    pub(crate) fn do_call(vm: &mut Vm, state: FunctionState) -> FunctionResult {
+        match state {
+            FunctionState::Begin => {
+                // get the top item off the stack and call to_float on it
+                let arg = vm.peek();
+                let method =
+                    if let Some(method) = arg.borrow().get_vtable_attr(arg.clone(), "to_float") {
+                        method
+                    } else {
+                        // TODO Float::do_call - throw exception when target doesn't have a to_float()
+                        // method
+                        // BLOCKED-ON: exceptions
+                        todo!(
+                            "{} does not have a to_float() method",
+                            arg.borrow().ty_name()
+                        );
+                    };
+                vm.push(method.clone());
+                method.borrow().call(vm, 0);
+
+                // resume execution
+                FunctionResult::Yield(0)
+            }
+            FunctionState::Resume(0) => FunctionResult::Return,
+            _ => unreachable!(),
+        }
+    }
+
+    pub(crate) fn init(_vm: &mut Vm, _state: FunctionState) -> FunctionResult {
+        // This is a no-op. We don't want the user-exposed `__init__` function to do anything,
+        // instantiation is done in the `__call__` function.
+        FunctionResult::ReturnPush(Nil::create())
+    }
+
     pub(crate) fn to_int(vm: &mut Vm, _state: FunctionState) -> FunctionResult {
         let float_value = with_obj_downcast(vm.frame_stack()[0].clone(), Float::float_value);
         Int::create(float_value as i64).into()

src/obj.rs

@@ -126,6 +126,11 @@ pub fn init_types() {
             to_float => BuiltinFunction::create("to_float", BaseObj::not_implemented_un, 1),
             len => BuiltinFunction::create("len", BaseObj::not_implemented_un, 1),
 
+            // Constructor
+            // TODO Ty::do_call, Ty::init - implement these methods
+            __call__ => BuiltinFunction::create("__call__", BaseObj::not_implemented_un, 1),
+            __init__ => BuiltinFunction::create("__init__", BaseObj::not_implemented_un, 1),
+
             // Operators
             __add__ => BuiltinFunction::create("__add__", BaseObj::not_implemented_bin, 2),
             __sub__ => BuiltinFunction::create("__sub__", BaseObj::not_implemented_bin, 2),
@@ -143,7 +148,9 @@ pub fn init_types() {
             __neg__ => BuiltinFunction::create("__neg__", BaseObj::not_implemented_un, 1),
             __not__ => BuiltinFunction::create("__not__", BaseObj::not, 1),
         },
-        Obj { },
+        Obj {
+            //__call__ => BuiltinFunction::create("__call__",
+        },
         Str {
             // Conversion methods
             to_str => BuiltinFunction::create("to_str", Str::to_str, 1),
@@ -151,6 +158,10 @@ pub fn init_types() {
             to_float => BuiltinFunction::create("to_float", Str::to_float, 1),
             len => BuiltinFunction::create("len", Str::len, 1),
 
+            // Constructor
+            __call__ => BuiltinFunction::create("__call__", Str::do_call, 2),
+            __init__ => BuiltinFunction::create("__init__", Str::init, 2),
+
             // Operators
             __add__ => BuiltinFunction::create("__add__", Str::add, 2),
             __mul__ => BuiltinFunction::create("__mul__", Str::mul, 2),
@@ -161,6 +172,10 @@ pub fn init_types() {
             to_int => BuiltinFunction::create("to_int", Int::to_int, 1),
             to_float => BuiltinFunction::create("to_float", Int::to_float, 1),
 
+            // Constructor
+            __call__ => BuiltinFunction::create("__call__", Int::do_call, 2),
+            __init__ => BuiltinFunction::create("__init__", Int::init, 2),
+
             // Operators
             __add__ => BuiltinFunction::create("__add__", Int::add, 2),
             __sub__ => BuiltinFunction::create("__sub__", Int::sub, 2),
@@ -179,6 +194,10 @@ pub fn init_types() {
             to_int => BuiltinFunction::create("to_int", Float::to_int, 1),
             to_float => BuiltinFunction::create("to_float", Float::to_float, 1),
 
+            // Constructor
+            __call__ => BuiltinFunction::create("__call__", Float::do_call, 2),
+            __init__ => BuiltinFunction::create("__init__", Float::init, 2),
+
             // Operators
             __add__ => BuiltinFunction::create("__add__", Float::add, 2),
             __sub__ => BuiltinFunction::create("__sub__", Float::sub, 2),
@@ -279,7 +298,14 @@ pub trait Object: Debug + Display + Any + Trace {
     fn call(&self, _vm: &mut Vm, _argc: Argc) {
         // TODO Object::call - need to handle "this object cannot be called" errors
         // BLOCKED-ON: exceptions
-        todo!("Raise some kind of not implemented/not callable error for non-callable objects")
+        /*
+        let method = self.get_vtable_attr("__call__").expect(
+            "TODO: Raise some kind of not implemented/not callable error for non-callable objects",
+        );
+        */
+        todo!(
+            "TODO: Raise some kind of not implemented/not callable error for non-callable objects"
+        )
     }
 
     fn is_truthy(&self) -> bool {
@@ -428,18 +454,18 @@ impl Ty {
         }
     }
 
-    impl_create!(name: impl ToString);
-
     pub fn name(&self) -> &Rc<String> {
         &self.name
     }
+
+    impl_create!(name: impl ToString);
 }
 
 impl Debug for Ty {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         write!(
             fmt,
-            "<Ty {} at {:x}>",
+            "<Ty {} at {:#x}>",
             self.name,
             (self as *const _ as usize)
         )
@@ -448,12 +474,7 @@ impl Debug for Ty {
 
 impl Display for Ty {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        write!(
+        Debug::fmt(self, fmt)
-            fmt,
-            "<Ty {} at {:x}>",
-            self.name,
-            (self as *const _ as usize)
-        )
     }
 }
 
@@ -472,6 +493,43 @@ impl Object for Ty {
         }
     }
 
+    fn call(&self, vm: &mut Vm, argc: Argc) {
+        // TODO Object::call - need to handle "this object cannot be called" errors
+        // BLOCKED-ON: exceptions
+
+        // I don't think there's any way we could call this *without* it being a method.
+        // If you do e.g. `Int.__call__`, Int is an object, so it should be doing `__call__` as a
+        // vtable value.
+
+        if cfg!(debug_assertions) {
+            let index = vm.stack().len() - 1 - argc as usize;
+            let this = vm.stack()[index].clone();
+            assert!(
+                std::ptr::addr_eq(&*this.borrow(), self),
+                "calling {}.__call__ on type that is not ourselves",
+                self.ty_name()
+            );
+        }
+
+        let function = self
+            .vtable
+            .get("__call__")
+            .expect("Why does a type not have a __call__ member?");
+        function.borrow().call(vm, argc);
+    }
+
+    fn arity(&self) -> Option<Argc> {
+        // HACK XXX NOTE Ty __call__ arity :
+        // We need to tread carefully here. Normally, `__call__` would be wrapped as a method.
+        // However, we have to get the `__call__` member directly from the vtable.
+        // We are subtracting 1 from the arity, because whenever it *does* become a method, the
+        // arity will match when we call `Ty` directly.
+        self.vtable
+            .get("__call__")
+            .and_then(|function| function.borrow().arity())
+            .map(|n| n - 1)
+    }
+
     impl_base_obj!(Ty);
 }
 
@@ -536,7 +594,7 @@ impl Object for Str {
 #[derive(Trace, Finalize)]
 pub struct Int {
     base: BaseObj,
-    int_value: i64,
+    pub(crate) int_value: i64,
 }
 
 impl Int {

src/vm.rs

@@ -357,8 +357,10 @@ impl Vm {
                         //                          does not match the function's arity
                         // BLOCKED-ON: exceptions
                         todo!(
-                            "throw an error because we passed the wrong number of arguments to {}",
+                            "throw an error because we passed the wrong number of arguments to {} (got {}, expected {})",
-                            fun_ptr.borrow()
+                            fun_ptr.borrow(),
+                            argc,
+                            arity
                         );
                     }
                     fun_ptr.borrow().call(self, argc as Argc);