Raise error when comparing base units

… instead of returning false closes #230
sharkdp · Nov 10, 2023 · 9605739 · 9605739
1 parent 527e87e
commit 9605739
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 6 deletions.
diff --git a/numbat/src/interpreter.rs b/numbat/src/interpreter.rs
@@ -147,6 +147,8 @@ mod tests {
         @aliases(m: short)
         unit meter : Length
 
+        unit alternative_length_base_unit: Length # maybe this should be disallowed
+
         @aliases(s: short)
         unit second : Time
 
@@ -219,6 +221,22 @@ mod tests {
         assert_evaluates_to_scalar("+2 - +3", 2.0 - 3.0);
     }
 
+    #[test]
+    fn comparisons() {
+        assert_evaluates_to_scalar("if 2 meter > 150 cm then 1 else 0", 1.0);
+
+        assert_runtime_error(
+            "1 meter > alternative_length_base_unit",
+            RuntimeError::QuantityError(QuantityError::IncompatibleUnits(
+                Unit::new_base("meter", "m"),
+                Unit::new_base(
+                    "alternative_length_base_unit",
+                    "alternative_length_base_unit",
+                ),
+            )),
+        );
+    }
+
     #[test]
     fn arithmetic_with_units() {
         use crate::unit::Unit;

diff --git a/numbat/src/typechecker.rs b/numbat/src/typechecker.rs
@@ -1489,6 +1489,17 @@ mod tests {
         ));
     }
 
+    #[test]
+    fn converisons() {
+        assert_successful_typecheck("2 a > a");
+        assert_successful_typecheck("2 a / (3 a) > 3");
+
+        assert!(matches!(
+            get_typecheck_error("a > b"),
+            TypeCheckError::IncompatibleDimensions(..)
+        ));
+    }
+
     #[test]
     fn variable_definitions() {
         assert_successful_typecheck(

diff --git a/numbat/src/vm.rs b/numbat/src/vm.rs
@@ -1,12 +1,12 @@
-use std::fmt::Display;
+use std::{cmp::Ordering, fmt::Display};
 
 use crate::{
     ffi::{self, ArityRange, Callable, ForeignFunction},
     interpreter::{InterpreterResult, PrintFunction, Result, RuntimeError},
     markup::Markup,
     math,
     prefix::Prefix,
-    quantity::Quantity,
+    quantity::{Quantity, QuantityError},
     unit::Unit,
     unit_registry::{UnitMetadata, UnitRegistry},
     value::Value,
@@ -606,11 +606,18 @@ impl Vm {
                     let rhs = self.pop_quantity();
                     let lhs = self.pop_quantity();
 
+                    let result = lhs.partial_cmp(&rhs).ok_or_else(|| {
+                        RuntimeError::QuantityError(QuantityError::IncompatibleUnits(
+                            lhs.unit().clone(),
+                            rhs.unit().clone(),
+                        ))
+                    })?;
+
                     let result = match op {
-                        Op::LessThan => lhs < rhs,
-                        Op::GreaterThan => lhs > rhs,
-                        Op::LessOrEqual => lhs <= rhs,
-                        Op::GreatorOrEqual => lhs >= rhs,
+                        Op::LessThan => result == Ordering::Less,
+                        Op::GreaterThan => result == Ordering::Greater,
+                        Op::LessOrEqual => result != Ordering::Greater,
+                        Op::GreatorOrEqual => result != Ordering::Less,
                         _ => unreachable!(),
                     };