Remove BaseObj and fix Object::equals

* BaseObj felt a bit redundant. For everything that BaseObj did, we use
  Obj instead.
* Object::equals was a little weird. It was used for giving back
  equality, except when it wasn't. It's a little better defined now,
  here's what I'm shooting for:
    * *In general*, Object::equals will return true when two objects
      refer to the same object.
    * The exception to this rule is for "constant" objects, or "copy on
      write" objects. These include, but are not limited to: Int, Float,
      Bool, Nil, Str. Their base values are immutable and are the heart
      of object equality.

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

src/obj.rs

@@ -201,7 +201,22 @@ pub trait Object: Debug + Display + Any + Trace {
         true
     }
 
-    fn equals(&self, other: &dyn Object) -> bool;
+    /// Check identity equality - kinda.
+    ///
+    /// Generally, you should not be using this function for equality. *For the most part*, it will
+    /// check referential equality. The only exceptions are:
+    ///
+    /// * `Int` values
+    /// * `Float` values
+    /// * `Bool` values
+    /// * `Str` values
+    ///
+    /// Additionally, if an `Int` is compared to a `Float` or vice versa, they could potentially be
+    /// seen as equal too.
+    fn equals(&self, other: &dyn Object) -> bool {
+        // by default check referential equality
+        std::ptr::addr_eq(self, other)
+    }
 
     fn as_any(&self) -> &dyn Any;
 
