Clearer error messages for failed comparisons

Fixes #1231

library/src/compute/calc.rs

@@ -831,20 +831,11 @@ fn minmax(
     };
 
     for Spanned { v, span } in iter {
-        match v.partial_cmp(&extremum) {
-            Some(ordering) => {
+        let ordering = typst::eval::ops::compare(&v, &extremum).at(span)?;
         if ordering == goal {
             extremum = v;
         }
     }
-            None => bail!(
-                span,
-                "cannot compare {} and {}",
-                extremum.type_name(),
-                v.type_name(),
-            ),
-        }
-    }
 
     Ok(extremum)
 }

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(|| {
+                (Ok(a), Ok(b)) => {
+                    typst::eval::ops::compare(&a, &b).unwrap_or_else(|err| {
                         if result.is_ok() {
-                        result = Err(eco_format!(
-                            "cannot order {} and {}",
-                            a.type_name(),
-                            b.type_name(),
-                        ))
-                        .at(span);
+                            result = Err(err).at(span);
                         }
                         Ordering::Equal
-                }),
+                    })
+                }
                 (Err(e), _) | (_, Err(e)) => {
                     if result.is_ok() {
                         result = Err(e);

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;
 

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<Value> {
-            if let Some(ordering) = compare(&lhs, &rhs) {
+            let ordering = compare(&lhs, &rhs)?;
             Ok(Bool(matches!(ordering, $($pat)*)))
-            } else {
-                mismatch!(concat!("cannot apply '", $op, "' to {} and {}"), lhs, rhs);
-            }
         }
     };
 }
@@ -371,28 +369,34 @@ pub fn equal(lhs: &Value, rhs: &Value) -> bool {
 }
 
 /// Compare two values.
-pub fn compare(lhs: &Value, rhs: &Value) -> Option<Ordering> {
-    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<Ordering> {
+    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<T: PartialOrd + Debug>(a: &T, b: &T) -> StrResult<Ordering> {
+    a.partial_cmp(b)
+        .ok_or_else(|| eco_format!("cannot compare {:?} with {:?}", a, b))
 }
 
 /// Test whether one value is "in" another one.

src/eval/value.rs

@@ -207,7 +207,7 @@ impl PartialEq for Value {
 
 impl PartialOrd for Value {
     fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
-        ops::compare(self, other)
+        ops::compare(self, other).ok()
     }
 }
 

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)

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)

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))