From 08870d4a4c3890b9e36c67c8972e41f6af1e0042 Mon Sep 17 00:00:00 2001 From: Laurenz Date: Mon, 22 May 2023 13:03:35 +0200 Subject: [PATCH] Clearer error messages for failed comparisons Fixes #1231 --- library/src/compute/calc.rs | 15 ++------- src/eval/array.rs | 19 +++++------ src/eval/mod.rs | 2 +- src/eval/ops.rs | 52 ++++++++++++++++-------------- src/eval/value.rs | 2 +- tests/typ/compiler/array.typ | 6 +++- tests/typ/compiler/ops-invalid.typ | 8 +++-- tests/typ/compute/calc.typ | 6 +++- 8 files changed, 57 insertions(+), 53 deletions(-) diff --git a/library/src/compute/calc.rs b/library/src/compute/calc.rs index bd673c542..f485f8171 100644 --- a/library/src/compute/calc.rs +++ b/library/src/compute/calc.rs @@ -831,18 +831,9 @@ fn minmax( }; for Spanned { v, span } in iter { - match v.partial_cmp(&extremum) { - Some(ordering) => { - if ordering == goal { - extremum = v; - } - } - None => bail!( - span, - "cannot compare {} and {}", - extremum.type_name(), - v.type_name(), - ), + let ordering = typst::eval::ops::compare(&v, &extremum).at(span)?; + if ordering == goal { + extremum = v; } } diff --git a/src/eval/array.rs b/src/eval/array.rs index 6ae5d7cfe..d0b63b352 100644 --- a/src/eval/array.rs +++ b/src/eval/array.rs @@ -344,17 +344,14 @@ impl Array { vec.make_mut().sort_by(|a, b| { // Until we get `try` blocks :) match (key_of(a.clone()), key_of(b.clone())) { - (Ok(a), Ok(b)) => a.partial_cmp(&b).unwrap_or_else(|| { - if result.is_ok() { - result = Err(eco_format!( - "cannot order {} and {}", - a.type_name(), - b.type_name(), - )) - .at(span); - } - Ordering::Equal - }), + (Ok(a), Ok(b)) => { + typst::eval::ops::compare(&a, &b).unwrap_or_else(|err| { + if result.is_ok() { + result = Err(err).at(span); + } + Ordering::Equal + }) + } (Err(e), _) | (_, Err(e)) => { if result.is_ok() { result = Err(e); diff --git a/src/eval/mod.rs b/src/eval/mod.rs index e80c95c88..a91bea10b 100644 --- a/src/eval/mod.rs +++ b/src/eval/mod.rs @@ -16,7 +16,7 @@ mod args; mod func; mod methods; mod module; -mod ops; +pub mod ops; mod scope; mod symbol; diff --git a/src/eval/ops.rs b/src/eval/ops.rs index 48e74c8ff..3e397c30b 100644 --- a/src/eval/ops.rs +++ b/src/eval/ops.rs @@ -1,6 +1,7 @@ //! Operations on values. use std::cmp::Ordering; +use std::fmt::Debug; use ecow::eco_format; @@ -318,11 +319,8 @@ macro_rules! comparison { ($name:ident, $op:tt, $($pat:tt)*) => { /// Compute how a value compares with another value. pub fn $name(lhs: Value, rhs: Value) -> StrResult { - if let Some(ordering) = compare(&lhs, &rhs) { - Ok(Bool(matches!(ordering, $($pat)*))) - } else { - mismatch!(concat!("cannot apply '", $op, "' to {} and {}"), lhs, rhs); - } + let ordering = compare(&lhs, &rhs)?; + Ok(Bool(matches!(ordering, $($pat)*))) } }; } @@ -371,28 +369,34 @@ pub fn equal(lhs: &Value, rhs: &Value) -> bool { } /// Compare two values. -pub fn compare(lhs: &Value, rhs: &Value) -> Option { - match (lhs, rhs) { - (Bool(a), Bool(b)) => a.partial_cmp(b), - (Int(a), Int(b)) => a.partial_cmp(b), - (Float(a), Float(b)) => a.partial_cmp(b), - (Length(a), Length(b)) => a.partial_cmp(b), - (Angle(a), Angle(b)) => a.partial_cmp(b), - (Ratio(a), Ratio(b)) => a.partial_cmp(b), - (Relative(a), Relative(b)) => a.partial_cmp(b), - (Fraction(a), Fraction(b)) => a.partial_cmp(b), - (Str(a), Str(b)) => a.partial_cmp(b), +pub fn compare(lhs: &Value, rhs: &Value) -> StrResult { + Ok(match (lhs, rhs) { + (Bool(a), Bool(b)) => a.cmp(b), + (Int(a), Int(b)) => a.cmp(b), + (Float(a), Float(b)) => try_cmp_values(a, b)?, + (Length(a), Length(b)) => try_cmp_values(a, b)?, + (Angle(a), Angle(b)) => a.cmp(b), + (Ratio(a), Ratio(b)) => a.cmp(b), + (Relative(a), Relative(b)) => try_cmp_values(a, b)?, + (Fraction(a), Fraction(b)) => a.cmp(b), + (Str(a), Str(b)) => a.cmp(b), // Some technically different things should be comparable. - (&Int(a), &Float(b)) => (a as f64).partial_cmp(&b), - (&Float(a), &Int(b)) => a.partial_cmp(&(b as f64)), - (&Length(a), &Relative(b)) if b.rel.is_zero() => a.partial_cmp(&b.abs), - (&Ratio(a), &Relative(b)) if b.abs.is_zero() => a.partial_cmp(&b.rel), - (&Relative(a), &Length(b)) if a.rel.is_zero() => a.abs.partial_cmp(&b), - (&Relative(a), &Ratio(b)) if a.abs.is_zero() => a.rel.partial_cmp(&b), + (Int(a), Float(b)) => try_cmp_values(&(*a as f64), b)?, + (Float(a), Int(b)) => try_cmp_values(a, &(*b as f64))?, + (Length(a), Relative(b)) if b.rel.is_zero() => try_cmp_values(a, &b.abs)?, + (Ratio(a), Relative(b)) if b.abs.is_zero() => a.cmp(&b.rel), + (Relative(a), Length(b)) if a.rel.is_zero() => try_cmp_values(&a.abs, b)?, + (Relative(a), Ratio(b)) if a.abs.is_zero() => a.rel.cmp(b), - _ => Option::None, - } + _ => mismatch!("cannot compare {} and {}", lhs, rhs), + }) +} + +/// Try to compare two values. +fn try_cmp_values(a: &T, b: &T) -> StrResult { + a.partial_cmp(b) + .ok_or_else(|| eco_format!("cannot compare {:?} with {:?}", a, b)) } /// Test whether one value is "in" another one. diff --git a/src/eval/value.rs b/src/eval/value.rs index 36cda80b6..6d9478660 100644 --- a/src/eval/value.rs +++ b/src/eval/value.rs @@ -207,7 +207,7 @@ impl PartialEq for Value { impl PartialOrd for Value { fn partial_cmp(&self, other: &Self) -> Option { - ops::compare(self, other) + ops::compare(self, other).ok() } } diff --git a/tests/typ/compiler/array.typ b/tests/typ/compiler/array.typ index ef6e4b6ba..59e9f70b6 100644 --- a/tests/typ/compiler/array.typ +++ b/tests/typ/compiler/array.typ @@ -244,9 +244,13 @@ #(1, 2, 0, 3).sorted(key: x => 5 / x) --- -// Error: 2-26 cannot order content and content +// Error: 2-26 cannot compare content and content #([Hi], [There]).sorted() +--- +// Error: 2-26 cannot compare 3em with 2pt +#(1pt, 2pt, 3em).sorted() + --- // Error: 2-18 array index out of bounds (index: -4, len: 3) and no default value was specified #(1, 2, 3).at(-4) diff --git a/tests/typ/compiler/ops-invalid.typ b/tests/typ/compiler/ops-invalid.typ index 2af3b767e..0ed26a569 100644 --- a/tests/typ/compiler/ops-invalid.typ +++ b/tests/typ/compiler/ops-invalid.typ @@ -26,13 +26,17 @@ #(not ()) --- -// Error: 3-19 cannot apply '<=' to relative length and ratio +// Error: 3-19 cannot compare relative length and ratio #(30% + 1pt <= 40%) --- -// Error: 3-14 cannot apply '<=' to length and length +// Error: 3-14 cannot compare 1em with 10pt #(1em <= 10pt) +--- +// Error: 3-22 cannot compare 2.2 with NaN +#(2.2 <= float("nan")) + --- // Error: 3-12 cannot divide by zero #(1.2 / 0.0) diff --git a/tests/typ/compute/calc.typ b/tests/typ/compute/calc.typ index 9ff7c8bd6..3a1e85f32 100644 --- a/tests/typ/compute/calc.typ +++ b/tests/typ/compute/calc.typ @@ -191,9 +191,13 @@ #calc.min() --- -// Error: 14-18 cannot compare integer and string +// Error: 14-18 cannot compare string and integer #calc.min(1, "hi") +--- +// Error: 16-19 cannot compare 1pt with 1em +#calc.max(1em, 1pt) + --- // Test the `range` function. #test(range(4), (0, 1, 2, 3))