From 7eb15f8123fe3de531ee3dde66c5aa103dae4b7d Mon Sep 17 00:00:00 2001
From: Florian Klink <flokli@flokli.de>
Date: Wed, 7 May 2025 15:19:31 +0300
Subject: [PATCH] refactor(eval): make CatchableErrorKind::Throw hold a
 NixString

The messages we can throw are not necessarily UTF-8 strings. The
to_string() in there did store the result of the Display impl, which is
a quoted string.

Change-Id: I65a77ccc7f2d62ff06a2a9458cdb7e7292f132b0
Reviewed-on: https://cl.snix.dev/c/snix/+/30489
Tested-by: besadii
Autosubmit: Florian Klink <flokli@flokli.de>
Reviewed-by: Bence Nemes <nemes.bence1@gmail.com>
---
 snix/eval/src/builtins/mod.rs   |  7 +++----
 snix/eval/src/errors.rs         |  2 +-
 snix/eval/src/tests/one_offs.rs | 14 +++-----------
 3 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/snix/eval/src/builtins/mod.rs b/snix/eval/src/builtins/mod.rs
index 9efb2820f..5f271bee6 100644
--- a/snix/eval/src/builtins/mod.rs
+++ b/snix/eval/src/builtins/mod.rs
@@ -1405,11 +1405,10 @@ mod pure_builtins {
         if message.is_catchable() {
             return Ok(message);
         }
-        // TODO(sterni): coerces to string
+
+        // TODO(sterni): coerce to string
         // We do not care about the context here explicitly.
-        Ok(Value::from(CatchableErrorKind::Throw(
-            message.to_contextful_str()?.to_string().into(),
-        )))
+        Ok(Value::from(CatchableErrorKind::Throw(message.to_str()?)))
     }
 
     #[builtin("toString")]
diff --git a/snix/eval/src/errors.rs b/snix/eval/src/errors.rs
index 098ae3211..2e09a0881 100644
--- a/snix/eval/src/errors.rs
+++ b/snix/eval/src/errors.rs
@@ -39,7 +39,7 @@ use crate::{SourceCode, Value};
 #[derive(thiserror::Error, Clone, Debug)]
 pub enum CatchableErrorKind {
     #[error("error thrown: {0}")]
-    Throw(Box<str>),
+    Throw(NixString),
 
     #[error("assertion failed")]
     AssertionFailed,
diff --git a/snix/eval/src/tests/one_offs.rs b/snix/eval/src/tests/one_offs.rs
index 7550f4e7a..28ab999e9 100644
--- a/snix/eval/src/tests/one_offs.rs
+++ b/snix/eval/src/tests/one_offs.rs
@@ -18,11 +18,7 @@ fn test_source_builtin() {
     );
 
     let value = result.value.unwrap();
-    assert!(
-        matches!(value, Value::Integer(42)),
-        "expected the integer 42, but got {}",
-        value,
-    );
+    assert_matches!(value, Value::Integer(i) if i == 42);
 }
 
 #[test]
@@ -32,11 +28,7 @@ fn skip_broken_bytecode() {
         .evaluate(/* code = */ "x", None);
 
     assert_eq!(result.errors.len(), 1);
-
-    assert!(matches!(
-        result.errors[0].kind,
-        ErrorKind::UnknownStaticVariable
-    ));
+    assert_matches!(result.errors[0].kind, ErrorKind::UnknownStaticVariable);
 }
 
 /// Checks that deep forcing happens in lexicographic key order
@@ -58,6 +50,6 @@ fn key_order_deep_force() {
 
     assert_matches!(
         &result.errors[0].kind,
-        ErrorKind::CatchableError(CatchableErrorKind::Throw(s)) if s.as_ref() == "\"aaa\""
+        ErrorKind::CatchableError(CatchableErrorKind::Throw(s)) if s == &NixString::from("aaa")
     );
 }