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 = <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>
			
			
This commit is contained in:
		
							parent
							
								
									4ff06ba67d
								
							
						
					
					
						commit
						60f24c3c53
					
				
					 4 changed files with 45 additions and 24 deletions
				
			
		|  | @ -7,12 +7,13 @@ | ||||||
| //! some peculiarities that are encapsulated within this module.
 | //! some peculiarities that are encapsulated within this module.
 | ||||||
| use std::collections::btree_map; | use std::collections::btree_map; | ||||||
| use std::collections::BTreeMap; | use std::collections::BTreeMap; | ||||||
| use std::fmt::Display; |  | ||||||
| 
 | 
 | ||||||
| use crate::errors::ErrorKind; | use crate::errors::ErrorKind; | ||||||
| use crate::vm::VM; | use crate::vm::VM; | ||||||
| 
 | 
 | ||||||
| use super::string::NixString; | use super::string::NixString; | ||||||
|  | use super::thunk::ThunkSet; | ||||||
|  | use super::TotalDisplay; | ||||||
| use super::Value; | use super::Value; | ||||||
| 
 | 
 | ||||||
| #[cfg(test)] | #[cfg(test)] | ||||||
|  | @ -74,19 +75,26 @@ impl AttrsRep { | ||||||
| #[derive(Clone, Debug, PartialEq)] | #[derive(Clone, Debug, PartialEq)] | ||||||
| pub struct NixAttrs(AttrsRep); | pub struct NixAttrs(AttrsRep); | ||||||
| 
 | 
 | ||||||
| impl Display for NixAttrs { | impl TotalDisplay for NixAttrs { | ||||||
|     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { |     fn total_fmt(&self, f: &mut std::fmt::Formatter<'_>, set: &mut ThunkSet) -> std::fmt::Result { | ||||||
|         f.write_str("{ ")?; |         f.write_str("{ ")?; | ||||||
| 
 | 
 | ||||||
|         match &self.0 { |         match &self.0 { | ||||||
|             AttrsRep::KV { name, value } => { |             AttrsRep::KV { name, value } => { | ||||||
|                 write!(f, "name = {}; ", name)?; |                 f.write_str("name = ")?; | ||||||
|                 write!(f, "value = {}; ", value)?; |                 name.total_fmt(f, set)?; | ||||||
|  |                 f.write_str("; ")?; | ||||||
|  | 
 | ||||||
|  |                 f.write_str("value = ")?; | ||||||
|  |                 value.total_fmt(f, set)?; | ||||||
|  |                 f.write_str("; ")?; | ||||||
|             } |             } | ||||||
| 
 | 
 | ||||||
|             AttrsRep::Map(map) => { |             AttrsRep::Map(map) => { | ||||||
|                 for (name, value) in 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("; ")?; | ||||||
|                 } |                 } | ||||||
|             } |             } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -1,21 +1,21 @@ | ||||||
| //! This module implements Nix lists.
 | //! This module implements Nix lists.
 | ||||||
| use std::fmt::Display; |  | ||||||
| 
 |  | ||||||
| use crate::errors::ErrorKind; | use crate::errors::ErrorKind; | ||||||
| use crate::vm::VM; | use crate::vm::VM; | ||||||
| 
 | 
 | ||||||
|  | use super::thunk::ThunkSet; | ||||||
|  | use super::TotalDisplay; | ||||||
| use super::Value; | use super::Value; | ||||||
| 
 | 
 | ||||||
| #[repr(transparent)] | #[repr(transparent)] | ||||||
| #[derive(Clone, Debug, PartialEq)] | #[derive(Clone, Debug, PartialEq)] | ||||||
| pub struct NixList(Vec<Value>); | pub struct NixList(Vec<Value>); | ||||||
| 
 | 
 | ||||||
| impl Display for NixList { | impl TotalDisplay for NixList { | ||||||
|     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { |     fn total_fmt(&self, f: &mut std::fmt::Formatter<'_>, set: &mut ThunkSet) -> std::fmt::Result { | ||||||
|         f.write_str("[ ")?; |         f.write_str("[ ")?; | ||||||
| 
 | 
 | ||||||
|         for v in &self.0 { |         for v in &self.0 { | ||||||
|             v.fmt(f)?; |             v.total_fmt(f, set)?; | ||||||
|             f.write_str(" ")?; |             f.write_str(" ")?; | ||||||
|         } |         } | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -1,9 +1,9 @@ | ||||||
| //! This module implements the backing representation of runtime
 | //! This module implements the backing representation of runtime
 | ||||||
| //! values in the Nix language.
 | //! values in the Nix language.
 | ||||||
| use std::cell::Ref; |  | ||||||
| use std::ops::Deref; | use std::ops::Deref; | ||||||
|  | use std::path::PathBuf; | ||||||
| use std::rc::Rc; | use std::rc::Rc; | ||||||
| use std::{fmt::Display, path::PathBuf}; | use std::{cell::Ref, fmt::Display}; | ||||||
| 
 | 
 | ||||||
| #[cfg(feature = "arbitrary")] | #[cfg(feature = "arbitrary")] | ||||||
| mod 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 { | impl Display for Value { | ||||||
|     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { |     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 { |         match self { | ||||||
|             Value::Null => f.write_str("null"), |             Value::Null => f.write_str("null"), | ||||||
|             Value::Bool(true) => f.write_str("true"), |             Value::Bool(true) => f.write_str("true"), | ||||||
|  | @ -397,8 +407,8 @@ impl Display for Value { | ||||||
|             Value::Integer(num) => write!(f, "{}", num), |             Value::Integer(num) => write!(f, "{}", num), | ||||||
|             Value::String(s) => s.fmt(f), |             Value::String(s) => s.fmt(f), | ||||||
|             Value::Path(p) => p.display().fmt(f), |             Value::Path(p) => p.display().fmt(f), | ||||||
|             Value::Attrs(attrs) => attrs.fmt(f), |             Value::Attrs(attrs) => attrs.total_fmt(f, set), | ||||||
|             Value::List(list) => list.fmt(f), |             Value::List(list) => list.total_fmt(f, set), | ||||||
|             Value::Closure(_) => f.write_str("lambda"), // TODO: print position
 |             Value::Closure(_) => f.write_str("lambda"), // TODO: print position
 | ||||||
|             Value::Builtin(builtin) => builtin.fmt(f), |             Value::Builtin(builtin) => builtin.fmt(f), | ||||||
| 
 | 
 | ||||||
|  | @ -408,15 +418,15 @@ impl Display for Value { | ||||||
|                 write!(f, "{}", format!("{:.5}", num).trim_end_matches(['.', '0'])) |                 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
 |             // internal types
 | ||||||
|             Value::AttrNotFound => f.write_str("internal[not found]"), |             Value::AttrNotFound => f.write_str("internal[not found]"), | ||||||
|             Value::Blueprint(_) => f.write_str("internal[blueprint]"), |             Value::Blueprint(_) => f.write_str("internal[blueprint]"), | ||||||
|             Value::DeferredUpvalue(_) => f.write_str("internal[deferred_upvalue]"), |             Value::DeferredUpvalue(_) => f.write_str("internal[deferred_upvalue]"), | ||||||
|             Value::UnresolvedPath(_) => f.write_str("internal[unresolved_path]"), |             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), | ||||||
|         } |         } | ||||||
|     } |     } | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -21,7 +21,6 @@ | ||||||
| use std::{ | use std::{ | ||||||
|     cell::{Ref, RefCell, RefMut}, |     cell::{Ref, RefCell, RefMut}, | ||||||
|     collections::HashSet, |     collections::HashSet, | ||||||
|     fmt::Display, |  | ||||||
|     rc::Rc, |     rc::Rc, | ||||||
| }; | }; | ||||||
| 
 | 
 | ||||||
|  | @ -35,7 +34,7 @@ use crate::{ | ||||||
|     Value, |     Value, | ||||||
| }; | }; | ||||||
| 
 | 
 | ||||||
| use super::Lambda; | use super::{Lambda, TotalDisplay}; | ||||||
| 
 | 
 | ||||||
| /// Internal representation of the different states of a thunk.
 | /// Internal representation of the different states of a thunk.
 | ||||||
| ///
 | ///
 | ||||||
|  | @ -188,11 +187,15 @@ impl Thunk { | ||||||
|     } |     } | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| impl Display for Thunk { | impl TotalDisplay for Thunk { | ||||||
|     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { |     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() { |         match self.0.try_borrow() { | ||||||
|             Ok(repr) => match &*repr { |             Ok(repr) => match &*repr { | ||||||
|                 ThunkRepr::Evaluated(v) => v.fmt(f), |                 ThunkRepr::Evaluated(v) => v.total_fmt(f, set), | ||||||
|                 _ => f.write_str("internal[thunk]"), |                 _ => f.write_str("internal[thunk]"), | ||||||
|             }, |             }, | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue