feat(arxjit): lower comparisons and boolean operators to astx

[Jaskirat-s7]

Lowers the comparison and logical half of the expression layer, plus the type inference both need. Continues wiki Issue 5 after #109.

What lowers now

== != < <= > >=, and and / or, including chained comparisons and n-ary boolean expressions.

Design points for review

1. A comparison lowers to astx.BinaryOp, not astx.CompareOp.

CompareOp is the node that looks right, but IRx's visitor for it is _not_implemented (irx/base/visitors/base.py), and there is no CompareOp codegen in the builder or handlers. It would pass this stage and fail in codegen — the failure mode the unary-minus comment in this file already warns about. The six comparison op codes are exactly what irx.analysis.typing.binary_result_type resolves to Boolean, so that is what is emitted. test_the_comparison_tables_agree_with_irx reads IRx directly so the tables cannot drift.

2. Operands cannot be lowered at the expected type.

Everywhere else an operand takes the width its context declares. That breaks down at a comparison: the expected type there is the bool the comparison yields, so a < 3 would try to build 3 as a bool and refuse a correct program. So this PR adds infer, a dispatch family mirroring the expression overloads, over a scope seeded with the function's parameters. Operands are lowered at the widest type among them.

The rank is bool < i32 < i64 < f32 < f64. Float outranks every integer because mixing the two compares as float, which is what Python and C both do. Worth a second opinion: that means i64 < f32 compares in single precision.

3. Chained comparisons repeat an operand.

a < b < c folds to (a < b) && (b < c), which evaluates b twice. That is only safe because the v1 subset admits no expression with an effect — no calls, no walrus — so a second evaluation returns what the first did. Stated explicitly because it stops being true the moment the subset grows.

4. and / or are not Python's and / or.

Python's evaluate to one of their operands; lowered they are logical operators. Same answer only where the operands are already bools. IRx enforces this: binary_result_type("&&", Int64, Int64) returns None, so a and b over two integers is a type error there, not here. This is the same question as your not-in-a-numeric-context note on #109 — flagging them together since the answer is one decision.

From the #109 review

• -True is fixed. It folded to Int64 -1: bool subclasses int, and negating one in Python produces an int, so the value reached _literal_value as an int and the bool-before-int check could not see it. Now refused.
• _UNARY_OPS' generic lookup — no change needed. test_a_unary_operator_astx_lacks_is_rejected already exercises it: it lowers ~a through the test helper that bypasses validation, so ast.Invert does reach the op_code is None branch.

An upstream astx bug this surfaces

astx.UnaryOp.type_ is the generic ExprType, not a DataType, and astx.BinaryOp requires a DataType on both operands. So not a cannot be an operand of any binary operator, and astx raises a bare Exception with no source location.

This is not new — a + (not b) fails the same way on main today. Every binary node is now built through one guard that reports it as a located diagnostic instead. The real fix belongs in astx: giving UnaryOp the type of its operand would make these compose. Happy to open that upstream if you agree.

Two unreachable branches removed

Found while getting coverage back to 100%, and deleted rather than left in:

• the boolean operator lookup had no failing case — Python defines exactly two boolean operators and both are mapped;
• a rank lookup cannot fail, because a signature naming a type with no astx class is refused while the prototype is built, before any of the body is lowered. test_every_sig_type_is_ranked pins the tables together instead.

Verification

285 tests, lowering.py at 100% line and branch coverage, arxjit total 99.69%. Run on CPython 3.10, 3.11 and 3.14. ruff, mypy strict, bandit, vulture and douki all clean; douki idempotent.

Comparison tests assert through analyze() rather than only on node shape, so each emitted form is proven to be one IRx accepts — including the narrow-type cases where an i32 or f32 parameter is compared against a wider literal.

Next

Local assignment (locals join the scope, rebinding rules), then control flow.

[xmnlab]

Thanks for the detailed PR. Lowering simple comparisons through IRx-supported BinaryOp opcodes looks reasonable, but I found two correctness bugs that should be fixed before merge. There is also one numeric-promotion decision that needs to be corrected or explicitly documented.

Blocking issue 1: and, or, and chained comparisons must short-circuit

The current implementation lowers and, or, and chained comparisons by building ordinary &&/|| binary nodes with _fold().

That does not preserve Python evaluation order. IRx visits both operands before emitting the LLVM logical instruction, so the right-hand side is always evaluated.

For example:

def sample(a):
    return a and (1 % 0 > 0)

When a is False, Python returns False without evaluating 1 % 0. The lowered version evaluates the RHS anyway and can trap.