@@ -209,31 +224,41 @@ pub trait Object: Debug + Display + Any + Trace {
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// BaseObj
+// Obj
 ////////////////////////////////////////////////////////////////////////////////
 
-#[derive(Default, Debug, Trace, Finalize)]
+#[derive(Clone, Default, Trace, Finalize)]
-pub(crate) struct BaseObj {
+pub struct Obj {
     attrs: Attrs,
     is_instantiated: bool,
 }
 
-impl Clone for BaseObj {
+impl Obj {
-    fn clone(&self) -> Self {
+    pub fn new() -> Self {
-        Self {
+        Default::default()
-            attrs: self.attrs.clone(),
-            is_instantiated: self.is_instantiated,
-        }
     }
+
+    impl_create!();
 }
 
-impl Display for BaseObj {
+impl Display for Obj {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        write!(fmt, "<BaseObj at {:#x}>", (self as *const _ as usize))
+        Debug::fmt(self, fmt)
     }
 }
 
-impl Object for BaseObj {
+impl Debug for Obj {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        write!(
+            fmt,
+            "<{} at {:#x}>",
+            self.ty_name(),
+            (self as *const _ as usize)
+        )
+    }
+}
+
+impl Object for Obj {
     fn instantiate(&mut self) {
         self.is_instantiated = true;
     }
@@ -251,7 +276,7 @@ impl Object for BaseObj {
     }
 
     fn equals(&self, other: &dyn Object) -> bool {
-        if let Some(other) = other.as_any().downcast_ref::<BaseObj>() {
+        if let Some(other) = other.as_any().downcast_ref::<Obj>() {
             // compare all attrs
             self.attrs.iter().all(|(k1, v1)| {
                 other
@@ -275,10 +300,10 @@ impl Object for BaseObj {
 }
 
 //
-// BaseObj implementations
+// Obj methods
 //
 
-impl BaseObj {
+impl Obj {
     //
     // Common functions
     //
@@ -385,7 +410,7 @@ impl BaseObj {
 
     pub(crate) fn not_implemented_un(vm: &mut Vm, _state: FunctionState) -> FunctionResult {
         let fname = &vm.frame().name;
-        // TODO BaseObj::not_implemented_un - throw an exception of some kind for not
+        // TODO Obj::not_implemented_un - throw an exception of some kind for not
         // implemented/not available errors on unary operators
         // BLOCKED-ON: exceptions
         todo!(
@@ -396,67 +421,17 @@ impl BaseObj {
     }
 
     pub(crate) fn not_implemented_bin(vm: &mut Vm, _state: FunctionState) -> FunctionResult {
-        // TODO BaseObj::not_implemented_un - throw an exception of some kind for not
+        // TODO Obj::not_implemented_un - throw an exception of some kind for not
         // implemented/not available errors on unary operators
         // BLOCKED-ON: exceptions
         let fname = &vm.frame().name;
         todo!("Raise some kind of not implemented/not callable error for {} function (self: {}, rhs: {})", fname, vm.frame_stack()[0].borrow(), vm.frame_stack()[1].borrow())
     }
-}
 
-////////////////////////////////////////////////////////////////////////////////
+    //
-// Obj
+    // Do call
-////////////////////////////////////////////////////////////////////////////////
+    //
 
-#[derive(Trace, Finalize)]
-pub struct Obj {
-    base: BaseObj,
-}
-
-impl Obj {
-    pub fn new() -> Self {
-        Self {
-            base: Default::default(),
-        }
-    }
-
-    impl_create!();
-}
-
-impl Display for Obj {
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        Debug::fmt(self, fmt)
-    }
-}
-
-impl Debug for Obj {
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        write!(
-            fmt,
-            "<{} at {:#x}>",
-            self.ty_name(),
-            (self as *const _ as usize)
-        )
-    }
-}
-
-impl Object for Obj {
-    fn equals(&self, other: &dyn Object) -> bool {
-        if let Some(other) = other.as_any().downcast_ref::<Obj>() {
-            self.base.equals(&other.base)
-        } else {
-            false
-        }
-    }
-
-    impl_base_obj!(Obj);
-}
-
-//
-// Obj methods
-//
-
-impl Obj {
     pub(crate) fn do_call(vm: &mut Vm, state: FunctionState) -> FunctionResult {
         let ty = vm.frame_stack()[0].clone();
 
@@ -523,7 +498,7 @@ impl Obj {
 
 #[derive(Default, Trace, Finalize)]
 pub struct Nil {
-    base: BaseObj,
+    base: Obj,
 }
 
 impl Nil {

src/obj/bool.rs

@@ -4,12 +4,11 @@ use gc::{Finalize, Trace};
 
 use crate::obj::macros::*;
 use crate::obj::prelude::*;
-use crate::obj::BaseObj;
 use crate::vm::Vm;
 
 #[derive(Trace, Finalize)]
 pub struct Bool {
-    base: BaseObj,
+    base: Obj,
     bool_value: bool,
 }
 

src/obj/float.rs

@@ -4,12 +4,11 @@ use gc::{Finalize, Trace};
 
 use crate::obj::macros::*;
 use crate::obj::prelude::*;
-use crate::obj::BaseObj;
 use crate::vm::Vm;
 
 #[derive(Trace, Finalize)]
 pub struct Float {
-    base: BaseObj,
+    base: Obj,
     float_value: f64,
 }
 

src/obj/function.rs

@@ -1,11 +1,10 @@
 use std::fmt::{self, Debug, Display};
-use std::ptr;
 use std::rc::Rc;
 
 use gc::{Finalize, Trace};
 
 use crate::obj::macros::*;
-use crate::obj::{make_ptr, BaseObj, ObjP, Object};
+use crate::obj::prelude::*;
 use crate::vm::{Argc, Chunk, Frame, Function, Vm};
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -56,7 +55,7 @@ pub type BuiltinFunctionPtr = fn(vm: &mut Vm, function_state: FunctionState) ->
 
 #[derive(Trace, Finalize)]
 pub struct BuiltinFunction {
-    base: BaseObj,
+    base: Obj,
     #[unsafe_ignore_trace]
     name: Rc<String>,
     #[unsafe_ignore_trace]
@@ -117,16 +116,6 @@ impl Object for BuiltinFunction {
         vm.push_frame(new_frame);
     }
 
-    fn equals(&self, other: &dyn Object) -> bool {
-        // TODO BuiltinFunction::equals : need something more robust than checking addr_eq,
-        // maybe check the self_binding pointer too?
-        if let Some(other) = other.as_any().downcast_ref::<BuiltinFunction>() {
-            ptr::addr_eq(self, other)
-        } else {
-            false
-        }
-    }
-
     impl_base_obj!(BuiltinFunction);
 }
 
@@ -136,7 +125,7 @@ impl Object for BuiltinFunction {
 
 #[derive(Clone, Trace, Finalize)]
 pub struct UserFunction {
-    base: BaseObj,
+    base: Obj,
     #[unsafe_ignore_trace]
     path: Rc<String>,
     #[unsafe_ignore_trace]
@@ -218,15 +207,6 @@ impl Object for UserFunction {
         }
     }
 
-    fn equals(&self, other: &dyn Object) -> bool {
-        if let Some(other) = other.as_any().downcast_ref::<UserFunction>() {
-            // TODO UserFunction::equals : need something more robust than checking addr_eq.
-            ptr::addr_eq(self, other)
-        } else {
-            false
-        }
-    }
-
     impl_base_obj!(UserFunction);
 }
 
@@ -236,7 +216,7 @@ impl Object for UserFunction {
 
 #[derive(Trace, Finalize)]
 pub struct Method {
-    base: BaseObj,
+    base: Obj,
     self_binding: ObjP,
     function: ObjP,
 }
@@ -312,14 +292,5 @@ impl Object for Method {
         self.function.borrow().call(vm, argc)
     }
 
-    fn equals(&self, other: &dyn Object) -> bool {
-        if let Some(other) = other.as_any().downcast_ref::<Method>() {
-            ptr::addr_eq(&*self.self_binding(), &*other.self_binding())
-                && ptr::addr_eq(&*self.function, &*other.function)
-        } else {
-            false
-        }
-    }
-
     impl_base_obj!(Method);
 }

src/obj/int.rs

@@ -4,12 +4,11 @@ use gc::{Finalize, Trace};
 
 use crate::obj::macros::*;
 use crate::obj::prelude::*;
-use crate::obj::BaseObj;
 use crate::vm::Vm;
 
 #[derive(Trace, Finalize)]
 pub struct Int {
-    base: BaseObj,
+    base: Obj,
     pub(crate) int_value: i64,
 }
 

src/obj/list.rs

@@ -4,12 +4,11 @@ use gc::{Finalize, Trace};
 
 use crate::obj::macros::*;
 use crate::obj::prelude::*;
-use crate::obj::BaseObj;
 use crate::vm::Vm;
 
 #[derive(Trace, Finalize)]
 pub struct List {
-    base: BaseObj,
+    base: Obj,
     list: Vec<ObjP>,
 }
 
@@ -55,19 +54,6 @@ impl Debug for List {
 }
 
 impl Object for List {
-    fn equals(&self, other: &dyn Object) -> bool {
-        if let Some(other) = other.as_any().downcast_ref::<List>() {
-            self.list.len() == other.list.len()
-                && self
-                    .list
-                    .iter()
-                    .zip(other.list.iter())
-                    .all(|(me, you)| me.borrow().equals(&*you.borrow()))
-        } else {
-            false
-        }
-    }
-
     impl_base_obj!(List);
 }
 

src/obj/module.rs

@@ -5,12 +5,12 @@ use gc::{Finalize, Trace};
 
 use crate::obj::macros::*;
 use crate::obj::prelude::*;
-use crate::obj::BaseObj;
+use crate::obj::Obj;
 use crate::vm::Chunk;
 
 #[derive(Trace, Finalize)]
 pub struct Module {
-    base: BaseObj,
+    base: Obj,
     #[unsafe_ignore_trace]
     path: Rc<String>,
     #[unsafe_ignore_trace]
@@ -66,15 +66,6 @@ impl Display for Module {
 }
 
 impl Object for Module {
-    fn equals(&self, other: &dyn Object) -> bool {
-        if let Some(other) = other.as_any().downcast_ref::<Module>() {
-            // only referential identity
-            std::ptr::addr_eq(self, other)
-        } else {
-            false
-        }
-    }
-
     impl_base_obj!(Module);
 }
 

src/obj/str.rs

@@ -5,12 +5,11 @@ use gc::{Finalize, Trace};
 
 use crate::obj::macros::*;
 use crate::obj::prelude::*;
-use crate::obj::BaseObj;
 use crate::vm::Vm;
 
 #[derive(Trace, Finalize)]
 pub struct Str {
-    base: BaseObj,
+    base: Obj,
     #[unsafe_ignore_trace]
     str_value: Rc<String>,
 }

src/obj/ty.rs

@@ -4,14 +4,12 @@ use std::rc::Rc;
 
 use gc::{Finalize, Trace};
 
-use crate::obj::macros::*;
+use crate::obj::{macros::*, prelude::*, Obj, BUILTINS};
-use crate::obj::prelude::*;
-use crate::obj::{BaseObj, BUILTINS};
 use crate::vm::{Argc, Vm};
 
 #[derive(Trace, Finalize)]
 pub struct Ty {
-    base: BaseObj,
+    base: Obj,
     #[unsafe_ignore_trace]
     name: Rc<String>,
     vtable: HashMap<String, ObjP>,
@@ -55,20 +53,6 @@ impl Display for Ty {
 }
 
 impl Object for Ty {
-    fn equals(&self, other: &dyn Object) -> bool {
-        if let Some(other) = other.as_any().downcast_ref::<Ty>() {
-            // TODO Ty::equals : something more robust than this
-            // Tys should hold equality if they have the same name
-            // the problem is that Ty.get_attr("__ty__") is going to return itself, so we have
-            // to go through attributes to specially exclude to the __ty__ attribute if it points
-            // to ourself.
-            // How do we detect that it's pointing to ourself? I suppose pointers are the way
-            self.name == other.name
-        } else {
-            false
-        }
-    }
-
     fn call(&self, vm: &mut Vm, argc: Argc) {
         // TODO Object::call - need to handle "this object cannot be called" errors
         // BLOCKED-ON: exceptions
@@ -167,54 +151,56 @@ pub fn init_types() {
         base_type: Ty,
         // type definitions
         Ty {
-            // Conversion methods
-            to_str => BuiltinFunction::create("to_str", BaseObj::to_str, 1),
-            to_repr => BuiltinFunction::create("to_repr", BaseObj::to_repr, 1),
-            to_bool => BuiltinFunction::create("to_bool", BaseObj::to_bool, 1),
-            to_int => BuiltinFunction::create("to_int", BaseObj::not_implemented_un, 1),
-            to_float => BuiltinFunction::create("to_float", BaseObj::not_implemented_un, 1),
-            to_list => BuiltinFunction::create("to_list", BaseObj::not_implemented_un, 1),
-
             // Constructor
             // TODO Ty::do_call, Ty::init - implement these methods
-            __call__ => BuiltinFunction::create("__call__", BaseObj::not_implemented_un, 1),
+            __call__ => BuiltinFunction::create("__call__", Obj::not_implemented_un, 1),
-            __init__ => BuiltinFunction::create("__init__", BaseObj::not_implemented_un, 1),
+            __init__ => BuiltinFunction::create("__init__", Obj::not_implemented_un, 1),
+
+            // Conversion methods
+            to_str => BuiltinFunction::create("to_str", Obj::to_str, 1),
+            to_repr => BuiltinFunction::create("to_repr", Obj::to_repr, 1),
+            to_bool => BuiltinFunction::create("to_bool", Obj::to_bool, 1),
+            to_int => BuiltinFunction::create("to_int", Obj::not_implemented_un, 1),
+            to_float => BuiltinFunction::create("to_float", Obj::not_implemented_un, 1),
+            to_list => BuiltinFunction::create("to_list", Obj::not_implemented_un, 1),
 
             // Operators
-            __add__ => BuiltinFunction::create("__add__", BaseObj::not_implemented_bin, 2),
+            __add__ => BuiltinFunction::create("__add__", Obj::not_implemented_bin, 2),
-            __sub__ => BuiltinFunction::create("__sub__", BaseObj::not_implemented_bin, 2),
+            __sub__ => BuiltinFunction::create("__sub__", Obj::not_implemented_bin, 2),
-            __mul__ => BuiltinFunction::create("__mul__", BaseObj::not_implemented_bin, 2),
+            __mul__ => BuiltinFunction::create("__mul__", Obj::not_implemented_bin, 2),
-            __div__ => BuiltinFunction::create("__div__", BaseObj::not_implemented_bin, 2),
+            __div__ => BuiltinFunction::create("__div__", Obj::not_implemented_bin, 2),
-            __and__ => BuiltinFunction::create("__and__", BaseObj::and, 2),
+            __and__ => BuiltinFunction::create("__and__", Obj::and, 2),
-            __or__ => BuiltinFunction::create("__or__", BaseObj::or, 2),
+            __or__ => BuiltinFunction::create("__or__", Obj::or, 2),
-            __ne__ => BuiltinFunction::create("__ne__", BaseObj::ne, 2),
+            __ne__ => BuiltinFunction::create("__ne__", Obj::ne, 2),
-            __eq__ => BuiltinFunction::create("__eq__", BaseObj::eq, 2),
+            __eq__ => BuiltinFunction::create("__eq__", Obj::eq, 2),
-            __gt__ => BuiltinFunction::create("__gt__", BaseObj::not_implemented_bin, 2),
+            __gt__ => BuiltinFunction::create("__gt__", Obj::not_implemented_bin, 2),
-            __ge__ => BuiltinFunction::create("__ge__", BaseObj::not_implemented_bin, 2),
+            __ge__ => BuiltinFunction::create("__ge__", Obj::not_implemented_bin, 2),
-            __lt__ => BuiltinFunction::create("__lt__", BaseObj::not_implemented_bin, 2),
+            __lt__ => BuiltinFunction::create("__lt__", Obj::not_implemented_bin, 2),
-            __le__ => BuiltinFunction::create("__le__", BaseObj::not_implemented_bin, 2),
+            __le__ => BuiltinFunction::create("__le__", Obj::not_implemented_bin, 2),
-            __pos__ => BuiltinFunction::create("__pos__", BaseObj::not_implemented_un, 1),
+            __pos__ => BuiltinFunction::create("__pos__", Obj::not_implemented_un, 1),
-            __neg__ => BuiltinFunction::create("__neg__", BaseObj::not_implemented_un, 1),
+            __neg__ => BuiltinFunction::create("__neg__", Obj::not_implemented_un, 1),
-            __not__ => BuiltinFunction::create("__not__", BaseObj::not, 1),
+            __not__ => BuiltinFunction::create("__not__", Obj::not, 1),
 
             // Methods
-            len => BuiltinFunction::create("len", BaseObj::not_implemented_un, 1),
+            len => BuiltinFunction::create("len", Obj::not_implemented_un, 1),
+            hash => BuiltinFunction::create("hash", Obj::not_implemented_bin, 2),
         },
         Obj {
             // Constructor
             __call__ => BuiltinFunction::create("__call__", Obj::do_call, 1),
             __init__ => BuiltinFunction::create("__init__", Obj::init, 1),
+
             // Methods
         },
         List {
-            // Conversion methods
-            to_repr => BuiltinFunction::create("to_repr", List::to_repr, 1),
-            to_list => BuiltinFunction::create("to_list", List::to_list, 1),
-
             // Constructor
             __call__ => BuiltinFunction::create("__call__", List::do_call, 2),
             __init__ => BuiltinFunction::create("__init__", List::init, 2),
 
+            // Conversion methods
+            to_repr => BuiltinFunction::create("to_repr", List::to_repr, 1),
+            to_list => BuiltinFunction::create("to_list", List::to_list, 1),
+
             // Operators
             __index__ => BuiltinFunction::create("__index__", List::index, 2),
 
@@ -226,16 +212,16 @@ pub fn init_types() {
             extend => BuiltinFunction::create("extend", List::extend, 2),
         },
         Str {
+            // Constructor
+            __call__ => BuiltinFunction::create("__call__", Str::do_call, 2),
+            __init__ => BuiltinFunction::create("__init__", Str::init, 2),
+
             // Conversion methods
             to_str => BuiltinFunction::create("to_str", Str::to_str, 1),
             to_int => BuiltinFunction::create("to_int", Str::to_int, 1),
             to_float => BuiltinFunction::create("to_float", Str::to_float, 1),
             to_list => BuiltinFunction::create("to_list", Str::to_list, 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),
@@ -247,14 +233,14 @@ pub fn init_types() {
             // TODO Str methods - .lower, .upper, .slice, etc
         },
         Int {
-            // Conversion methods
-            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),
 
+            // Conversion methods
+            to_int => BuiltinFunction::create("to_int", Int::to_int, 1),
+            to_float => BuiltinFunction::create("to_float", Int::to_float, 1),
+
             // Operators
             __add__ => BuiltinFunction::create("__add__", Int::add, 2),
             __sub__ => BuiltinFunction::create("__sub__", Int::sub, 2),
@@ -270,14 +256,14 @@ pub fn init_types() {
             // Methods
         },
         Float {
-            // Conversion methods
-            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),
 
+            // Conversion methods
+            to_int => BuiltinFunction::create("to_int", Float::to_int, 1),
+            to_float => BuiltinFunction::create("to_float", Float::to_float, 1),
+
             // Operators
             __add__ => BuiltinFunction::create("__add__", Float::add, 2),
             __sub__ => BuiltinFunction::create("__sub__", Float::sub, 2),
@@ -292,48 +278,48 @@ pub fn init_types() {
             // Methods
         },
         Bool {
-            // Conversion methods
-            to_int => BuiltinFunction::create("to_int", Bool::to_int, 1),
-            to_float => BuiltinFunction::create("to_float", Bool::to_float, 1),
-
             // Constructor
             __call__ => BuiltinFunction::create("__call__", Bool::do_call, 2),
             __init__ => BuiltinFunction::create("__init__", Bool::init, 2),
 
+            // Conversion methods
+            to_int => BuiltinFunction::create("to_int", Bool::to_int, 1),
+            to_float => BuiltinFunction::create("to_float", Bool::to_float, 1),
+
             // Operators
             // Methods
         },
         Nil {
-            // Conversion methods
-
             // Constructor
             __call__ => BuiltinFunction::create("__call__", Nil::do_call, 1),
             __init__ => BuiltinFunction::create("__init__", Nil::init, 1),
 
+            // Conversion methods
+
             // Operators
             // Methods
         },
         BuiltinFunction {
-            // Conversion methods
             // Constructor
+            // Conversion methods
             // Operators
             // Methods
         },
         UserFunction {
-            // Conversion methods
             // Constructor
+            // Conversion methods
             // Operators
             // Methods
         },
         Method {
-            // Conversion methods
             // Constructor
+            // Conversion methods
             // Operators
             // Methods
         },
         Module {
-            // Conversion methods
             // Constructor
+            // Conversion methods
             // Operators
             // Methods
         },