BE-525: HashQL: Remove implicit type-widening from Integer operations, and make comparison size aware#8693
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
PR SummaryMedium Risk Overview Makes Reviewed by Cursor Bugbot for commit bb9d9da. Bugbot is set up for automated code reviews on this repo. Configure here. |
🤖 Augment PR SummarySummary: This PR fixes HashQL MIR interpreter integer semantics by removing implicit type-widening on overflow and making comparisons size-aware. Changes:
🤖 Was this summary useful? React with 👍 or 👎 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## bm/be-515-hashql-remove-offset-0-materialization-fence-from-generated #8693 +/- ##
======================================================================================================
Coverage 21.16% 21.16%
======================================================================================================
Files 544 544
Lines 26213 26213
Branches 2994 2994
======================================================================================================
Hits 5548 5548
Misses 20589 20589
Partials 76 76
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Merging this PR will not alter performance
Comparing Footnotes
|
07de63c to
b87ca3d
Compare
a0db929 to
58bba8c
Compare
58bba8c to
2a89970
Compare
b87ca3d to
1d64eff
Compare
1d64eff to
b63a08d
Compare
2a89970 to
3610d86
Compare
3610d86 to
1d27736
Compare
b63a08d to
0fcfe76
Compare
1d27736 to
f952099
Compare
0fcfe76 to
8e7b7a8
Compare
f952099 to
677b341
Compare
8e7b7a8 to
80c753b
Compare
80c753b to
bb9d9da
Compare
677b341 to
5b4aeb0
Compare

🌟 What is the purpose of this PR?
Integer arithmetic in the MIR interpreter previously silently promoted overflowing
i128operations to floating-point results via aNumericenum. This was semantically incorrect: the language specifies arbitrary-precision integers, and silently widening tof64loses precision and masks errors. This PR replaces that silent promotion with explicit overflow detection, surfacing a user-facingIntegerOverflowruntime error when addition, subtraction, or negation exceeds the 128-bit range supported by the interpreter.As a side effect, the
Inttype's equality, ordering, and hashing semantics are corrected so that booleans (1-bit integers) are treated as a distinct type from integers (128-bit), rather than comparing equal to numeric integers with the same value. This restores type-level correctness forBoolvsIntcomparisons throughout the value layer.🔍 What does this change?
RuntimeError::IntegerOverflowvariant with anoperationfield, and a correspondinginteger_overflowdiagnostic emitted under theRuntimeLimitcategory with a note that the interpreter supports up to 128-bit integers.Neg,Add<Int>, andSub<Int>operator impls onInt(which returnedNumericand silently promoted tof64on overflow) withchecked_neg,checked_add, andchecked_submethods that returnOption<Int>.Numericenum entirely, along with theFrom<Numeric> for Valueconversion and all associated forwarding impls.RuntimeError::IntegerOverflowonNone.Int'sPartialEq,Ord, andHashimplementations to include thesizefield, so thatInt::from(true)(1-bit) andInt::from(1_i32)(128-bit) are no longer considered equal.PartialEq<Int> for Num,PartialOrd<Int> for Num, and their reverses to returnfalse/Nonewhen theIntis a boolean, since booleans are not a subtype ofNumber.Value::Ordto guard the cross-typeInteger/Numberordering arms with!this.is_bool()/!other.is_bool().InstSimplifyVisitor::eval_un_opto returnOption<Int>and skip constant folding for negation ofi128::MIN.inst_simplifytest confirming thatneg(i128::MIN)is not folded.int.rsandmod.rsto reflect the corrected bool/int distinction.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
🛡 What tests cover this?
integer_add_overflow,integer_sub_overflow,integer_neg_overflow— each confirms aRuntimeLimitdiagnostic is produced.inst_simplifytest:const_fold_neg_overflow— confirms that negation ofi128::MINis left unfolded.IntandValuecovering corrected equality, ordering, hashing, and cross-typeNum/Intcomparisons.❓ How to test this?
cargo test -p hashql-mirand confirm all tests pass.const_fold_neg_overflow.snapmatches the expected unfold output.RuntimeLimitand a message referencing the 128-bit limit.