Change to_repr/to_str implementation story

Let's talk about to_repr and to_str.

to_repr tries to do what Python's `repr` function does - that is, it
converts an object into a developer-readable (but maybe not
human-readable) string. This function is implemented for every object,
and may very well just write out "<MyType at 0x12345678>".

to_str, on the other hand, tries to turn an object into an explicitly
human-readable format. In Python (which we are modeling a lot of our
design after), the str() function usually will end up calling `repr()`
itself, if no other implementation has been provided.

Previously in our implementation, there was a bit of a disconnect
between `to_repr` and `to_str`, versus `Debug` and `Display`. `to_repr`
would kind of do its own thing, and then maybe call either `Display` or
`Debug` to format an object. Consequently, `to_str` would kind of do its
own thing too - usually calling `to_repr` but not always.

This change attempts to strengthen the definitions of `to_repr` and
`to_str`. *In general*, a call to `to_repr` should be calling an
object's `Debug::fmt` function, and *in general* a call to `to_str()`
should be calling an object's `Display::fmt` function. Often, the
`Display::fmt` will just end up calling `Debug::fmt` itself, but now
the `to_str()` and `to_repr()` interfaces are much better defined than
they used to be.

The only major downside is that we are giving up the `Debug`
implementation for language logic, rather than
debugging-the-language-itself logic. I can see this biting us down the
road if we ever need a Rust-style `Debug` implementation, but for now, I
think this is going to serve our needs just fine.

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

src/builtins.rs

