fix(tvix/eval): detect cycles when printing infinite values r/5178

Using the same method as in Thunk::deep_force, detect cycles when
printing values by maintaining a set of already seen thunks.

With this, display of infinite values matches that of Nix:

    > nix-instantiate --eval --strict -E 'let as = { x = 123; y = as; }; in as'
    { x = 123; y = { x = 123; y = <CYCLE>; }; }

    > tvix-eval -E 'let as = { x = 123; y = as; }; in as'
    => { x = 123; y = { x = 123; y = <CYCLE>; }; } :: set

Change-Id: I007b918d5131d82c28884e46e46ff365ef691aa8
Reviewed-on: https://cl.tvl.fyi/c/depot/+/7056
Tested-by: BuildkiteCI
Reviewed-by: grfn <grfn@gws.fyi>

4 files changed, 45 insertions, 24 deletions

diff --git a/tvix/eval/src/value/attrs.rs b/tvix/eval/src/value/attrs.rs
index a67fd2f3e3..17c7b422cb 100644
--- a/tvix/eval/src/value/attrs.rs
+++ b/tvix/eval/src/value/attrs.rs
@@ -7,12 +7,13 @@
 //! some peculiarities that are encapsulated within this module.
 use std::collections::btree_map;
 use std::collections::BTreeMap;
-use std::fmt::Display;
 
 use crate::errors::ErrorKind;
 use crate::vm::VM;
 
 use super::string::NixString;
+use super::thunk::ThunkSet;
+use super::TotalDisplay;
 use super::Value;
 
 #[cfg(test)]
