diff --git a/libs/@local/hashql/mir/src/interpret/error.rs b/libs/@local/hashql/mir/src/interpret/error.rs index e6ac8eeecf8..10fe9e2c79b 100644 --- a/libs/@local/hashql/mir/src/interpret/error.rs +++ b/libs/@local/hashql/mir/src/interpret/error.rs @@ -351,6 +351,17 @@ pub enum RuntimeError<'heap, E, A: Allocator> { limit: usize, }, + /// Integer arithmetic overflowed the supported range. + /// + /// This is a user-facing error caused by an implementation limitation: + /// the interpreter uses 128-bit integers, but the language specifies + /// arbitrary-precision integers. Operations that produce results outside + /// the `i128` range fail here. + IntegerOverflow { + /// Description of the operation that overflowed (e.g., "addition"). + operation: &'static str, + }, + /// Value has the wrong runtime type. /// /// This is an ICE: type checking should ensure values have the @@ -423,6 +434,7 @@ impl RuntimeError<'_, E, A> { Self::OutOfRange { length, index } => out_of_range(span, length, index), Self::InputNotFound { name } => input_not_found(span, name), Self::RecursionLimitExceeded { limit } => recursion_limit_exceeded(span, limit), + Self::IntegerOverflow { operation } => integer_overflow(span, operation), Self::UnexpectedValueType { expected, actual } => { unexpected_value_type(span, &expected, &actual) } @@ -474,6 +486,7 @@ impl<'heap, A: Allocator> RuntimeError<'heap, !, A> { Self::RecursionLimitExceeded { limit } => { RuntimeError::RecursionLimitExceeded { limit } } + Self::IntegerOverflow { operation } => RuntimeError::IntegerOverflow { operation }, Self::UnexpectedValueType { expected, actual } => { RuntimeError::UnexpectedValueType { expected, actual } } @@ -808,3 +821,19 @@ fn recursion_limit_exceeded(span: SpanId, limit: usize) -> InterpretDiagnostic { diagnostic } + +fn integer_overflow(span: SpanId, operation: &str) -> InterpretDiagnostic { + let mut diagnostic = + Diagnostic::new(InterpretDiagnosticCategory::RuntimeLimit, Severity::Error).primary( + Label::new( + span, + format!("integer {operation} produced a result outside the supported range"), + ), + ); + + diagnostic.add_message(Message::note( + "the interpreter currently supports integers up to 128 bits", + )); + + diagnostic +} diff --git a/libs/@local/hashql/mir/src/interpret/runtime.rs b/libs/@local/hashql/mir/src/interpret/runtime.rs index 4b06a2e85e7..8d827100982 100644 --- a/libs/@local/hashql/mir/src/interpret/runtime.rs +++ b/libs/@local/hashql/mir/src/interpret/runtime.rs @@ -513,10 +513,15 @@ impl<'ctx, 'heap, A: Allocator + Clone> Runtime<'ctx, 'heap, A> { match op { BinOp::Add => match (lhs.as_ref(), rhs.as_ref()) { - (Value::Integer(lhs), Value::Integer(rhs)) => Ok(Value::from(lhs + rhs)), - (Value::Integer(lhs), Value::Number(rhs)) => Ok(Value::Number(lhs + rhs)), - (Value::Number(lhs), Value::Integer(rhs)) => Ok(Value::Number(lhs + rhs)), - (Value::Number(lhs), Value::Number(rhs)) => Ok(Value::Number(lhs + rhs)), + (Value::Integer(lhs), Value::Integer(rhs)) => lhs + .checked_add(*rhs) + .map(Value::Integer) + .ok_or(RuntimeError::IntegerOverflow { + operation: "addition", + }), + (Value::Integer(lhs), Value::Number(rhs)) => Ok(Value::Number(*lhs + *rhs)), + (Value::Number(lhs), Value::Integer(rhs)) => Ok(Value::Number(*lhs + *rhs)), + (Value::Number(lhs), Value::Number(rhs)) => Ok(Value::Number(*lhs + *rhs)), _ => { cold_path(); @@ -532,10 +537,15 @@ impl<'ctx, 'heap, A: Allocator + Clone> Runtime<'ctx, 'heap, A> { } }, BinOp::Sub => match (lhs.as_ref(), rhs.as_ref()) { - (Value::Integer(lhs), Value::Integer(rhs)) => Ok(Value::from(lhs - rhs)), - (Value::Integer(lhs), Value::Number(rhs)) => Ok(Value::Number(lhs - rhs)), - (Value::Number(lhs), Value::Integer(rhs)) => Ok(Value::Number(lhs - rhs)), - (Value::Number(lhs), Value::Number(rhs)) => Ok(Value::Number(lhs - rhs)), + (Value::Integer(lhs), Value::Integer(rhs)) => lhs + .checked_sub(*rhs) + .map(Value::Integer) + .ok_or(RuntimeError::IntegerOverflow { + operation: "subtraction", + }), + (Value::Integer(lhs), Value::Number(rhs)) => Ok(Value::Number(*lhs - *rhs)), + (Value::Number(lhs), Value::Integer(rhs)) => Ok(Value::Number(*lhs - *rhs)), + (Value::Number(lhs), Value::Number(rhs)) => Ok(Value::Number(*lhs - *rhs)), _ => { cold_path(); @@ -623,7 +633,13 @@ impl<'ctx, 'heap, A: Allocator + Clone> Runtime<'ctx, 'heap, A> { } }, UnOp::Neg => match operand.as_ref() { - Value::Integer(int) => Ok((-int).into()), + Value::Integer(int) => { + int.checked_neg() + .map(Value::Integer) + .ok_or(RuntimeError::IntegerOverflow { + operation: "negation", + }) + } Value::Number(number) => Ok(Value::Number(-number)), Value::Unit | Value::String(_) diff --git a/libs/@local/hashql/mir/src/interpret/tests.rs b/libs/@local/hashql/mir/src/interpret/tests.rs index a6bbcfe2768..a14cb7d88c1 100644 --- a/libs/@local/hashql/mir/src/interpret/tests.rs +++ b/libs/@local/hashql/mir/src/interpret/tests.rs @@ -1191,6 +1191,63 @@ fn recursion_limit_exceeded() { assert_eq!(result.category, InterpretDiagnosticCategory::RuntimeLimit); } +#[test] +fn integer_add_overflow() { + let heap = Heap::new(); + let interner = Interner::new(&heap); + let env = Environment::new(&heap); + + let body = body!(interner, env; fn@0/0 -> Int { + decl result: Int; + + bb0() { + result = bin.+ 170141183460469231731687303715884105727 1; + return result; + } + }); + + let result = run_body(body).expect_err("should fail with integer overflow"); + assert_eq!(result.category, InterpretDiagnosticCategory::RuntimeLimit); +} + +#[test] +fn integer_sub_overflow() { + let heap = Heap::new(); + let interner = Interner::new(&heap); + let env = Environment::new(&heap); + + let body = body!(interner, env; fn@0/0 -> Int { + decl result: Int; + + bb0() { + result = bin.- (-0x8000_0000_0000_0000_0000_0000_0000_0000) 1; + return result; + } + }); + + let result = run_body(body).expect_err("should fail with integer overflow"); + assert_eq!(result.category, InterpretDiagnosticCategory::RuntimeLimit); +} + +#[test] +fn integer_neg_overflow() { + let heap = Heap::new(); + let interner = Interner::new(&heap); + let env = Environment::new(&heap); + + let body = body!(interner, env; fn@0/0 -> Int { + decl result: Int; + + bb0() { + result = un.neg (-0x8000_0000_0000_0000_0000_0000_0000_0000); + return result; + } + }); + + let result = run_body(body).expect_err("should fail with integer overflow"); + assert_eq!(result.category, InterpretDiagnosticCategory::RuntimeLimit); +} + #[test] fn out_of_range_list_index() { let heap = Heap::new(); diff --git a/libs/@local/hashql/mir/src/interpret/value/int.rs b/libs/@local/hashql/mir/src/interpret/value/int.rs index 0e7a47565b7..5e84e98be30 100644 --- a/libs/@local/hashql/mir/src/interpret/value/int.rs +++ b/libs/@local/hashql/mir/src/interpret/value/int.rs @@ -23,15 +23,14 @@ use core::{ error::Error, fmt::{self, Display}, hash::{Hash, Hasher}, - hint, num::{NonZero, TryFromIntError}, - ops::{Add, BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Neg, Not, Sub}, + ops::{Add, BitAnd, BitAndAssign, BitOr, BitOrAssign, BitXor, BitXorAssign, Not, Sub}, }; use hashql_core::value::{Integer, Primitive}; use crate::{ - interpret::value::{Num, Numeric}, + interpret::value::Num, macros::{forward_ref_binop, forward_ref_op_assign, forward_ref_unop}, }; @@ -61,8 +60,8 @@ const INT_BITS: NonZero = NonZero::new(128).unwrap(); /// assert_eq!(n.size(), 128); /// assert_eq!(n.as_int(), 42); /// -/// // Bool provenance is metadata, not identity: from(true) == from(1) -/// assert_eq!(Int::from(true), Int::from(1_i32)); +/// // Booleans and integers are distinct types +/// assert_ne!(Int::from(true), Int::from(1_i32)); /// ``` // Uses `#[repr(packed)]` to avoid alignment padding, which would duplicate size, same as // rust-lang's ScalarInt. @@ -254,6 +253,19 @@ impl Int { } } + /// Checked integer negation. Returns `None` on overflow. + /// + /// Only `i128::MIN` overflows, since its negation exceeds `i128::MAX`. + /// Always produces a 128-bit result (arithmetic promotes booleans). + #[inline] + #[must_use] + pub const fn checked_neg(self) -> Option { + match self.as_int().checked_neg() { + Some(result) => Some(Self::from_i128(result)), + None => None, + } + } + /// Converts this integer to [`f32`]. /// /// This may lose precision for values that cannot be exactly represented @@ -310,7 +322,7 @@ impl Display for Int { impl PartialEq for Int { #[inline] fn eq(&self, other: &Self) -> bool { - self.as_int() == other.as_int() + self.size == other.size && self.as_int() == other.as_int() } } @@ -326,13 +338,16 @@ impl PartialOrd for Int { impl Ord for Int { #[inline] fn cmp(&self, other: &Self) -> cmp::Ordering { - self.as_int().cmp(&other.as_int()) + self.size + .cmp(&other.size) + .then_with(|| self.as_int().cmp(&other.as_int())) } } impl Hash for Int { #[inline] fn hash(&self, state: &mut H) { + self.size.hash(state); self.as_int().hash(state); } } @@ -499,39 +514,6 @@ impl Not for Int { } } -impl Neg for Int { - type Output = Numeric; - - #[expect(clippy::cast_precision_loss, clippy::float_arithmetic)] - fn neg(self) -> Self::Output { - let value = self.as_int(); - let (result, overflow) = value.overflowing_neg(); - - if hint::unlikely(overflow) { - // Only i128::MIN overflows: return i128::MAX + 1 as float. - Numeric::Num(Num::from((i128::MAX as f64) + 1.0)) - } else { - Numeric::Int(Self::from_i128(result)) - } - } -} - -impl Add for Int { - type Output = Numeric; - - #[expect(clippy::float_arithmetic)] - fn add(self, rhs: Self) -> Self::Output { - let (lhs, rhs_val) = (self.as_int(), rhs.as_int()); - let (result, overflow) = lhs.overflowing_add(rhs_val); - - if hint::unlikely(overflow) { - Numeric::Num(Num::from(self.as_f64() + rhs.as_f64())) - } else { - Numeric::Int(Self::from_i128(result)) - } - } -} - impl Add for Int { type Output = Num; @@ -542,22 +524,6 @@ impl Add for Int { } } -impl Sub for Int { - type Output = Numeric; - - #[expect(clippy::float_arithmetic)] - fn sub(self, rhs: Self) -> Self::Output { - let (lhs, rhs_val) = (self.as_int(), rhs.as_int()); - let (result, overflow) = lhs.overflowing_sub(rhs_val); - - if hint::unlikely(overflow) { - Numeric::Num(Num::from(self.as_f64() - rhs.as_f64())) - } else { - Numeric::Int(Self::from_i128(result)) - } - } -} - impl Sub for Int { type Output = Num; @@ -636,10 +602,7 @@ impl BitXorAssign for Int { } forward_ref_unop!(impl Not::not for Int); -forward_ref_unop!(impl Neg::neg for Int); -forward_ref_binop!(impl Add::add for Int); forward_ref_binop!(impl Add::add for Int); -forward_ref_binop!(impl Sub::sub for Int); forward_ref_binop!(impl Sub::sub for Int); forward_ref_binop!(impl BitOr::bitor for Int); forward_ref_binop!(impl BitAnd::bitand for Int); @@ -658,7 +621,7 @@ mod tests { use core::hash::BuildHasher as _; - use crate::interpret::value::{Int, Numeric}; + use crate::interpret::value::Int; #[test] fn layout() { @@ -682,12 +645,12 @@ mod tests { } #[test] - fn bool_int_equality_is_numeric() { - // Bool provenance (size) does not affect equality — only the numeric value matters. - // The type checker prevents comparing bools with ints; at the value level, - // same numeric content means same value. - assert_eq!(Int::from(true), Int::from(1_i32)); - assert_eq!(Int::from(false), Int::from(0_i32)); + fn bool_and_int_are_distinct_types() { + // Booleans and integers are distinct types in HashQL. Even when they + // carry the same numeric value, they compare unequal because the size + // (1 bit vs 128 bits) encodes the type distinction. + assert_ne!(Int::from(true), Int::from(1_i32)); + assert_ne!(Int::from(false), Int::from(0_i32)); } #[test] @@ -729,22 +692,26 @@ mod tests { } #[test] - fn equality_is_numeric() { + fn equality_respects_size() { assert_eq!(Int::from(true), Int::from(true)); assert_eq!(Int::from(42_i64), Int::from(42_i64)); - assert_eq!(Int::from(true), Int::from(1_i64)); - assert_eq!(Int::from(false), Int::from(0_i64)); + + // Same numeric value, different size: not equal. + assert_ne!(Int::from(true), Int::from(1_i64)); + assert_ne!(Int::from(false), Int::from(0_i64)); } #[test] - fn ordering_is_numeric() { - // Ordering is purely by numeric value, size is not considered. - assert_eq!( - Int::from(true).cmp(&Int::from(1_i32)), - core::cmp::Ordering::Equal - ); - assert!(Int::from(false) < Int::from(1_i32)); - assert!(Int::from(true) > Int::from(0_i32)); + fn ordering_separates_bools_from_ints() { + use core::cmp::Ordering; + + // Bools (size=1) sort before ints (size=128). + assert_eq!(Int::from(true).cmp(&Int::from(1_i32)), Ordering::Less); + assert_eq!(Int::from(false).cmp(&Int::from(0_i32)), Ordering::Less); + + // Within the same size, ordering is by value. + assert!(Int::from(false) < Int::from(true)); + assert!(Int::from(0_i32) < Int::from(1_i32)); } #[test] @@ -762,55 +729,66 @@ mod tests { } #[test] - fn add_ints() { - let result = Int::from(2_i64) + Int::from(3_i64); - assert!(matches!(result, Numeric::Int(int) if int.as_int() == 5 && int.size() == 128)); + fn checked_add_success() { + let result = Int::from(2_i64).checked_add(Int::from(3_i64)); + let int = result.expect("should not overflow"); + assert_eq!(int.as_int(), 5); + assert_eq!(int.size(), 128); } #[test] - fn add_bools_promotes() { - let result = Int::from(true) + Int::from(true); - assert!(matches!(result, Numeric::Int(int) if int.as_int() == 2 && int.size() == 128)); + fn checked_add_bools_promotes() { + let result = Int::from(true).checked_add(Int::from(true)); + let int = result.expect("should not overflow"); + assert_eq!(int.as_int(), 2); + assert_eq!(int.size(), 128); } #[test] - fn sub_ints() { - let result = Int::from(5_i64) - Int::from(3_i64); - assert!(matches!(result, Numeric::Int(int) if int.as_int() == 2 && int.size() == 128)); + fn checked_add_overflow_returns_none() { + assert_eq!(Int::from(i128::MAX).checked_add(Int::from(1_i32)), None); } #[test] - fn neg_positive() { - let result = -Int::from(42_i64); - assert!(matches!(result, Numeric::Int(int) if int.as_int() == -42)); + fn checked_sub_success() { + let result = Int::from(5_i64).checked_sub(Int::from(3_i64)); + let int = result.expect("should not overflow"); + assert_eq!(int.as_int(), 2); + assert_eq!(int.size(), 128); } #[test] - fn neg_negative() { - let result = -Int::from(-100_i64); - assert!(matches!(result, Numeric::Int(int) if int.as_int() == 100)); + fn checked_sub_overflow_returns_none() { + assert_eq!(Int::from(i128::MIN).checked_sub(Int::from(1_i32)), None); } #[test] - fn neg_zero() { - let result = -Int::from(0_i64); - assert!(matches!(result, Numeric::Int(int) if int.as_int() == 0)); + fn checked_neg_positive() { + let result = Int::from(42_i64).checked_neg(); + assert_eq!(result.expect("should not overflow").as_int(), -42); } #[test] - fn neg_i128_max() { - let result = -Int::from(i128::MAX); - assert!(matches!(result, Numeric::Int(int) if int.as_int() == -i128::MAX)); + fn checked_neg_negative() { + let result = Int::from(-100_i64).checked_neg(); + assert_eq!(result.expect("should not overflow").as_int(), 100); } #[test] - fn neg_i128_min_overflows_to_float() { - let result = -Int::from(i128::MIN); - let Numeric::Num(num) = result else { - panic!("expected Numeric::Num for -i128::MIN, got {result:?}"); - }; - let expected = -(i128::MIN as f64); - assert_eq!(num.as_f64(), expected); + fn checked_neg_zero() { + let result = Int::from(0_i64).checked_neg(); + assert_eq!(result.expect("should not overflow").as_int(), 0); + } + + #[test] + fn checked_neg_i128_max() { + let result = Int::from(i128::MAX).checked_neg(); + assert_eq!(result.expect("should not overflow").as_int(), -i128::MAX); + } + + #[test] + fn checked_neg_i128_min_overflows() { + assert_eq!(Int::from(i128::MIN).checked_neg(), None); } #[test] @@ -848,24 +826,29 @@ mod tests { } #[test] - fn eq_ord_transitivity_with_num() { - use core::cmp::Ordering; - + fn num_int_cross_type_equality() { use crate::interpret::value::Num; - let bool_one = Int::from(true); - let int_one = Int::from(1_i32); - let num_one = Num::from(1.0); + // Integer and Number share a numeric domain (Integer <: Number). + assert_eq!(Int::from(1_i32), Num::from(1.0)); + assert_eq!(Num::from(42.0), Int::from(42_i32)); - // Transitivity: bool_one == num_one, num_one == int_one, therefore bool_one == int_one - assert_eq!(bool_one, num_one); - assert_eq!(num_one, int_one); - assert_eq!(bool_one, int_one); + // Booleans are not a subtype of Number, so they never equal a Num. + assert_ne!(Int::from(true), Num::from(1.0)); + assert_ne!(Num::from(0.0), Int::from(false)); + } - // Ord consistency - assert_eq!(num_one.cmp_int(&bool_one), Ordering::Equal); - assert_eq!(num_one.cmp_int(&int_one), Ordering::Equal); - assert_eq!(bool_one.cmp(&int_one), Ordering::Equal); + #[test] + fn num_int_cross_type_ordering() { + use crate::interpret::value::Num; + + // Integer and Number are orderable. + assert!(Int::from(1_i32) < Num::from(1.5)); + assert!(Num::from(2.5) < Int::from(3_i32)); + + // Booleans and Numbers are not orderable. + assert_eq!(Int::from(true).partial_cmp(&Num::from(1.0)), None); + assert_eq!(Num::from(0.0).partial_cmp(&Int::from(false)), None); } #[test] @@ -874,14 +857,21 @@ mod tests { let build = FastHasher::default(); - // Equal values must have equal hashes + // Equal values must have equal hashes. assert_eq!( build.hash_one(Int::from(true)), - build.hash_one(Int::from(1_i32)) + build.hash_one(Int::from(true)) ); assert_eq!( - build.hash_one(Int::from(false)), - build.hash_one(Int::from(0_i32)) + build.hash_one(Int::from(42_i32)), + build.hash_one(Int::from(42_i64)) + ); + + // Different types must have different hashes (not guaranteed but + // extremely likely for a good hash function). + assert_ne!( + build.hash_one(Int::from(true)), + build.hash_one(Int::from(1_i32)) ); } diff --git a/libs/@local/hashql/mir/src/interpret/value/mod.rs b/libs/@local/hashql/mir/src/interpret/value/mod.rs index 8f9f94781d4..a97f4ca3948 100644 --- a/libs/@local/hashql/mir/src/interpret/value/mod.rs +++ b/libs/@local/hashql/mir/src/interpret/value/mod.rs @@ -51,7 +51,7 @@ pub use self::{ dict::Dict, int::{Int, TryFromIntegerError, TryFromPrimitiveError}, list::List, - num::{Num, Numeric}, + num::Num, opaque::Opaque, ptr::Ptr, str::Str, @@ -431,16 +431,6 @@ impl<'heap, A: Allocator> From<&Constant<'heap>> for Value<'heap, A> { } } -impl From for Value<'_, A> { - #[inline] - fn from(value: Numeric) -> Self { - match value { - Numeric::Int(int) => Self::Integer(int), - Numeric::Num(num) => Self::Number(num), - } - } -} - impl PartialEq for Value<'_, A> { #[inline] fn eq(&self, other: &Self) -> bool { @@ -482,8 +472,10 @@ impl Ord for Value<'_, A> { (Value::Integer(this), Value::Integer(other)) => this.cmp(other), (Value::Number(this), Value::Number(other)) => this.cmp(other), - (Value::Integer(this), Value::Number(other)) => other.cmp_int(this).reverse(), - (Value::Number(this), Value::Integer(other)) => this.cmp_int(other), + (Value::Integer(this), Value::Number(other)) if !this.is_bool() => { + other.cmp_int(this).reverse() + } + (Value::Number(this), Value::Integer(other)) if !other.is_bool() => this.cmp_int(other), (Value::String(this), Value::String(other)) => this.cmp(other), (Value::Pointer(this), Value::Pointer(other)) => this.cmp(other), @@ -560,33 +552,40 @@ impl<'value, 'heap, A: Allocator> From<&'value Value<'heap, A>> #[cfg(test)] mod tests { - #![expect(clippy::min_ident_chars)] use core::cmp::Ordering; use super::{Int, Num, Value}; #[test] - fn value_eq_transitivity_bool_num_int() { - // Regression test: Value::Eq must be transitive across Integer/Number variants. - // Previously, Integer(from(true)) == Number(1.0) and Number(1.0) == Integer(from(1)), - // but Integer(from(true)) != Integer(from(1)) — violating transitivity. - let a: Value = Value::Integer(Int::from(true)); - let b: Value = Value::Number(Num::from(1.0)); - let c: Value = Value::Integer(Int::from(1_i32)); - - assert_eq!(a, b, "bool-int == num"); - assert_eq!(b, c, "num == int"); - assert_eq!(a, c, "bool-int == int (transitivity)"); + fn value_eq_bool_num_int_are_distinct() { + let bool_val: Value = Value::Integer(Int::from(true)); + let num_val: Value = Value::Number(Num::from(1.0)); + let int_val: Value = Value::Integer(Int::from(1_i32)); + + // Number and Integer share a numeric domain. + assert_eq!(num_val, int_val); + + // Boolean is a distinct type from both. + assert_ne!(bool_val, num_val); + assert_ne!(bool_val, int_val); } #[test] - fn value_ord_transitivity_bool_num_int() { - let a: Value = Value::Integer(Int::from(true)); - let b: Value = Value::Number(Num::from(1.0)); - let c: Value = Value::Integer(Int::from(1_i32)); - - assert_eq!(a.cmp(&b), Ordering::Equal); - assert_eq!(b.cmp(&c), Ordering::Equal); - assert_eq!(a.cmp(&c), Ordering::Equal); + fn value_ord_bool_num_int_consistent() { + let bool_val: Value = Value::Integer(Int::from(true)); + let num_val: Value = Value::Number(Num::from(1.0)); + let int_val: Value = Value::Integer(Int::from(1_i32)); + + // Number(1.0) == Integer(1) in ordering. + assert_eq!(num_val.cmp(&int_val), Ordering::Equal); + + // Bool sorts before Number (discriminant: Integer < Number). + assert_eq!(bool_val.cmp(&num_val), Ordering::Less); + + // Bool sorts before Int (size: 1 < 128). + assert_eq!(bool_val.cmp(&int_val), Ordering::Less); + + // Transitivity: bool < num, num == int, therefore bool < int. + assert!(bool_val < int_val); } } diff --git a/libs/@local/hashql/mir/src/interpret/value/num.rs b/libs/@local/hashql/mir/src/interpret/value/num.rs index 5dec409c395..5ec17d7a585 100644 --- a/libs/@local/hashql/mir/src/interpret/value/num.rs +++ b/libs/@local/hashql/mir/src/interpret/value/num.rs @@ -11,20 +11,6 @@ use hashql_core::value::Float; use super::Int; use crate::macros::{forward_ref_binop, forward_ref_unop}; -/// A numeric value, either a floating-point number or an integer. -/// -/// This is a subset of [`Value`] that encompasses all numeric variants, providing -/// a unified type for operations that accept any numeric value. -/// -/// [`Value`]: super::Value -#[derive(Debug, Copy, Clone)] -pub enum Numeric { - /// A floating-point number with total ordering semantics. - Num(Num), - /// An arbitrary-precision integer. - Int(Int), -} - /// A floating-point number value with total ordering semantics. /// /// Wraps an [`f64`] and implements [`Ord`] using [`f64::total_cmp`], which follows @@ -136,14 +122,16 @@ impl PartialEq for Num { impl PartialEq for Num { #[inline] fn eq(&self, other: &Int) -> bool { - self.cmp_int(other) == cmp::Ordering::Equal + // Booleans and floating-point numbers are distinct types. + !other.is_bool() && self.cmp_int(other) == cmp::Ordering::Equal } } impl PartialEq for Int { #[inline] fn eq(&self, other: &Num) -> bool { - other.cmp_int(self) == cmp::Ordering::Equal + // Booleans and floating-point numbers are distinct types. + !self.is_bool() && other.cmp_int(self) == cmp::Ordering::Equal } } @@ -159,6 +147,10 @@ impl PartialOrd for Num { impl PartialOrd for Num { #[inline] fn partial_cmp(&self, other: &Int) -> Option { + // Booleans and floating-point numbers are not orderable. + if other.is_bool() { + return None; + } Some(self.cmp_int(other)) } } @@ -166,6 +158,10 @@ impl PartialOrd for Num { impl PartialOrd for Int { #[inline] fn partial_cmp(&self, other: &Num) -> Option { + // Booleans and floating-point numbers are not orderable. + if self.is_bool() { + return None; + } Some(other.cmp_int(self).reverse()) } } diff --git a/libs/@local/hashql/mir/src/lib.rs b/libs/@local/hashql/mir/src/lib.rs index ee755063446..aecce9b1e49 100644 --- a/libs/@local/hashql/mir/src/lib.rs +++ b/libs/@local/hashql/mir/src/lib.rs @@ -21,7 +21,6 @@ get_mut_unchecked, iter_array_chunks, iter_collect_into, - likely_unlikely, maybe_uninit_fill, maybe_uninit_uninit_array_transpose, option_into_flat_iter, diff --git a/libs/@local/hashql/mir/src/pass/transform/inst_simplify/mod.rs b/libs/@local/hashql/mir/src/pass/transform/inst_simplify/mod.rs index 2fc24b17e1c..9a00cd75261 100644 --- a/libs/@local/hashql/mir/src/pass/transform/inst_simplify/mod.rs +++ b/libs/@local/hashql/mir/src/pass/transform/inst_simplify/mod.rs @@ -258,8 +258,8 @@ impl<'heap, A: Allocator> InstSimplifyVisitor<'_, 'heap, A> { BinOp::BitAnd => return Some(lhs & rhs), BinOp::BitOr => return Some(lhs | rhs), // Comparisons produce booleans - BinOp::Eq => lhs.as_int() == rhs.as_int(), - BinOp::Ne => lhs.as_int() != rhs.as_int(), + BinOp::Eq => lhs == rhs, + BinOp::Ne => lhs != rhs, BinOp::Lt => lhs.as_int() < rhs.as_int(), BinOp::Lte => lhs.as_int() <= rhs.as_int(), BinOp::Gt => lhs.as_int() > rhs.as_int(), @@ -270,10 +270,12 @@ impl<'heap, A: Allocator> InstSimplifyVisitor<'_, 'heap, A> { } /// Evaluates a unary operation on a constant integer. - fn eval_un_op(op: UnOp, operand: Int) -> Int { + /// + /// Returns `None` if the operation overflows (only possible for negation of `i128::MIN`). + fn eval_un_op(op: UnOp, operand: Int) -> Option { match op { - UnOp::BitNot => !operand, - UnOp::Neg => Int::from(-operand.as_int()), + UnOp::BitNot => Some(!operand), + UnOp::Neg => operand.checked_neg(), } } @@ -510,8 +512,9 @@ impl<'heap, A: Allocator> VisitorMut<'heap> for InstSimplifyVisitor<'_, 'heap, A _: Location, Unary { op, operand }: &mut Unary<'heap>, ) -> Self::Result<()> { - if let OperandKind::Int(value) = self.try_eval(*operand) { - let result = Self::eval_un_op(*op, value); + if let OperandKind::Int(value) = self.try_eval(*operand) + && let Some(result) = Self::eval_un_op(*op, value) + { self.trampoline = Some(RValue::Load(Operand::Constant(Constant::Int(result)))); } diff --git a/libs/@local/hashql/mir/src/pass/transform/inst_simplify/tests.rs b/libs/@local/hashql/mir/src/pass/transform/inst_simplify/tests.rs index 9fd06bdb62e..cf8fcacb013 100644 --- a/libs/@local/hashql/mir/src/pass/transform/inst_simplify/tests.rs +++ b/libs/@local/hashql/mir/src/pass/transform/inst_simplify/tests.rs @@ -246,6 +246,38 @@ fn const_fold_bit_or() { ); } +/// Tests that `Eq` uses size-aware comparison when folding constants. +/// +/// `true` and `1` have the same numeric value but different sizes (1 bit vs +/// 128 bits), so they must compare as not equal. If `Eq` used `as_int()` +/// instead of the size-aware `==`, this would incorrectly fold to `true`. +#[test] +fn const_fold_eq_bool_vs_int() { + let heap = Heap::new(); + let interner = Interner::new(&heap); + let env = Environment::new(&heap); + + let body = body!(interner, env; fn@0/0 -> Bool { + decl result: Bool; + + bb0() { + result = bin.== true 1; + return result; + } + }); + + assert_inst_simplify_pass( + "const_fold_eq_bool_vs_int", + body, + &mut MirContext { + heap: &heap, + env: &env, + interner: &interner, + diagnostics: DiagnosticIssues::new(), + }, + ); +} + /// Tests constant folding for unary NOT. #[test] fn const_fold_unary_not() { @@ -302,6 +334,34 @@ fn const_fold_unary_neg() { ); } +/// Tests that negation of `i128::MIN` is not folded (returns `None` from `checked_neg`). +#[test] +fn const_fold_neg_overflow() { + let heap = Heap::new(); + let interner = Interner::new(&heap); + let env = Environment::new(&heap); + + let body = body!(interner, env; fn@0/0 -> Int { + decl result: Int; + + bb0() { + result = un.neg (-0x8000_0000_0000_0000_0000_0000_0000_0000); + return result; + } + }); + + assert_inst_simplify_pass( + "const_fold_neg_overflow", + body, + &mut MirContext { + heap: &heap, + env: &env, + interner: &interner, + diagnostics: DiagnosticIssues::new(), + }, + ); +} + /// Tests identity simplification for addition with zero on the right (x + 0 => x). #[test] fn identity_add_zero() { diff --git a/libs/@local/hashql/mir/tests/ui/pass/inst_simplify/const_fold_eq_bool_vs_int.snap b/libs/@local/hashql/mir/tests/ui/pass/inst_simplify/const_fold_eq_bool_vs_int.snap new file mode 100644 index 00000000000..aebb9a97eac --- /dev/null +++ b/libs/@local/hashql/mir/tests/ui/pass/inst_simplify/const_fold_eq_bool_vs_int.snap @@ -0,0 +1,26 @@ +--- +source: libs/@local/hashql/mir/src/pass/transform/inst_simplify/tests.rs +assertion_line: 78 +expression: value +--- +fn {closure@4294967040}() -> Boolean { + let %0: Boolean + + bb0(): { + %0 = true == 1 + + return %0 + } +} + +================== Changed: Yes ================== + +fn {closure@4294967040}() -> Boolean { + let %0: Boolean + + bb0(): { + %0 = false + + return %0 + } +} diff --git a/libs/@local/hashql/mir/tests/ui/pass/inst_simplify/const_fold_neg_overflow.snap b/libs/@local/hashql/mir/tests/ui/pass/inst_simplify/const_fold_neg_overflow.snap new file mode 100644 index 00000000000..c56b2bde1bb --- /dev/null +++ b/libs/@local/hashql/mir/tests/ui/pass/inst_simplify/const_fold_neg_overflow.snap @@ -0,0 +1,26 @@ +--- +source: libs/@local/hashql/mir/src/pass/transform/inst_simplify/tests.rs +assertion_line: 78 +expression: value +--- +fn {closure@4294967040}() -> Integer { + let %0: Integer + + bb0(): { + %0 = --170141183460469231731687303715884105728 + + return %0 + } +} + +================== Changed: No =================== + +fn {closure@4294967040}() -> Integer { + let %0: Integer + + bb0(): { + %0 = --170141183460469231731687303715884105728 + + return %0 + } +} diff --git a/libs/@local/hashql/mir/tests/ui/pass/pre_inline/chain-simplification.stdout b/libs/@local/hashql/mir/tests/ui/pass/pre_inline/chain-simplification.stdout index 13f25b39434..5fb4ccb993a 100644 --- a/libs/@local/hashql/mir/tests/ui/pass/pre_inline/chain-simplification.stdout +++ b/libs/@local/hashql/mir/tests/ui/pass/pre_inline/chain-simplification.stdout @@ -54,7 +54,7 @@ } bb11(): { - goto -> bb12(false) + goto -> bb12(0) } bb12(%0): {