Skip to content

Commit 7e018f0

Browse files
committed
fix: suggestions from review
1 parent b709b7b commit 7e018f0

8 files changed

Lines changed: 251 additions & 13 deletions

File tree

libs/@local/hashql/mir/src/interpret/value/int.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ impl Display for Int {
322322
impl PartialEq for Int {
323323
#[inline]
324324
fn eq(&self, other: &Self) -> bool {
325-
self.size == other.size && self.as_int() == other.as_int()
325+
self.as_int() == other.as_int() && self.size == other.size
326326
}
327327
}
328328

@@ -338,9 +338,9 @@ impl PartialOrd for Int {
338338
impl Ord for Int {
339339
#[inline]
340340
fn cmp(&self, other: &Self) -> cmp::Ordering {
341-
self.size
342-
.cmp(&other.size)
343-
.then_with(|| self.as_int().cmp(&other.as_int()))
341+
self.as_int()
342+
.cmp(&other.as_int())
343+
.then_with(|| self.size.cmp(&other.size))
344344
}
345345
}
346346

libs/@local/hashql/mir/src/interpret/value/mod.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,10 @@ impl<A: Allocator> Ord for Value<'_, A> {
472472
(Value::Integer(this), Value::Integer(other)) => this.cmp(other),
473473
(Value::Number(this), Value::Number(other)) => this.cmp(other),
474474

475+
// Bool is a separate type from Int/Number in the subtype lattice, so
476+
// cross-type numeric comparison only applies to integers. Booleans fall
477+
// through to discriminant ordering (which is fine: no well-typed program
478+
// can observe the ordering between Bool and Number).
475479
(Value::Integer(this), Value::Number(other)) if !this.is_bool() => {
476480
other.cmp_int(this).reverse()
477481
}

libs/@local/hashql/mir/src/pass/transform/inst_simplify/mod.rs

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -322,20 +322,28 @@ impl<'heap, A: Allocator> InstSimplifyVisitor<'_, 'heap, A> {
322322
}
323323
(BinOp::BitOr, _) => None,
324324
// true == rhs => rhs (boolean equivalence)
325-
(BinOp::Eq, 1) if is_bool => Some(RValue::Load(Operand::Place(rhs))),
325+
(BinOp::Eq, 1) if is_bool && lhs.is_bool() => Some(RValue::Load(Operand::Place(rhs))),
326326
// false == rhs => !rhs == ~rhs (boolean equivalence)
327-
(BinOp::Eq, 0) if is_bool => Some(RValue::Unary(Unary {
327+
(BinOp::Eq, 0) if is_bool && lhs.is_bool() => Some(RValue::Unary(Unary {
328328
op: UnOp::BitNot,
329329
operand: Operand::Place(rhs),
330330
})),
331+
// bool == int => false
332+
(BinOp::Eq, _) if is_bool && !lhs.is_bool() => {
333+
Some(RValue::Load(Operand::Constant(Constant::Int(Int::FALSE))))
334+
}
331335
(BinOp::Eq, _) => None,
332336
// false != rhs => rhs (boolean equivalence)
333-
(BinOp::Ne, 0) if is_bool => Some(RValue::Load(Operand::Place(rhs))),
337+
(BinOp::Ne, 0) if is_bool && lhs.is_bool() => Some(RValue::Load(Operand::Place(rhs))),
334338
// true != rhs => !rhs == ~rhs (boolean equivalence)
335-
(BinOp::Ne, 1) if is_bool => Some(RValue::Unary(Unary {
339+
(BinOp::Ne, 1) if is_bool && lhs.is_bool() => Some(RValue::Unary(Unary {
336340
op: UnOp::BitNot,
337341
operand: Operand::Place(rhs),
338342
})),
343+
// bool != int => true
344+
(BinOp::Ne, _) if is_bool && !lhs.is_bool() => {
345+
Some(RValue::Load(Operand::Constant(Constant::Int(Int::TRUE))))
346+
}
339347
(BinOp::Ne, _) => None,
340348
(BinOp::Lt, _) => None,
341349
(BinOp::Lte, _) => None,
@@ -373,7 +381,7 @@ impl<'heap, A: Allocator> InstSimplifyVisitor<'_, 'heap, A> {
373381
(BinOp::BitAnd, 0) if is_bool => {
374382
Some(RValue::Load(Operand::Constant(Constant::Int(false.into()))))
375383
}
376-
// 0 & lhs => 0 (annihilator)
384+
// lhs & 0 => 0 (annihilator)
377385
(BinOp::BitAnd, 0) => Some(RValue::Load(Operand::Constant(Constant::Int(0.into())))),
378386
(BinOp::BitAnd, _) => None,
379387
// lhs | 0 => lhs (identity)
@@ -384,20 +392,28 @@ impl<'heap, A: Allocator> InstSimplifyVisitor<'_, 'heap, A> {
384392
}
385393
(BinOp::BitOr, _) => None,
386394
// lhs == true => lhs (boolean equivalence)
387-
(BinOp::Eq, 1) if is_bool => Some(RValue::Load(Operand::Place(lhs))),
395+
(BinOp::Eq, 1) if is_bool && rhs.is_bool() => Some(RValue::Load(Operand::Place(lhs))),
388396
// lhs == false => !lhs == ~lhs (boolean equivalence)
389-
(BinOp::Eq, 0) if is_bool => Some(RValue::Unary(Unary {
397+
(BinOp::Eq, 0) if is_bool && rhs.is_bool() => Some(RValue::Unary(Unary {
390398
op: UnOp::BitNot,
391399
operand: Operand::Place(lhs),
392400
})),
401+
// bool == int => false
402+
(BinOp::Eq, _) if is_bool && !rhs.is_bool() => {
403+
Some(RValue::Load(Operand::Constant(Constant::Int(Int::FALSE))))
404+
}
393405
(BinOp::Eq, _) => None,
394406
// lhs != false => lhs (boolean equivalence)
395-
(BinOp::Ne, 0) if is_bool => Some(RValue::Load(Operand::Place(lhs))),
407+
(BinOp::Ne, 0) if is_bool && rhs.is_bool() => Some(RValue::Load(Operand::Place(lhs))),
396408
// lhs != true => !lhs == ~lhs (boolean equivalence)
397-
(BinOp::Ne, 1) if is_bool => Some(RValue::Unary(Unary {
409+
(BinOp::Ne, 1) if is_bool && rhs.is_bool() => Some(RValue::Unary(Unary {
398410
op: UnOp::BitNot,
399411
operand: Operand::Place(lhs),
400412
})),
413+
// bool != int => true
414+
(BinOp::Ne, _) if is_bool && !rhs.is_bool() => {
415+
Some(RValue::Load(Operand::Constant(Constant::Int(Int::TRUE))))
416+
}
401417
(BinOp::Ne, _) => None,
402418
(BinOp::Lt, _) => None,
403419
(BinOp::Lte, _) => None,

libs/@local/hashql/mir/src/pass/transform/inst_simplify/tests.rs

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,124 @@ fn const_fold_eq_bool_vs_int() {
278278
);
279279
}
280280

281+
/// Regression: `bool_place == int(1)` must fold to `false`, not simplify to `bool_place`.
282+
///
283+
/// With structural typing, a Bool-typed place can be compared against an Int constant.
284+
/// The types have no intersection: booleans (size 1) can never equal integers (size 128).
285+
#[test]
286+
fn eq_bool_place_vs_int_constant_right() {
287+
let heap = Heap::new();
288+
let interner = Interner::new(&heap);
289+
let env = Environment::new(&heap);
290+
291+
let body = body!(interner, env; fn@0/1 -> Bool {
292+
decl x: Bool, result: Bool;
293+
294+
bb0() {
295+
result = bin.== x 1;
296+
return result;
297+
}
298+
});
299+
300+
assert_inst_simplify_pass(
301+
"eq_bool_place_vs_int_constant_right",
302+
body,
303+
&mut MirContext {
304+
heap: &heap,
305+
env: &env,
306+
interner: &interner,
307+
diagnostics: DiagnosticIssues::new(),
308+
},
309+
);
310+
}
311+
312+
/// Same as above but with the constant on the left: `1 == bool_place`.
313+
#[test]
314+
fn eq_bool_place_vs_int_constant_left() {
315+
let heap = Heap::new();
316+
let interner = Interner::new(&heap);
317+
let env = Environment::new(&heap);
318+
319+
let body = body!(interner, env; fn@0/1 -> Bool {
320+
decl x: Bool, result: Bool;
321+
322+
bb0() {
323+
result = bin.== 1 x;
324+
return result;
325+
}
326+
});
327+
328+
assert_inst_simplify_pass(
329+
"eq_bool_place_vs_int_constant_left",
330+
body,
331+
&mut MirContext {
332+
heap: &heap,
333+
env: &env,
334+
interner: &interner,
335+
diagnostics: DiagnosticIssues::new(),
336+
},
337+
);
338+
}
339+
340+
/// Regression: `bool_place != int(0)` must fold to `true`, not simplify to `bool_place`.
341+
///
342+
/// The Ne counterpart of the eq tests above. Bool and Int have no intersection,
343+
/// so they are always unequal.
344+
#[test]
345+
fn ne_bool_place_vs_int_constant_right() {
346+
let heap = Heap::new();
347+
let interner = Interner::new(&heap);
348+
let env = Environment::new(&heap);
349+
350+
let body = body!(interner, env; fn@0/1 -> Bool {
351+
decl x: Bool, result: Bool;
352+
353+
bb0() {
354+
result = bin.!= x 0;
355+
return result;
356+
}
357+
});
358+
359+
assert_inst_simplify_pass(
360+
"ne_bool_place_vs_int_constant_right",
361+
body,
362+
&mut MirContext {
363+
heap: &heap,
364+
env: &env,
365+
interner: &interner,
366+
diagnostics: DiagnosticIssues::new(),
367+
},
368+
);
369+
}
370+
371+
/// Same as above but with the constant on the left: `0 != bool_place`.
372+
#[test]
373+
fn ne_bool_place_vs_int_constant_left() {
374+
let heap = Heap::new();
375+
let interner = Interner::new(&heap);
376+
let env = Environment::new(&heap);
377+
378+
let body = body!(interner, env; fn@0/1 -> Bool {
379+
decl x: Bool, result: Bool;
380+
381+
bb0() {
382+
result = bin.!= 0 x;
383+
return result;
384+
}
385+
});
386+
387+
assert_inst_simplify_pass(
388+
"ne_bool_place_vs_int_constant_left",
389+
body,
390+
&mut MirContext {
391+
heap: &heap,
392+
env: &env,
393+
interner: &interner,
394+
diagnostics: DiagnosticIssues::new(),
395+
},
396+
);
397+
}
398+
281399
/// Tests constant folding for unary NOT.
282400
#[test]
283401
fn const_fold_unary_not() {

libs/@local/hashql/mir/tests/ui/pass/inst_simplify/eq_bool_place_vs_int_constant_left.snap

Lines changed: 25 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

libs/@local/hashql/mir/tests/ui/pass/inst_simplify/eq_bool_place_vs_int_constant_right.snap

Lines changed: 25 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

libs/@local/hashql/mir/tests/ui/pass/inst_simplify/ne_bool_place_vs_int_constant_left.snap

Lines changed: 25 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

libs/@local/hashql/mir/tests/ui/pass/inst_simplify/ne_bool_place_vs_int_constant_right.snap

Lines changed: 25 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)