Chained comparisons have the same problem:

def sample():
    return 0 > 1 > (1 % 0)

Python stops after 0 > 1 is false. The current lowering evaluates the second comparison too.

The statement that repeating operands is safe because the subset has no side effects is therefore not sufficient. Exceptions and computations that should be unreachable are also observable behavior.

What to change

Please do not lower Python short-circuit expressions to ordinary IRx logical binary operators.

The safest solution for this PR is:

1. Keep support for a single comparison such as a < b.
2. Reject chained comparisons for now with a clear LoweringError.
3. Reject ast.BoolOp for now with a clear LoweringError.
4. Add them later, together with control-flow lowering.

Concretely, the ast.Compare overload can reject len(node.ops) > 1, and the ast.BoolOp overload can reject the expression instead of calling _fold().

If you prefer to support them in this PR, they must be lowered using branches/control-flow so that:

• operands are evaluated from left to right;
• an and RHS runs only when the LHS is true;
• an or RHS runs only when the LHS is false;
• a chained comparison stops after its first false link;
• each middle operand in a comparison chain is evaluated exactly once.

Changing the opcode or changing the fold from left to right will not fix this. IRx’s binary logical nodes are eager.

Tests required

Please add builder/execution-level tests for at least:

False and (1 % 0 > 0)  # returns False without evaluating RHS
True or (1 % 0 > 0)    # returns True without evaluating RHS
0 > 1 > (1 % 0)        # returns False without evaluating final comparator

Calling only analyze() is not enough to test evaluation order.

Blocking issue 2: / is inferred as integer division

The new infer(ast.BinOp) implementation only examines the operands:

return self._wider(self.infer(node.left), self.infer(node.right))

This is incorrect for /. In Python, true division of two integers produces a floating-point result.

For example:

def sample():
    return 1 / 2 > 0

Python evaluates this as:

0.5 > 0  # True

The current lowering does this instead:

1. Infer 1 as i64.
2. Infer 2 as i64.
3. Infer 1 / 2 as i64.
4. Lower / as integer sdiv.
5. Produce 0 > 0, which is False.

What to change

Operator semantics must participate in BinOp inference.

At minimum:

• integer / must infer a floating-point result, normally f64;
• +, -, *, and % may continue using numeric operand promotion;
• mixed integer/float division must follow the project’s documented promotion rule.

Please do not fix this by changing only the test expectation or by relying on IRx analysis. Once the operands have already been lowered as integers, IRx correctly treats / as integer division.

Tests required

Please add a builder/execution-level regression proving:

1 / 2 > 0

returns True.

It would also be useful to cover:

3 / 2 == 1.5

Numeric-promotion issue: Float32 cannot safely outrank Int64

The _TYPE_RANK table places Float32 above Int64 and describes the selected type as one that can represent both operands.

That statement is not true: Float32 cannot exactly represent every Int64.

For example, with an f32 parameter:

def sample(a):
    return a < 16_777_217

The integer literal can be converted to Float32 and rounded to 16_777_216. If a is 16_777_216, Python’s comparison is true, but the lowered comparison becomes false.

This also disagrees with IRx’s existing promotion policy: IRx promotes an f32/i64 pair to f64.

What to change

Please align the frontend promotion rule with IRx’s numeric-promotion policy.

A pairwise promotion table or shared helper is safer than a single ordered rank because promotion depends on both types. In particular:

i32 + f32 -> f32
i64 + f32 -> f64

Please do not simply reorder two entries without checking every supported type pair.

If importing the IRx promotion helper here is intentionally avoided, add a test that compares every arxjit promotion pair with IRx’s expected result so the two policies cannot drift.

Add a regression around 16_777_217 to confirm that an integer literal is not silently rounded before the comparison.

Recommended scope for this PR

The smallest safe revision would be:

1. Keep simple comparisons.
2. Temporarily reject and, or, and chained comparisons.
3. Make BinOp inference operator-aware, especially for /.
4. Align numeric promotion with IRx.
5. Add builder- or execution-level tests for the examples above.

The current CI checks are green, but AST-shape tests and analyze() cannot detect these runtime semantic differences. Given the first two issues, I’m requesting changes before merge.

PS: additionally please don't prefix private methods, classes or variables with _ (underscores), just use "atpublic.private" when necessary

[xmnlab]

@Jaskirat-s7 any estimate to finish this pr? I am planning to implement new stuff in the compiler ... so it will be nice to have this merged as soon as possible

[xmnlab]

@Jaskirat-s7 I will merge this just to start to work here ... otherwise a lot of conflicts will pop up ... thanks for working on that