diff options
author | Vincent Ambo <mail@tazj.in> | 2022-11-20T22·52+0100 |
---|---|---|
committer | tazjin <tazjin@tvl.su> | 2022-11-21T15·26+0000 |
commit | 878dc6c22717971f5dd4720f314f36c47a0c762e (patch) | |
tree | 2f117566e5adc825552570de70d2ca6d9b48695f | |
parent | f7907db69d07d9358a269e86237524b0142c23c0 (diff) |
fix(tvix/eval): aggressively fix a borrow error in nix_eq r/5299
When comparing Nix values for equality, an issue can occur where recursive values contain thunks to themselves which causes borrow errors when forcing them for comparison later down the line. To work around this we clone the values for now. There might be some optimisations possible like checking for thunk equality directly and short-circuiting on that (we have to check what Nix does). Change-Id: I7e75c992ea68f100058f52b4b46168da7d671994 Reviewed-on: https://cl.tvl.fyi/c/depot/+/7314 Reviewed-by: sterni <sternenseemann@systemli.org> Tested-by: BuildkiteCI
-rw-r--r-- | tvix/eval/src/value/mod.rs | 12 |
1 files changed, 9 insertions, 3 deletions
diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index f0bb3628d831..ed29ae938aa3 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -326,13 +326,19 @@ impl Value { // Optimised attribute set comparison (Value::Attrs(a1), Value::Attrs(a2)) => Ok(Rc::ptr_eq(a1, a2) || a1.nix_eq(a2, vm)?), - // If either value is a thunk, the thunk should be forced, and then the resulting value - // must be compared instead. + // If either value is a thunk, the thunk should be forced, and then + // the resulting value must be compared instead. (Value::Thunk(lhs), Value::Thunk(rhs)) => { lhs.force(vm)?; rhs.force(vm)?; - lhs.value().nix_eq(&*rhs.value(), vm) + // TODO: this cloning is done because there is a potential issue + // with keeping borrows into both thunks around while recursing, + // as they might recurse themselves, leading to a borrow error + // when they are later being forced. + let lhs = lhs.value().clone(); + let rhs = rhs.value().clone(); + lhs.nix_eq(&rhs, vm) } (Value::Thunk(lhs), rhs) => { lhs.force(vm)?; |