From 60f24c3c53eae116bb3051f02f05b74a8ecf37af Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Sat, 22 Oct 2022 16:51:29 +0300 Subject: fix(tvix/eval): detect cycles when printing infinite values 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 = ; }; } > tvix-eval -E 'let as = { x = 123; y = as; }; in as' => { x = 123; y = { x = 123; y = ; }; } :: set Change-Id: I007b918d5131d82c28884e46e46ff365ef691aa8 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7056 Tested-by: BuildkiteCI Reviewed-by: grfn --- tvix/eval/src/value/attrs.rs | 20 ++++++++++++++------ tvix/eval/src/value/list.rs | 10 +++++----- tvix/eval/src/value/mod.rs | 26 ++++++++++++++++++-------- tvix/eval/src/value/thunk.rs | 13 ++++++++----- 4 files changed, 45 insertions(+), 24 deletions(-) (limited to 'tvix/eval') 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); -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(""); + } + 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]"), }, -- cgit 1.4.1