@@ -102,8 +102,7 @@ impl BaseObj {
     }
 
     pub(crate) fn to_repr(vm: &mut Vm, _state: FunctionState) -> FunctionResult {
-        let str_value = format!("{}", vm.frame_stack()[0].borrow());
+        Str::create(format!("{:?}", vm.frame_stack()[0].borrow())).into()
-        Str::create(str_value).into()
     }
 
     pub(crate) fn to_bool(vm: &mut Vm, _state: FunctionState) -> FunctionResult {
@@ -213,13 +212,6 @@ impl Str {
         FunctionResult::Return
     }
 
-    pub(crate) fn to_repr(vm: &mut Vm, _state: FunctionState) -> FunctionResult {
-        let escaped: String = with_obj_downcast(vm.frame_stack()[0].clone(), |str_inst: &Str| {
-            str_inst.str_value().as_str().escape_default().collect()
-        });
-        Str::create(format!("'{}'", escaped)).into()
-    }
-
     pub(crate) fn to_int(vm: &mut Vm, _state: FunctionState) -> FunctionResult {
         let parsed: Result<i64, _> =
             with_obj_downcast(vm.frame_stack()[0].clone(), |str_inst: &Str| {

src/obj.rs

@@ -143,11 +143,10 @@ pub fn init_types() {
             __neg__ => BuiltinFunction::create("__neg__", BaseObj::not_implemented_un, 1),
             __not__ => BuiltinFunction::create("__not__", BaseObj::not, 1),
         },
-        Object { },
+        Obj { },
         Str {
             // Conversion methods
             to_str => BuiltinFunction::create("to_str", Str::to_str, 1),
-            to_repr => BuiltinFunction::create("to_repr", Str::to_repr, 1),
             to_int => BuiltinFunction::create("to_int", Str::to_int, 1),
             to_float => BuiltinFunction::create("to_float", Str::to_float, 1),
             len => BuiltinFunction::create("len", Str::len, 1),
@@ -364,7 +363,7 @@ impl Object for BaseObj {
 // Obj
 ////////////////////////////////////////////////////////////////////////////////
 
-#[derive(Debug, Trace, Finalize)]
+#[derive(Trace, Finalize)]
 pub struct Obj {
     base: BaseObj,
 }
@@ -381,7 +380,18 @@ impl Obj {
 
 impl Display for Obj {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        write!(fmt, "<Obj at {:x}>", (self as *const _ as usize))
+        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)
+        )
     }
 }
 
@@ -469,7 +479,7 @@ impl Object for Ty {
 // Str
 ////////////////////////////////////////////////////////////////////////////////
 
-#[derive(Debug, Trace, Finalize)]
+#[derive(Trace, Finalize)]
 pub struct Str {
     base: BaseObj,
     #[unsafe_ignore_trace]
@@ -497,6 +507,12 @@ impl Display for Str {
     }
 }
 
+impl Debug for Str {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        write!(fmt, "'{}'", self.str_value.as_str().escape_default())
+    }
+}
+
 impl Object for Str {
     fn is_truthy(&self) -> bool {
         !self.str_value.is_empty()
@@ -517,7 +533,7 @@ impl Object for Str {
 // Int
 ////////////////////////////////////////////////////////////////////////////////
 
-#[derive(Debug, Trace, Finalize)]
+#[derive(Trace, Finalize)]
 pub struct Int {
     base: BaseObj,
     int_value: i64,
@@ -544,6 +560,12 @@ impl Display for Int {
     }
 }
 
+impl Debug for Int {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        write!(fmt, "{}", self.int_value)
+    }
+}
+
 impl Object for Int {
     fn is_truthy(&self) -> bool {
         self.int_value != 0
@@ -566,7 +588,7 @@ impl Object for Int {
 // Float
 ////////////////////////////////////////////////////////////////////////////////
 
-#[derive(Debug, Trace, Finalize)]
+#[derive(Trace, Finalize)]
 pub struct Float {
     base: BaseObj,
     float_value: f64,
@@ -587,7 +609,7 @@ impl Float {
     }
 }
 
-impl Display for Float {
+impl Debug for Float {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         // we want to force the .0 if it's a whole number
         if self.float_value == self.float_value.floor() {
@@ -598,6 +620,12 @@ impl Display for Float {
     }
 }
 
+impl Display for Float {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        Debug::fmt(self, fmt)
+    }
+}
+
 impl Object for Float {
     fn is_truthy(&self) -> bool {
         self.float_value != 0.0
@@ -620,7 +648,7 @@ impl Object for Float {
 // Bool
 ////////////////////////////////////////////////////////////////////////////////
 
-#[derive(Debug, Trace, Finalize)]
+#[derive(Trace, Finalize)]
 pub struct Bool {
     base: BaseObj,
     bool_value: bool,
@@ -647,6 +675,12 @@ impl Display for Bool {
     }
 }
 
+impl Debug for Bool {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        write!(fmt, "{}", self.bool_value)
+    }
+}
+
 impl Object for Bool {
     fn is_truthy(&self) -> bool {
         self.bool_value
@@ -667,7 +701,7 @@ impl Object for Bool {
 // Nil
 ////////////////////////////////////////////////////////////////////////////////
 
-#[derive(Debug, Default, Trace, Finalize)]
+#[derive(Default, Trace, Finalize)]
 pub struct Nil {
     base: BaseObj,
 }
@@ -686,6 +720,12 @@ impl Display for Nil {
     }
 }
 
+impl Debug for Nil {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        write!(fmt, "nil")
+    }
+}
+
 impl Object for Nil {
     fn is_truthy(&self) -> bool {
         false

src/obj/function.rs

@@ -54,7 +54,7 @@ pub enum FunctionState {
 
 pub type BuiltinFunctionPtr = fn(vm: &mut Vm, function_state: FunctionState) -> FunctionResult;
 
-#[derive(Debug, Trace, Finalize)]
+#[derive(Trace, Finalize)]
 pub struct BuiltinFunction {
     base: BaseObj,
     #[unsafe_ignore_trace]
@@ -80,12 +80,18 @@ impl BuiltinFunction {
         arity: Argc,
     );
 
-    pub fn name(&self) -> &String {
+    pub fn name(&self) -> &Rc<String> {
         &self.name
     }
 }
 
 impl Display for BuiltinFunction {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        Debug::fmt(self, fmt)
+    }
+}
+
+impl Debug for BuiltinFunction {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         write!(
             fmt,
@@ -128,7 +134,7 @@ impl Object for BuiltinFunction {
 // UserFunction
 ////////////////////////////////////////////////////////////////////////////////
 
-#[derive(Debug, Clone, Trace, Finalize)]
+#[derive(Clone, Trace, Finalize)]
 pub struct UserFunction {
     base: BaseObj,
     #[unsafe_ignore_trace]
@@ -152,7 +158,7 @@ impl UserFunction {
 
     impl_create!(chunk: Chunk, arity: Argc);
 
-    pub fn name(&self) -> &String {
+    pub fn name(&self) -> &Rc<String> {
         &self.name
     }
 
@@ -170,6 +176,12 @@ impl UserFunction {
 }
 
 impl Display for UserFunction {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        Debug::fmt(self, fmt)
+    }
+}
+
+impl Debug for UserFunction {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
         write!(
             fmt,
@@ -222,16 +234,6 @@ pub struct Method {
     function: ObjP,
 }
 
-impl Debug for Method {
-    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        fmt.debug_struct("Method")
-            .field("base", &self.base)
-            .field("self_binding", &format!("{}", self.self_binding.borrow()))
-            .field("function", &self.function)
-            .finish()
-    }
-}
-
 impl Method {
     pub fn new(self_binding: ObjP, function: ObjP) -> Self {
         Self {
@@ -254,7 +256,37 @@ impl Method {
 
 impl Display for Method {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        write!(fmt, "{}", self.function.borrow())
+        Debug::fmt(self, fmt)
+    }
+}
+
+impl Debug for Method {
+    fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
+        let function_name: Rc<_> = if let Some(function) = self
+            .function
+            .borrow()
+            .as_any()
+            .downcast_ref::<BuiltinFunction>()
+        {
+            Rc::clone(&function.name())
+        } else if let Some(function) = self
+            .function
+            .borrow()
+            .as_any()
+            .downcast_ref::<UserFunction>()
+        {
+            function.name().clone()
+        } else {
+            unreachable!()
+        };
+        write!(
+            fmt,
+            "<Method {}.{}/{} at {}>",
+            self.self_binding.borrow().ty_name(),
+            function_name,
+            self.function.borrow().arity().unwrap(),
+            self as *const _ as usize
+        )
     }
 }