From ac6dad9dbd1044cf9bc014a41191bcad540e9391 Mon Sep 17 00:00:00 2001 From: Alek Ratzloff Date: Fri, 27 Sep 2024 08:10:09 -0700 Subject: [PATCH] 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 "". 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 --- src/builtins.rs | 10 +------- src/obj.rs | 60 +++++++++++++++++++++++++++++++++++-------- src/obj/function.rs | 62 ++++++++++++++++++++++++++++++++++----------- 3 files changed, 98 insertions(+), 34 deletions(-) diff --git a/src/builtins.rs b/src/builtins.rs index eaa4f10..8de9fed 100644 --- a/src/builtins.rs +++ b/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(str_value).into() + Str::create(format!("{:?}", vm.frame_stack()[0].borrow())).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 = with_obj_downcast(vm.frame_stack()[0].clone(), |str_inst: &Str| { diff --git a/src/obj.rs b/src/obj.rs index c1cad14..e5484a5 100644 --- a/src/obj.rs +++ b/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, "", (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 diff --git a/src/obj/function.rs b/src/obj/function.rs index 0218f8d..85cf1e1 100644 --- a/src/obj/function.rs +++ b/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 { &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 { &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::() + { + Rc::clone(&function.name()) + } else if let Some(function) = self + .function + .borrow() + .as_any() + .downcast_ref::() + { + function.name().clone() + } else { + unreachable!() + }; + write!( + fmt, + "", + self.self_binding.borrow().ty_name(), + function_name, + self.function.borrow().arity().unwrap(), + self as *const _ as usize + ) } }