feat(tvix/eval): report all known spans on infinite recursion r/6026

This reports the span

1. of the code within a thunk,
2. of the place where the thunk was instantiated,
3. of the place where the thunk was first forced,
4. of the place where the thunk was forced again,

when yielding an infinite recursion error, which hopefully makes it
easier to debug them.

The spans are tracked in the ThunkRepr::Blackhole variant when putting
a thunk under evaluation.

Note that we currently have some loss of span precision in the VM loop
when switching between frame types, so spans 3/4 are currently a bit
wonky. Working on it.

Change-Id: Icbd2a9df903d00e8c2545b3fc46dcd2a9e3e3e55
Reviewed-on: https://cl.tvl.fyi/c/depot/+/8270
Reviewed-by: flokli <flokli@flokli.de>
Tested-by: BuildkiteCI
Autosubmit: tazjin <tazjin@tvl.su>

3 files changed, 80 insertions, 16 deletions

diff --git a/tvix/eval/src/chunk.rs b/tvix/eval/src/chunk.rs
index 04b58bde20..86d78cbe63 100644
--- a/tvix/eval/src/chunk.rs
+++ b/tvix/eval/src/chunk.rs
@@ -65,6 +65,11 @@ impl Chunk {
         CodeIdx(idx)
     }
 
+    /// Get the first span of a chunk, no questions asked.
+    pub fn first_span(&self) -> codemap::Span {
+        self.spans[0].span
+    }
+
     /// Pop the last operation from the chunk and clean up its tracked
     /// span. Used when the compiler backtracks.
     pub fn pop_op(&mut self) {
diff --git a/tvix/eval/src/errors.rs b/tvix/eval/src/errors.rs
index 76a3da3ff5..2fbb6496ce 100644
--- a/tvix/eval/src/errors.rs
+++ b/tvix/eval/src/errors.rs
@@ -79,6 +79,8 @@ pub enum ErrorKind {
     /// Infinite recursion encountered while forcing thunks.
     InfiniteRecursion {
         first_force: Span,
+        suspended_at: Option<Span>,
+        content_span: Option<Span>,
     },
 
     ParseErrors(Vec<rnix::parser::ParseError>),
@@ -871,19 +873,42 @@ impl Error {
                 ]
             }
 
-            ErrorKind::InfiniteRecursion { first_force } => {
-                vec![
-                    SpanLabel {
-                        label: Some("first requested here".into()),
-                        span: *first_force,
+            ErrorKind::InfiniteRecursion {
+                first_force,
+                suspended_at,
+                content_span,
+            } => {
+                let mut spans = vec![];
+
+                if let Some(content_span) = content_span {
+                    spans.push(SpanLabel {
+                        label: Some("this lazily-evaluated code".into()),
+                        span: *content_span,
                         style: SpanStyle::Secondary,
-                    },
-                    SpanLabel {
-                        label: Some("requested again here".into()),
-                        span: self.span,
-                        style: SpanStyle::Primary,
-                    },
-                ]
+                    })
+                }
+
+                if let Some(suspended_at) = suspended_at {
+                    spans.push(SpanLabel {
+                        label: Some("which was instantiated here".into()),
+                        span: *suspended_at,
+                        style: SpanStyle::Secondary,
+                    })
+                }
+
+                spans.push(SpanLabel {
+                    label: Some("was first requested to be evaluated here".into()),
+                    span: *first_force,
+                    style: SpanStyle::Secondary,
+                });
+
+                spans.push(SpanLabel {
+                    label: Some("but then requested again here during its own evaluation".into()),
+                    span: self.span,
+                    style: SpanStyle::Primary,
+                });
+
+                spans
             }
 
             // All other errors pretty much have the same shape.
diff --git a/tvix/eval/src/value/thunk.rs b/tvix/eval/src/value/thunk.rs
index 0290e73ebb..7cdf3054f7 100644
--- a/tvix/eval/src/value/thunk.rs
+++ b/tvix/eval/src/value/thunk.rs
@@ -35,6 +35,7 @@ use crate::{
 };
 
 use super::{Lambda, TotalDisplay};
+use codemap::Span;
 
 /// Internal representation of a suspended native thunk.
 struct SuspendedNative(Box<dyn Fn() -> Result<Value, ErrorKind>>);
@@ -65,7 +66,17 @@ enum ThunkRepr {
 
     /// Thunk currently under-evaluation; encountering a blackhole
     /// value means that infinite recursion has occured.
-    Blackhole { forced_at: LightSpan },
+    Blackhole {
+        /// Span at which the thunk was first forced.
+        forced_at: LightSpan,
+
+        /// Span at which the thunk was originally suspended.
+        suspended_at: Option<LightSpan>,
+
+        /// Span of the first instruction of the actual code inside
+        /// the thunk.
+        content_span: Option<Span>,
+    },
 
     /// Fully evaluated thunk.
     Evaluated(Value),
@@ -113,6 +124,24 @@ impl Thunk {
         )))))
     }
 
+    fn prepare_blackhole(&self, forced_at: LightSpan) -> ThunkRepr {
+        match &*self.0.borrow() {
+            ThunkRepr::Suspended {
+                light_span, lambda, ..
+            } => ThunkRepr::Blackhole {
+                forced_at,
+                suspended_at: Some(light_span.clone()),
+                content_span: Some(lambda.chunk.first_span()),
+            },
+
+            _ => ThunkRepr::Blackhole {
+                forced_at,
+                suspended_at: None,
+                content_span: None,
+            },
+        }
+    }
+
     // TODO(amjoseph): de-asyncify this
     pub async fn force(self, co: GenCo, span: LightSpan) -> Result<Value, ErrorKind> {
         // If the current thunk is already fully evaluated, return its evaluated
@@ -124,13 +153,19 @@ impl Thunk {
         // Begin evaluation of this thunk by marking it as a blackhole, meaning
         // that any other forcing frame encountering this thunk before its
         // evaluation is completed detected an evaluation cycle.
-        let inner = self.0.replace(ThunkRepr::Blackhole { forced_at: span });
+        let inner = self.0.replace(self.prepare_blackhole(span));
 
         match inner {
             // If there was already a blackhole in the thunk, this is an
             // evaluation cycle.
-            ThunkRepr::Blackhole { forced_at } => Err(ErrorKind::InfiniteRecursion {
+            ThunkRepr::Blackhole {
+                forced_at,
+                suspended_at,
+                content_span,
+            } => Err(ErrorKind::InfiniteRecursion {
                 first_force: forced_at.span(),
+                suspended_at: suspended_at.map(|s| s.span()),
+                content_span,
             }),
 
             // If there is a native function stored in the thunk, evaluate it
@@ -148,7 +183,6 @@ impl Thunk {
 
             // When encountering a suspended thunk, request that the VM enters
             // it and produces the result.
-            //
             ThunkRepr::Suspended {
                 lambda,
                 upvalues,