From f26a8e85a549dcae51bf8a08e8edddd88db6e746 Mon Sep 17 00:00:00 2001
From: Sahas Subramanian <sahas.subramanian@proton.me>
Date: Sat, 11 Nov 2023 11:36:24 +0100
Subject: [PATCH 1/5] Remove confusing `==` oper between `TulispObject` and
 `TulispValue`

This is replaced by an internal `eq_value` method, which can be used
where it would help performance.
---
 src/builtin/functions/arithmetic_operations.rs | 4 +++-
 src/lists.rs                                   | 4 ++--
 src/object.rs                                  | 6 ------
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/builtin/functions/arithmetic_operations.rs b/src/builtin/functions/arithmetic_operations.rs
index 13a8998..e31120c 100644
--- a/src/builtin/functions/arithmetic_operations.rs
+++ b/src/builtin/functions/arithmetic_operations.rs
@@ -43,7 +43,9 @@ pub(crate) fn add(ctx: &mut TulispContext) {
         iter.next();
         while let Some(ele) = iter.eval_next(ctx) {
             let ele = ele?;
-            if ele == TulispValue::from(0) || ele == TulispValue::from(0.0) {
+            if *ele.inner_ref() == TulispValue::from(0)
+                || *ele.inner_ref() == TulispValue::from(0.0)
+            {
                 return Err(Error::new(
                     ErrorKind::Undefined,
                     "Division by zero".to_string(),
diff --git a/src/lists.rs b/src/lists.rs
index c3b6c27..a269fe3 100644
--- a/src/lists.rs
+++ b/src/lists.rs
@@ -189,7 +189,7 @@ mod tests {
             (b.clone(), 30.into()),
             (c.clone(), 40.into()),
         ]);
-        assert_eq!(alist_get(&mut ctx, &b, &list, None, None, None)?, 30.into());
+        assert!(alist_get(&mut ctx, &b, &list, None, None, None)?.equal(&30.into()));
         assert_eq!(
             alist_get(&mut ctx, &d, &list, None, None, None)?.null(),
             true
@@ -209,7 +209,7 @@ mod tests {
             (b.clone(), 30.into()),
             (c.clone(), 40.into()),
         ]);
-        assert_eq!(plist_get(list.clone(), &b)?, 30.into());
+        assert!(plist_get(list.clone(), &b)?.equal(&30.into()));
         assert_eq!(plist_get(list, &d)?.null(), true);
         Ok(())
     }
diff --git a/src/object.rs b/src/object.rs
index 88ca762..bdd31b4 100644
--- a/src/object.rs
+++ b/src/object.rs
@@ -39,12 +39,6 @@ impl Default for TulispObject {
     }
 }
 
-impl std::cmp::PartialEq<TulispValue> for TulispObject {
-    fn eq(&self, other: &TulispValue) -> bool {
-        *self.rc.borrow() == *other
-    }
-}
-
 impl std::fmt::Display for TulispObject {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.write_fmt(format_args!("{}", self.rc.borrow()))

From 3b7debddfc6954d846e1ad8a555094f9e23317b9 Mon Sep 17 00:00:00 2001
From: Sahas Subramanian <sahas.subramanian@proton.me>
Date: Sat, 11 Nov 2023 11:44:04 +0100
Subject: [PATCH 2/5] Hide `TulispValue` re-export from documentation

---
 src/lib.rs | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/lib.rs b/src/lib.rs
index a7040ee..456ee63 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -79,6 +79,7 @@ pub use error::{Error, ErrorKind};
 pub mod lists;
 
 mod value;
+#[doc(hidden)]
 pub use value::TulispValue;
 
 mod object;

From d2fa672af2ac3768b7521887e1cfb4155e5e1dc8 Mon Sep 17 00:00:00 2001
From: Sahas Subramanian <sahas.subramanian@proton.me>
Date: Sat, 11 Nov 2023 11:44:35 +0100
Subject: [PATCH 3/5] Remove redundant link targets in documentation

---
 src/lib.rs | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index 456ee63..f9cd4ca 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -48,11 +48,11 @@ A list of currently available builtin functions can be found [here](builtin).
 /*!
 ## Next steps
 
-1. Values in _Tulisp_ are represented in rust as [`TulispObject`](TulispObject)s.
-   That struct implements methods for performing operations on Tulisp values.
+1. Values in _Tulisp_ are represented in rust as [`TulispObject`]s.  That struct
+   implements methods for performing operations on Tulisp values.
 
-1. [`TulispContext`](TulispContext) tracks the state of the interpreter and
-   provides methods for executing _Tulisp_ programs.
+1. [`TulispContext`] tracks the state of the interpreter and provides methods
+   for executing _Tulisp_ programs.
 
 1. [`#[tulisp_fn]`](tulisp_fn) and [`#[tulisp_fn_no_eval]`](tulisp_fn_no_eval)
    are flexible attribute macros for adding many different kinds of functions to

From c3b1b35641a66b6b982a508750b13c8a79abecee Mon Sep 17 00:00:00 2001
From: Sahas Subramanian <sahas.subramanian@proton.me>
Date: Sat, 11 Nov 2023 12:47:00 +0100
Subject: [PATCH 4/5] Use string comparison for tests that use non-`intern`d
 symbols

---
 tests/tests.rs | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/tests/tests.rs b/tests/tests.rs
index 9051533..aa9f7ca 100644
--- a/tests/tests.rs
+++ b/tests/tests.rs
@@ -18,14 +18,33 @@ macro_rules! tulisp_assert {
             expected
         );
     };
+
+    (@impl $ctx: expr, program:$input:expr, result_str:$result:expr $(,)?) => {
+        let output = $ctx.eval_string($input).map_err(|err| {
+            println!("{}:{}: execution failed: {}", file!(), line!(),err.format(&$ctx));
+            err
+        })?;
+        let expected = $ctx.eval_string($result)?;
+        assert_eq!(output.to_string(), expected.to_string(),
+            "\n{}:{}: program: {}\n  output: {},\n  expected: {}\n",
+            file!(),
+            line!(),
+            $input,
+            output,
+            expected
+        );
+    };
+
     (@impl $ctx: expr, program:$input:expr, error:$desc:expr $(,)?) => {
         let output = $ctx.eval_string($input);
         assert!(output.is_err());
         assert_eq!(output.unwrap_err().format(&$ctx), $desc);
     };
+
     (ctx: $ctx: expr, program: $($tail:tt)+) => {
         tulisp_assert!(@impl $ctx, program: $($tail)+)
     };
+
     (program: $($tail:tt)+) => {
         let mut ctx = TulispContext::new();
         tulisp_assert!(ctx: ctx, program: $($tail)+)
@@ -159,7 +178,7 @@ fn test_conditionals() -> Result<(), Error> {
 
     tulisp_assert! {
         program: "(macroexpand '(if-let (c) (+ c 10) 2))",
-        result: "'(let ((s (and t c))) (if s (+ c 10) 2))",
+        result_str: "'(let ((s (and t c))) (if s (+ c 10) 2))",
     }
     tulisp_assert! {
         program: "(defun test (&optional c) (if-let (c) (+ c 10) 2)) (list (test) (test 2))",
@@ -184,15 +203,15 @@ fn test_conditionals() -> Result<(), Error> {
 
     tulisp_assert! {
         program: "(macroexpand '(when-let (c) (+ c 10)))",
-        result: "'(let ((s (and t c))) (if s (+ c 10) nil))",
+        result_str: "'(let ((s (and t c))) (if s (+ c 10) nil))",
     }
     tulisp_assert! {
         program: "(macroexpand '(when-let (c) (+ c 10) 2))",
-        result: "'(let ((s (and t c))) (if s (progn (+ c 10) 2) nil))",
+        result_str: "'(let ((s (and t c))) (if s (progn (+ c 10) 2) nil))",
     }
     tulisp_assert! {
         program: "(macroexpand '(when-let ((q c) d (w 10)) 2 (+ c d w)))",
-        result: r#"'
+        result_str: r#"'
         (let ((q (and t c)))
           (let ((d (and q d)))
             (let ((w (and d 10)))
@@ -204,7 +223,7 @@ fn test_conditionals() -> Result<(), Error> {
 
     tulisp_assert! {
         program: "(macroexpand '(while-let (c) (+ c 10)))",
-        result: "'(while (let ((s (and t c))) (if s (progn (+ c 10) t) nil)))",
+        result_str: "'(while (let ((s (and t c))) (if s (progn (+ c 10) t) nil)))",
     }
     tulisp_assert! {
         program: "(let ((ll '(1 2 3)) (vv 0)) (while-let (x (car ll))  (setq ll (cdr ll)) (setq vv (+ vv x))) vv)",
@@ -815,7 +834,7 @@ fn test_threading_macros() -> Result<(), Error> {
                            (if-let ((a) (b))
                                (print a))))
             "##,
-        result: r##"
+        result_str: r##"
         '(let ((s (and t a)))
           (let ((s (and s b)))
             (if s

From 58f8341c063dfacbc27162ffbbcfd2c98deabc47 Mon Sep 17 00:00:00 2001
From: Sahas Subramanian <sahas.subramanian@proton.me>
Date: Sat, 11 Nov 2023 13:02:29 +0100
Subject: [PATCH 5/5] Improve symbol comparison with `equal`

They are no longer compared by name-string, but whether they are the
same object or not, just like with `eq`.
---
 src/object.rs | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/object.rs b/src/object.rs
index bdd31b4..c7ed678 100644
--- a/src/object.rs
+++ b/src/object.rs
@@ -102,7 +102,11 @@ impl TulispObject {
     /// Read more about Emacs equality predicates
     /// [here](https://www.gnu.org/software/emacs/manual/html_node/elisp/Equality-Predicates.html).
     pub fn equal(&self, other: &TulispObject) -> bool {
-        *self.rc.borrow() == *other.rc.borrow()
+        if self.symbolp() {
+            self.eq_ptr(other)
+        } else {
+            self.eq_val(other)
+        }
     }
 
     /// Returns true if `self` and `other` are the same object.
@@ -110,7 +114,7 @@ impl TulispObject {
     /// Read more about Emacs equality predicates
     /// [here](https://www.gnu.org/software/emacs/manual/html_node/elisp/Equality-Predicates.html).
     pub fn eq(&self, other: &TulispObject) -> bool {
-        Rc::ptr_eq(&self.rc, &other.rc)
+        self.eq_ptr(other)
     }
 
     /// Returns an iterator over the values inside `self`.
@@ -322,6 +326,14 @@ impl TulispObject {
         }
     }
 
+    pub(crate) fn eq_ptr(&self, other: &TulispObject) -> bool {
+        Rc::ptr_eq(&self.rc, &other.rc)
+    }
+
+    pub(crate) fn eq_val(&self, other: &TulispObject) -> bool {
+        self.inner_ref().eq(&other.inner_ref())
+    }
+
     pub(crate) fn clone_without_span(&self) -> Self {
         Self {
             rc: Rc::clone(&self.rc),