tamer: diagnose::panic: Require thunk or static ref for diagnostic data

Some investigation into the disassembly of TAMER's binaries showed that Rust
was not able to conditionalize `expect`-like expressions as I was hoping due
to eager evaluation language semantics in combination with the use of
`format!`.

This solves the problem for the diagnostic system be creating types that
prevent this situation from occurring statically, without the need for a
lint.

tamer/src/asg/graph.rs

@@ -443,7 +443,7 @@ impl Asg {
     ) -> ObjectIndex<O> {
         let obj_container =
             self.graph.node_weight_mut(index.into()).diagnostic_expect(
-                diagnostic_node_missing_desc(index),
+                || diagnostic_node_missing_desc(index),
                 "invalid ObjectIndex: data are missing from the ASG",
             );
 
@@ -527,11 +527,13 @@ impl Asg {
         ident: SPair,
     ) -> ObjectIndex<O> {
         self.get_ident_oi(ident).diagnostic_expect(
-            diagnostic_opaque_ident_desc(ident),
-            &format!(
-                "opaque identifier: {} has no object binding",
-                TtQuote::wrap(ident),
-            ),
+            || diagnostic_opaque_ident_desc(ident),
+            || {
+                format!(
+                    "opaque identifier: {} has no object binding",
+                    TtQuote::wrap(ident),
+                )
+            },
         )
     }
 
@@ -560,7 +562,7 @@ impl Asg {
     pub(super) fn expect_obj<O: ObjectKind>(&self, oi: ObjectIndex<O>) -> &O {
         let obj_container =
             self.graph.node_weight(oi.into()).diagnostic_expect(
-                diagnostic_node_missing_desc(oi),
+                || diagnostic_node_missing_desc(oi),
                 "invalid ObjectIndex: data are missing from the ASG",
             );
 
@@ -585,11 +587,13 @@ impl Asg {
     ///        graph).
     pub fn expect_ident_obj<O: ObjectKind>(&self, ident: SPair) -> &O {
         self.get_ident_obj(ident).diagnostic_expect(
-            diagnostic_opaque_ident_desc(ident),
-            &format!(
-                "opaque identifier: {} has no object binding",
-                TtQuote::wrap(ident),
-            ),
+            || diagnostic_opaque_ident_desc(ident),
+            || {
+                format!(
+                    "opaque identifier: {} has no object binding",
+                    TtQuote::wrap(ident),
+                )
+            },
         )
     }
 

tamer/src/asg/object.rs

@@ -461,10 +461,7 @@ impl ObjectContainer {
     pub fn get<O: ObjectKind>(&self) -> &O {
         let Self(container) = self;
 
-        container
-            .as_ref()
-            .diagnostic_unwrap(container_oops())
-            .into()
+        container.as_ref().diagnostic_unwrap(container_oops).into()
     }
 
     /// Attempt to modify the inner [`Object`],
@@ -483,7 +480,7 @@ impl ObjectContainer {
     ) -> Result<(), E> {
         let ObjectContainer(container) = self;
 
-        let obj = container.take().diagnostic_unwrap(container_oops()).into();
+        let obj = container.take().diagnostic_unwrap(container_oops).into();
 
         // NB: We must return the object to the container in all code paths!
         let result = f(obj)

tamer/src/convert.rs

@@ -39,10 +39,13 @@
 //! There may be better options;
 //!   these are most useful in writing tests.
 
-use crate::diagnose::panic::DiagnosticPanic;
+use crate::{
+    diagnose::{panic::DiagnosticPanic, NO_DESC},
+    f::ThunkOrStaticRef,
+};
 use std::{
     convert::{TryFrom, TryInto},
-    fmt::Debug,
+    fmt::{Debug, Display},
 };
 
 /// Safe type conversion that may panic under some circumstances.
@@ -69,8 +72,11 @@ where
     /// Panics
     /// ======
     /// Causes a panic on failure.
-    fn expect_from(value: T, msg: &str) -> Self {
-        Self::try_from(value).diagnostic_expect(vec![], msg)
+    fn expect_from<M: ThunkOrStaticRef<str>>(value: T, msg: M) -> Self
+    where
+        M::Output: Display,
+    {
+        Self::try_from(value).diagnostic_expect(|| NO_DESC, msg)
     }
 
     /// Attempt to convert and unwrap `value` using `T::try_from`.
@@ -79,7 +85,7 @@ where
     /// ======
     /// Causes a panic on failure.
     fn unwrap_from(value: T) -> Self {
-        Self::try_from(value).diagnostic_unwrap(vec![])
+        Self::try_from(value).diagnostic_unwrap(|| NO_DESC)
     }
 }
 
@@ -110,8 +116,11 @@ where
     /// Panics
     /// ======
     /// Causes a panic on failure.
-    fn expect_into(self, msg: &str) -> T {
-        self.try_into().diagnostic_expect(vec![], msg)
+    fn expect_into<M: ThunkOrStaticRef<str>>(self, msg: M) -> T
+    where
+        M::Output: Display,
+    {
+        self.try_into().diagnostic_expect(|| NO_DESC, msg)
     }
 
     /// Attempt to convert and unwrap a value using `self.try_into()`.
@@ -120,7 +129,7 @@ where
     /// ======
     /// Causes a panic on failure.
     fn unwrap_into(self) -> T {
-        self.try_into().diagnostic_unwrap(vec![])
+        self.try_into().diagnostic_unwrap(|| NO_DESC)
     }
 }
 

tamer/src/diagnose.rs

@@ -89,6 +89,14 @@ use std::{borrow::Cow, convert::Infallible, error::Error, fmt::Display};
 
 use crate::span::Span;
 
+/// No annotated description is applicable for the diagnostic message.
+///
+/// The system should strive to give the user as much relevant information
+///   as is useful to resolve the problem.
+/// Whether or not the absence of this description represents a deficiency
+///   depends on the error context.
+pub const NO_DESC: Vec<AnnotatedSpan> = vec![];
+
 /// Diagnostic report.
 ///
 /// This describes an error condition or other special event using a series

tamer/src/diagnose/panic.rs

@@ -57,6 +57,7 @@ use std::{
 // Macro exports are unintuitive.
 #[cfg(doc)]
 use crate::diagnostic_panic;
+use crate::f::ThunkOrStaticRef;
 
 /// The type of [`Reporter`](crate::diagnose::Reporter) used to produce
 ///   reports during panic operations.
@@ -211,7 +212,10 @@ pub trait DiagnosticPanic {
     /// Panics if the inner value is not available.
     /// For a custom message,
     ///   use [`DiagnosticPanic::diagnostic_expect`].
-    fn diagnostic_unwrap(self, desc: Vec<AnnotatedSpan>) -> Self::Inner;
+    fn diagnostic_unwrap<'a>(
+        self,
+        fdesc: impl FnOnce() -> Vec<AnnotatedSpan<'a>>,
+    ) -> Self::Inner;
 
     /// Attempt to return the inner value,
     ///   consuming `self`.
@@ -220,37 +224,50 @@ pub trait DiagnosticPanic {
     ///   producing diagnostic information in the event of a failure.
     /// See [`diagnostic_panic!`] for more information.
     ///
+    /// Unlike Rust's `expect`,
+    ///   this uses [`ThunkOrStaticRef`] to force deferring of any
+    ///   message-related computation to avoid situations like those in
+    ///     Clippy's `expect_fun_call` lint.
+    ///
     /// # Panics
     /// Panics if the inner value is not available with a custom `msg`.
-    fn diagnostic_expect(
+    fn diagnostic_expect<'a, M: ThunkOrStaticRef<str>>(
         self,
-        desc: Vec<AnnotatedSpan>,
-        msg: &str,
-    ) -> Self::Inner;
+        fdesc: impl FnOnce() -> Vec<AnnotatedSpan<'a>>,
+        msg: M,
+    ) -> Self::Inner
+    where
+        M::Output: Display;
 }
 
 impl<T> DiagnosticPanic for Option<T> {
     type Inner = T;
 
-    fn diagnostic_unwrap(self, desc: Vec<AnnotatedSpan>) -> Self::Inner {
+    fn diagnostic_unwrap<'a>(
+        self,
+        fdesc: impl FnOnce() -> Vec<AnnotatedSpan<'a>>,
+    ) -> Self::Inner {
         match self {
             Some(val) => val,
             // Same message as `Option::unwrap`
             None => diagnostic_panic!(
-                desc,
+                fdesc(),
                 "called `Option::unwrap()` on a `None` value"
             ),
         }
     }
 
-    fn diagnostic_expect(
+    fn diagnostic_expect<'a, M: ThunkOrStaticRef<str>>(
         self,
-        desc: Vec<AnnotatedSpan>,
-        msg: &str,
-    ) -> Self::Inner {
+        fdesc: impl FnOnce() -> Vec<AnnotatedSpan<'a>>,
+        msg: M,
+    ) -> Self::Inner
+    where
+        M::Output: Display,
+    {
         match self {
             Some(val) => val,
-            None => diagnostic_panic!(desc, "{}", msg),
+            None => diagnostic_panic!(fdesc(), "{}", msg.call()),
         }
     }
 }
@@ -261,25 +278,31 @@ where
 {
     type Inner = T;
 
-    fn diagnostic_unwrap(self, desc: Vec<AnnotatedSpan>) -> Self::Inner {
+    fn diagnostic_unwrap<'a>(
+        self,
+        fdesc: impl FnOnce() -> Vec<AnnotatedSpan<'a>>,
+    ) -> Self::Inner {
         match self {
             Ok(val) => val,
             // Same message as `Result::unwrap`
             Err(e) => diagnostic_panic!(
-                desc,
+                fdesc(),
                 "called `Result::unwrap()` on an `Err` value: {e:?}"
             ),
         }
     }
 
-    fn diagnostic_expect(
+    fn diagnostic_expect<'a, M: ThunkOrStaticRef<str>>(
         self,
-        desc: Vec<AnnotatedSpan>,
-        msg: &str,
-    ) -> Self::Inner {
+        fdesc: impl FnOnce() -> Vec<AnnotatedSpan<'a>>,
+        msg: M,
+    ) -> Self::Inner
+    where
+        M::Output: Display,
+    {
         match self {
             Ok(val) => val,
-            Err(e) => diagnostic_panic!(desc, "{}: {e:?}", msg),
+            Err(e) => diagnostic_panic!(fdesc(), "{}: {e:?}", msg.call()),
         }
     }
 }
@@ -294,7 +317,7 @@ pub trait DiagnosticOptionPanic<T> {
     fn diagnostic_expect_none<'a>(
         self,
         fdesc: impl FnOnce(T) -> Vec<AnnotatedSpan<'a>>,
-        msg: &str,
+        msg: &'static str,
     );
 }
 
@@ -302,7 +325,7 @@ impl<T> DiagnosticOptionPanic<T> for Option<T> {
     fn diagnostic_expect_none<'a>(
         self,
         fdesc: impl FnOnce(T) -> Vec<AnnotatedSpan<'a>>,
-        msg: &str,
+        msg: &'static str,
     ) {
         match self {
             None => (),

tamer/src/f.rs

@@ -125,3 +125,50 @@ impl<T, U> Functor<T, U> for Option<T> {
         Option::map(self, f)
     }
 }
+
+/// A nullary [`Fn`] delaying some computation.
+///
+/// For the history and usage of this term in computing,
+///   see <https://en.wikipedia.org/wiki/Thunk>.
+pub trait Thunk<T> = Fn() -> T;
+
+/// Data represented either as a reference with a `'static` lifetime
+///   (representing a computation already having been performed),
+///   or a [`Thunk`] that will produce similar data when invoked.
+///
+/// The purpose of this trait is to force an API to defer potentially
+///   expensive computations in situations where it may be too easy to
+///   accidentally do too much work due to Rust's eager argument evaluation
+///   strategy
+///     (e.g. see Clippy's `expect_fun_call` lint).
+///
+/// This is sort of like a static [`std::borrow::Cow`],
+///   in the sense that it can hold a reference or owned value;
+///     a thunk can return a value of any type or lifetime
+///       (including owned),
+///       whereas non-thunks require static references.
+/// _The burden is on the trait implementation to enforce these
+///   constraints._
+pub trait ThunkOrStaticRef<T: ?Sized> {
+    type Output: AsRef<T>;
+
+    /// Force the [`Thunk`] or return the static reference,
+    ///   depending on the type of [`Self`].
+    fn call(self) -> Self::Output;
+}
+
+impl<T: ?Sized, R: AsRef<T>, F: Fn() -> R> ThunkOrStaticRef<T> for F {
+    type Output = R;
+
+    fn call(self) -> R {
+        self()
+    }
+}
+
+impl ThunkOrStaticRef<str> for &'static str {
+    type Output = &'static str;
+
+    fn call(self) -> &'static str {
+        self
+    }
+}

tamer/src/nir/interp.rs

@@ -467,9 +467,11 @@ impl ParseState for InterpState {
 fn next_delim(s: &str, span: Span, offset: usize) -> Option<(usize, char)> {
     s.get(offset..)
         .diagnostic_expect(
-            span.internal_error("while parsing this specification")
-                .into(),
-            &format!("specification byte offset {offset} is out of bounds"),
+            || {
+                span.internal_error("while parsing this specification")
+                    .into()
+            },
+            || format!("specification byte offset {offset} is out of bounds"),
         )
         .char_indices()
         .find(|(_, ch)| matches!(*ch, '{' | '}'))