@@ -74,19 +75,26 @@ impl AttrsRep {
 #[derive(Clone, Debug, PartialEq)]
 pub struct NixAttrs(AttrsRep);
 
-impl Display for NixAttrs {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+impl TotalDisplay for NixAttrs {
+    fn total_fmt(&self, f: &mut std::fmt::Formatter<'_>, set: &mut ThunkSet) -> std::fmt::Result {
         f.write_str("{ ")?;
 
         match &self.0 {
             AttrsRep::KV { name, value } => {
-                write!(f, "name = {}; ", name)?;
-                write!(f, "value = {}; ", value)?;
+                f.write_str("name = ")?;
+                name.total_fmt(f, set)?;
+                f.write_str("; ")?;
+
+                f.write_str("value = ")?;
+                value.total_fmt(f, set)?;
+                f.write_str("; ")?;
             }
 
             AttrsRep::Map(map) => {
                 for (name, value) in map {
-                    write!(f, "{} = {}; ", name.ident_str(), value)?;
+                    write!(f, "{} = ", name.ident_str())?;
+                    value.total_fmt(f, set)?;
+                    f.write_str("; ")?;
                 }
             }
 
diff --git a/tvix/eval/src/value/list.rs b/tvix/eval/src/value/list.rs
index 42d91b6b26..d13e220bbe 100644
--- a/tvix/eval/src/value/list.rs
+++ b/tvix/eval/src/value/list.rs
@@ -1,21 +1,21 @@
 //! This module implements Nix lists.
-use std::fmt::Display;
-
 use crate::errors::ErrorKind;
 use crate::vm::VM;
 
+use super::thunk::ThunkSet;
+use super::TotalDisplay;
 use super::Value;
 
 #[repr(transparent)]
 #[derive(Clone, Debug, PartialEq)]
 pub struct NixList(Vec<Value>);
 
-impl Display for NixList {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+impl TotalDisplay for NixList {
+    fn total_fmt(&self, f: &mut std::fmt::Formatter<'_>, set: &mut ThunkSet) -> std::fmt::Result {
         f.write_str("[ ")?;
 
         for v in &self.0 {
-            v.fmt(f)?;
+            v.total_fmt(f, set)?;
             f.write_str(" ")?;
         }
 
diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs
index 1dc39d6832..c065d5c1bd 100644
--- a/tvix/eval/src/value/mod.rs
+++ b/tvix/eval/src/value/mod.rs
@@ -1,9 +1,9 @@
 //! This module implements the backing representation of runtime
 //! values in the Nix language.
-use std::cell::Ref;
 use std::ops::Deref;
+use std::path::PathBuf;
 use std::rc::Rc;
-use std::{fmt::Display, path::PathBuf};
+use std::{cell::Ref, fmt::Display};
 
 #[cfg(feature = "arbitrary")]
 mod arbitrary;
@@ -388,8 +388,18 @@ impl Value {
     }
 }
 
+trait TotalDisplay {
+    fn total_fmt(&self, f: &mut std::fmt::Formatter<'_>, set: &mut ThunkSet) -> std::fmt::Result;
+}
+
 impl Display for Value {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.total_fmt(f, &mut Default::default())
+    }
+}
+
+impl TotalDisplay for Value {
+    fn total_fmt(&self, f: &mut std::fmt::Formatter<'_>, set: &mut ThunkSet) -> std::fmt::Result {
         match self {
             Value::Null => f.write_str("null"),
             Value::Bool(true) => f.write_str("true"),
@@ -397,8 +407,8 @@ impl Display for Value {
             Value::Integer(num) => write!(f, "{}", num),
             Value::String(s) => s.fmt(f),
             Value::Path(p) => p.display().fmt(f),
-            Value::Attrs(attrs) => attrs.fmt(f),
-            Value::List(list) => list.fmt(f),
+            Value::Attrs(attrs) => attrs.total_fmt(f, set),
+            Value::List(list) => list.total_fmt(f, set),
             Value::Closure(_) => f.write_str("lambda"), // TODO: print position
             Value::Builtin(builtin) => builtin.fmt(f),
 
@@ -408,15 +418,15 @@ impl Display for Value {
                 write!(f, "{}", format!("{:.5}", num).trim_end_matches(['.', '0']))
             }
 
-            // Delegate thunk display to the type, as it must handle
-            // the case of already evaluated thunks.
-            Value::Thunk(t) => t.fmt(f),
-
             // internal types
             Value::AttrNotFound => f.write_str("internal[not found]"),
             Value::Blueprint(_) => f.write_str("internal[blueprint]"),
             Value::DeferredUpvalue(_) => f.write_str("internal[deferred_upvalue]"),
             Value::UnresolvedPath(_) => f.write_str("internal[unresolved_path]"),
+
+            // Delegate thunk display to the type, as it must handle
+            // the case of already evaluated or cyclic thunks.
+            Value::Thunk(t) => t.total_fmt(f, set),
         }
     }
 }
diff --git a/tvix/eval/src/value/thunk.rs b/tvix/eval/src/value/thunk.rs
index 7bad7e8777..bcf8e2fc87 100644
--- a/tvix/eval/src/value/thunk.rs
+++ b/tvix/eval/src/value/thunk.rs
@@ -21,7 +21,6 @@
 use std::{
     cell::{Ref, RefCell, RefMut},
     collections::HashSet,
-    fmt::Display,
     rc::Rc,
 };
 
@@ -35,7 +34,7 @@ use crate::{
     Value,
 };
 
-use super::Lambda;
+use super::{Lambda, TotalDisplay};
 
 /// Internal representation of the different states of a thunk.
 ///
@@ -188,11 +187,15 @@ impl Thunk {
     }
 }
 
-impl Display for Thunk {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+impl TotalDisplay for Thunk {
+    fn total_fmt(&self, f: &mut std::fmt::Formatter<'_>, set: &mut ThunkSet) -> std::fmt::Result {
+        if !set.insert(self) {
+            return f.write_str("<CYCLE>");
+        }
+
         match self.0.try_borrow() {
             Ok(repr) => match &*repr {
-                ThunkRepr::Evaluated(v) => v.fmt(f),
+                ThunkRepr::Evaluated(v) => v.total_fmt(f, set),
                 _ => f.write_str("internal[thunk]"),
             },