From fea7f90a7fa6024241cfc9bae7f8b9cc6e7ba13a Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Wed, 31 Aug 2022 04:42:11 +0300 Subject: fix(tvix/eval): thread Display & PartialEq through to thunk values If a thunk is already evaluated, there are cases where due to the memoisation implementation something might observe a value wrapped in a thunk. In these cases, the implementation of `Display` and `PartialEq` must delegate to the underlying value. Note that there are a handful of other cases like these which we need to cover. It is a little tricky to write integration tests for these directly, especially as some of the open-upvalue optimisations coming down the pipe will reduce the number of observable thunks. One test that covers a part of this behaviour is currently disabled (needs some more machinery), but it's being brought back in the next commits. Change-Id: Iaa8cd338c12236af844bbc99d8cec2205f0d0095 Reviewed-on: https://cl.tvl.fyi/c/depot/+/6370 Reviewed-by: sterni Tested-by: BuildkiteCI --- tvix/eval/src/value/mod.rs | 12 +++++++++++- tvix/eval/src/value/thunk.rs | 10 ++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) (limited to 'tvix/eval') diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index f4a67eb8b94b..c5565e6176b3 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -172,8 +172,11 @@ 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::Thunk(_) => f.write_str("internal[thunk]"), Value::AttrPath(path) => write!(f, "internal[attrpath({})]", path.len()), Value::AttrNotFound => f.write_str("internal[not found]"), Value::Blueprint(_) => f.write_str("internal[blueprint]"), @@ -203,6 +206,13 @@ impl PartialEq for Value { // Optimised attribute set comparison (Value::Attrs(a1), Value::Attrs(a2)) => Rc::ptr_eq(a1, a2) || { a1 == a2 }, + // If either value is a thunk, the inner value must be + // compared instead. The compiler should ensure that + // thunks under comparison have been forced, otherwise it + // is a bug. + (Value::Thunk(lhs), rhs) => &*lhs.value() == rhs, + (lhs, Value::Thunk(rhs)) => lhs == &*rhs.value(), + // Everything else is either incomparable (e.g. internal // types) or false. // TODO(tazjin): mirror Lambda equality behaviour diff --git a/tvix/eval/src/value/thunk.rs b/tvix/eval/src/value/thunk.rs index 3c3941e49402..307eb23a75f6 100644 --- a/tvix/eval/src/value/thunk.rs +++ b/tvix/eval/src/value/thunk.rs @@ -20,6 +20,7 @@ use std::{ cell::{Ref, RefCell, RefMut}, + fmt::Display, rc::Rc, }; @@ -131,3 +132,12 @@ impl UpvalueCarrier for Thunk { }) } } + +impl Display for Thunk { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match &*self.0.borrow() { + ThunkRepr::Evaluated(v) => v.fmt(f), + _ => f.write_str("internal[thunk]"), + } + } +} -- cgit 1.4.1