feat(tvix/eval): add error contexts to annotate error kinds r/5706

This makes it possible for users to add additional context to an
error, which will then be rendered as an additional secondary span in
the formatted error output.

We should strive to do this basically anywhere errors are raised that
can occur multiple times, *especially* during type casts. This was
triggered by me debugging a type cast error attached to a fairly
large-ish span (a builtin invocation).

Change-Id: I51be41fabee00cf04de973935daf34fe6424e76f
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7849
Tested-by: BuildkiteCI
Reviewed-by: flokli <flokli@flokli.de>

5 files changed, 99 insertions, 34 deletions

diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs
index 92084b031c6c..4a50a3be4935 100644
--- a/tvix/eval/src/compiler/mod.rs
+++ b/tvix/eval/src/compiler/mod.rs
@@ -157,12 +157,14 @@ impl<'observer> Compiler<'observer> {
         let mut root_dir = match location {
             Some(dir) if cfg!(target_arch = "wasm32") || dir.is_absolute() => Ok(dir),
             _ => {
-                let current_dir = std::env::current_dir().map_err(|e| Error {
-                    kind: ErrorKind::RelativePathResolution(format!(
-                        "could not determine current directory: {}",
-                        e
-                    )),
-                    span: file.span,
+                let current_dir = std::env::current_dir().map_err(|e| {
+                    Error::new(
+                        ErrorKind::RelativePathResolution(format!(
+                            "could not determine current directory: {}",
+                            e
+                        )),
+                        file.span,
+                    )
                 })?;
                 if let Some(dir) = location {
                     Ok(current_dir.join(dir))
@@ -1220,7 +1222,7 @@ impl Compiler<'_> {
 
     fn emit_error<N: ToSpan>(&mut self, node: &N, kind: ErrorKind) {
         let span = self.span_for(node);
-        self.errors.push(Error { kind, span })
+        self.errors.push(Error::new(kind, span))
     }
 }
 
diff --git a/tvix/eval/src/errors.rs b/tvix/eval/src/errors.rs
index ed161f4155f5..ccc2d8976440 100644
--- a/tvix/eval/src/errors.rs
+++ b/tvix/eval/src/errors.rs
@@ -158,6 +158,12 @@ pub enum ErrorKind {
     /// not actually implemented yet, and without which eval can not
     /// proceed.
     NotImplemented(&'static str),
+
+    /// Internal variant which should disappear during error construction.
+    WithContext {
+        context: String,
+        underlying: Box<ErrorKind>,
+    },
 }
 
 impl error::Error for Error {
@@ -242,8 +248,29 @@ impl From<serde_json::Error> for ErrorKind {
 
 #[derive(Clone, Debug)]
 pub struct Error {
-    pub kind: ErrorKind,
-    pub span: Span,
+    kind: ErrorKind,
+    span: Span,
+    contexts: Vec<String>,
+}
+
+impl Error {
+    pub fn new(mut kind: ErrorKind, span: Span) -> Self {
+        let mut contexts = vec![];
+        while let ErrorKind::WithContext {
+            context,
+            underlying,
+        } = kind
+        {
+            kind = *underlying;
+            contexts.push(context);
+        }
+
+        Error {
+            kind,
+            span,
+            contexts,
+        }
+    }
 }
 
 impl Display for ErrorKind {
@@ -434,6 +461,10 @@ to a missing value in the attribute set(s) included via `with`."#,
             ErrorKind::NotImplemented(feature) => {
                 write!(f, "feature not yet implemented in Tvix: {}", feature)
             }
+
+            ErrorKind::WithContext { .. } => {
+                panic!("internal ErrorKind::WithContext variant leaked")
+            }
         }
     }
 }
@@ -721,7 +752,8 @@ impl Error {
             | ErrorKind::Xml(_)
             | ErrorKind::TvixError(_)
             | ErrorKind::TvixBug { .. }
-            | ErrorKind::NotImplemented(_) => return None,
+            | ErrorKind::NotImplemented(_)
+            | ErrorKind::WithContext { .. } => return None,
         };
 
         Some(label.into())
@@ -782,11 +814,15 @@ impl Error {
             //
             // The error code for thunk forces is E017.
             ErrorKind::ThunkForce(ref err) => err.code(),
+
+            ErrorKind::WithContext { .. } => {
+                panic!("internal ErrorKind::WithContext variant leaked")
+            }
         }
     }
 
     fn spans(&self, source: &SourceCode) -> Vec<SpanLabel> {
-        match &self.kind {
+        let mut spans = match &self.kind {
             ErrorKind::ImportParseError { errors, file, .. } => {
                 spans_for_parse_errors(file, errors)
             }
@@ -840,7 +876,17 @@ impl Error {
                     style: SpanStyle::Primary,
                 }]
             }
+        };
+
+        for ctx in &self.contexts {
+            spans.push(SpanLabel {
+                label: Some(format!("while {}", ctx)),
+                span: self.span,
+                style: SpanStyle::Secondary,
+            });
         }
+
+        spans
     }
 
     /// Create the primary diagnostic for a given error.
@@ -869,3 +915,33 @@ impl Error {
         }
     }
 }
+
+// Convenience methods to add context on other types.
+pub trait AddContext {
+    /// Add context to the error-carrying type.
+    fn context<S: Into<String>>(self, ctx: S) -> Self;
+}
+
+impl AddContext for ErrorKind {
+    fn context<S: Into<String>>(self, ctx: S) -> Self {
+        ErrorKind::WithContext {
+            context: ctx.into(),
+            underlying: Box::new(self),
+        }
+    }
+}
+
+impl<T> AddContext for Result<T, ErrorKind> {
+    fn context<S: Into<String>>(self, ctx: S) -> Self {
+        self.map_err(|kind| kind.context(ctx))
+    }
+}
+
+impl<T> AddContext for Result<T, Error> {
+    fn context<S: Into<String>>(self, ctx: S) -> Self {
+        self.map_err(|err| Error {
+            kind: err.kind.context(ctx),
+            ..err
+        })
+    }
+}
diff --git a/tvix/eval/src/lib.rs b/tvix/eval/src/lib.rs
index 484d3e72e310..a759e0c0ab2c 100644
--- a/tvix/eval/src/lib.rs
+++ b/tvix/eval/src/lib.rs
@@ -48,7 +48,7 @@ use crate::vm::run_lambda;
 
 // Re-export the public interface used by other crates.
 pub use crate::compiler::{compile, prepare_globals, CompilationOutput};
-pub use crate::errors::{Error, ErrorKind, EvalResult};
+pub use crate::errors::{AddContext, Error, ErrorKind, EvalResult};
 pub use crate::io::{DummyIO, EvalIO, FileType};
 pub use crate::pretty_ast::pretty_print_expr;
 pub use crate::source::SourceCode;
@@ -278,10 +278,10 @@ fn parse_compile_internal(
     let parse_errors = parsed.errors();
 
     if !parse_errors.is_empty() {
-        result.errors.push(Error {
-            kind: ErrorKind::ParseErrors(parse_errors.to_vec()),
-            span: file.span,
-        });
+        result.errors.push(Error::new(
+            ErrorKind::ParseErrors(parse_errors.to_vec()),
+            file.span,
+        ));
         return None;
     }
 
diff --git a/tvix/eval/src/value/thunk.rs b/tvix/eval/src/value/thunk.rs
index 23c60aa37815..2d48550dad97 100644
--- a/tvix/eval/src/value/thunk.rs
+++ b/tvix/eval/src/value/thunk.rs
@@ -197,10 +197,8 @@ impl Thunk {
                                     self_clone.0.replace(ThunkRepr::Evaluated(vm.pop()));
                                 assert!(matches!(should_be_blackhole, ThunkRepr::Blackhole));
                                 vm.push(Value::Thunk(self_clone));
-                                Self::force_trampoline(vm).map_err(|kind| Error {
-                                    kind,
-                                    span: light_span.span(),
-                                })
+                                Self::force_trampoline(vm)
+                                    .map_err(|kind| Error::new(kind, light_span.span()))
                             })),
                         });
                     }
diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs
index 3dece0641281..60006ea76f9c 100644
--- a/tvix/eval/src/vm.rs
+++ b/tvix/eval/src/vm.rs
@@ -134,12 +134,7 @@ macro_rules! fallible {
     ( $self:ident, $body:expr) => {
         match $body {
             Ok(result) => result,
-            Err(kind) => {
-                return Err(Error {
-                    kind,
-                    span: $self.current_span(),
-                })
-            }
+            Err(kind) => return Err(Error::new(kind, $self.current_span())),
         }
     };
 }
@@ -281,10 +276,7 @@ impl<'o> VM<'o> {
     /// Construct an error from the given ErrorKind and the source
     /// span of the current instruction.
     pub fn error(&self, kind: ErrorKind) -> Error {
-        Error {
-            kind,
-            span: self.current_span(),
-        }
+        Error::new(kind, self.current_span())
     }
 
     /// Push an already constructed warning.
@@ -1197,10 +1189,7 @@ pub fn run_lambda(
 
     value
         .deep_force(&mut vm, &mut Default::default())
-        .map_err(|kind| Error {
-            kind,
-            span: root_span,
-        })?;
+        .map_err(|kind| Error::new(kind, root_span))?;
 
     Ok(RuntimeResult {
         value,