From ad566999ca81e5d584acb5107e327a1aec9fdd5a Mon Sep 17 00:00:00 2001 From: Adam Joseph Date: Sat, 9 Dec 2023 00:58:30 -0800 Subject: fix(tvix/eval): preserve catchables in nix_cmp_ordering(), fix b/338 This commit fixes b/338 by properly propagating catchables through comparison operations. Change-Id: I6b0283a40f228ecf9a6398d24c060bdacb1077cf Reviewed-on: https://cl.tvl.fyi/c/depot/+/10221 Reviewed-by: tazjin Tested-by: BuildkiteCI Autosubmit: Adam Joseph --- tvix/eval/src/builtins/mod.rs | 9 +++++---- .../tests/tvix_tests/eval-okay-compare-ordering-catchable.exp | 1 + .../tests/tvix_tests/eval-okay-compare-ordering-catchable.nix | 1 + .../notyetpassing/eval-okay-compare-ordering-catchable.exp | 1 - .../notyetpassing/eval-okay-compare-ordering-catchable.nix | 1 - tvix/eval/src/value/mod.rs | 10 ++++++---- tvix/eval/src/vm/macros.rs | 5 ++++- 7 files changed, 17 insertions(+), 11 deletions(-) create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-compare-ordering-catchable.exp create mode 100644 tvix/eval/src/tests/tvix_tests/eval-okay-compare-ordering-catchable.nix delete mode 100644 tvix/eval/src/tests/tvix_tests/notyetpassing/eval-okay-compare-ordering-catchable.exp delete mode 100644 tvix/eval/src/tests/tvix_tests/notyetpassing/eval-okay-compare-ordering-catchable.nix (limited to 'tvix/eval/src') diff --git a/tvix/eval/src/builtins/mod.rs b/tvix/eval/src/builtins/mod.rs index 1a34ea5dfb..09b341886c 100644 --- a/tvix/eval/src/builtins/mod.rs +++ b/tvix/eval/src/builtins/mod.rs @@ -611,10 +611,11 @@ mod pure_builtins { #[builtin("lessThan")] async fn builtin_less_than(co: GenCo, x: Value, y: Value) -> Result { let span = generators::request_span(&co).await; - Ok(Value::Bool(matches!( - x.nix_cmp_ordering(y, co, span).await?, - Ordering::Less - ))) + match x.nix_cmp_ordering(y, co, span).await? { + Err(cek) => Ok(Value::Catchable(cek)), + Ok(Ordering::Less) => Ok(Value::Bool(true)), + Ok(_) => Ok(Value::Bool(false)), + } } #[builtin("listToAttrs")] diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-compare-ordering-catchable.exp b/tvix/eval/src/tests/tvix_tests/eval-okay-compare-ordering-catchable.exp new file mode 100644 index 0000000000..c508d5366f --- /dev/null +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-compare-ordering-catchable.exp @@ -0,0 +1 @@ +false diff --git a/tvix/eval/src/tests/tvix_tests/eval-okay-compare-ordering-catchable.nix b/tvix/eval/src/tests/tvix_tests/eval-okay-compare-ordering-catchable.nix new file mode 100644 index 0000000000..9000160e57 --- /dev/null +++ b/tvix/eval/src/tests/tvix_tests/eval-okay-compare-ordering-catchable.nix @@ -0,0 +1 @@ +(builtins.tryEval ((throw "x") < 3)).success diff --git a/tvix/eval/src/tests/tvix_tests/notyetpassing/eval-okay-compare-ordering-catchable.exp b/tvix/eval/src/tests/tvix_tests/notyetpassing/eval-okay-compare-ordering-catchable.exp deleted file mode 100644 index c508d5366f..0000000000 --- a/tvix/eval/src/tests/tvix_tests/notyetpassing/eval-okay-compare-ordering-catchable.exp +++ /dev/null @@ -1 +0,0 @@ -false diff --git a/tvix/eval/src/tests/tvix_tests/notyetpassing/eval-okay-compare-ordering-catchable.nix b/tvix/eval/src/tests/tvix_tests/notyetpassing/eval-okay-compare-ordering-catchable.nix deleted file mode 100644 index 9000160e57..0000000000 --- a/tvix/eval/src/tests/tvix_tests/notyetpassing/eval-okay-compare-ordering-catchable.nix +++ /dev/null @@ -1 +0,0 @@ -(builtins.tryEval ((throw "x") < 3)).success diff --git a/tvix/eval/src/value/mod.rs b/tvix/eval/src/value/mod.rs index 7b64e9fc8f..9be57d43f5 100644 --- a/tvix/eval/src/value/mod.rs +++ b/tvix/eval/src/value/mod.rs @@ -616,7 +616,7 @@ impl Value { other: Self, co: GenCo, span: LightSpan, - ) -> Result { + ) -> Result, ErrorKind> { Self::nix_cmp_ordering_(self, other, co, span).await } @@ -625,7 +625,7 @@ impl Value { other: Self, co: GenCo, span: LightSpan, - ) -> Result { + ) -> Result, ErrorKind> { // this is a stack of ((v1,v2),peq) triples to be compared; // after each triple is popped off of the stack, v1 is // compared to v2 using peq-mode PointerEquality @@ -636,7 +636,7 @@ impl Value { abp } else { // stack is empty, so they are equal - return Ok(Ordering::Equal); + return Ok(Ok(Ordering::Equal)); }; if ptr_eq == PointerEquality::AllowAll { if a.clone() @@ -650,6 +650,8 @@ impl Value { b = b.force(&co, span.clone()).await?; } let result = match (a, b) { + (Value::Catchable(c), _) => return Ok(Err(c)), + (_, Value::Catchable(c)) => return Ok(Err(c)), // same types (Value::Integer(i1), Value::Integer(i2)) => i1.cmp(&i2), (Value::Float(f1), Value::Float(f2)) => f1.total_cmp(&f2), @@ -682,7 +684,7 @@ impl Value { } }; if result != Ordering::Equal { - return Ok(result); + return Ok(Ok(result)); } } } diff --git a/tvix/eval/src/vm/macros.rs b/tvix/eval/src/vm/macros.rs index 34e94bb5fa..eb386df92f 100644 --- a/tvix/eval/src/vm/macros.rs +++ b/tvix/eval/src/vm/macros.rs @@ -44,7 +44,10 @@ macro_rules! cmp_op { let b = generators::request_force(&co, b).await; let span = generators::request_span(&co).await; let ordering = a.nix_cmp_ordering(b, co, span).await?; - Ok(Value::Bool(cmp_op!(@order $op ordering))) + match ordering { + Err(cek) => Ok(Value::Catchable(cek)), + Ok(ordering) => Ok(Value::Bool(cmp_op!(@order $op ordering))), + } } let gen_span = $frame.current_light_span(); -- cgit 1.4.1