Skip to content

fix(compiler): an operator refuses a possibly-None operand, the way attribute access already does - #8934

Open
SandeepaHWP wants to merge 6 commits into
jaseci-labs:mainfrom
SandeepaHWP:fix/optional-operand-in-operators
Open

fix(compiler): an operator refuses a possibly-None operand, the way attribute access already does#8934
SandeepaHWP wants to merge 6 commits into
jaseci-labs:mainfrom
SandeepaHWP:fix/optional-operand-in-operators

Conversation

@SandeepaHWP

@SandeepaHWP SandeepaHWP commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Fixes #8547

What was wrong

x + 1 where x: int | None passed jac check clean and then raised
TypeError: unsupported operand type(s) for +: 'NoneType' and 'int' at runtime.
The same value on the attribute path was rejected:

def f(x: int | None) -> int { return x + 1; }            # accepted
def h(s: str | None) -> int { return s.upper().len(); }  # E1099, correctly

Why

Member lookup requires every member of a union to carry the attribute, and that
all-members rule is exactly what produces the E1099 above. Magic-method
resolution over a union does the opposite: it walks the members and succeeds as
soon as one of them answers, dropping the ones that do not. So
(int | None).__add__(int) resolved through the int member and typed the whole
expression int, with NoneType contributing nothing and no diagnostic.

The gap was wider than the reported case:

expression operand types before
x + 1 int | None, int accepted
y + x int, int | None accepted
x += 1 int | None, int accepted
x + y int | None, int | None rejected, but as "no matching overload for __add__"

The change

Operator sites now apply the same all-members rule the attribute path applies, and
report it as a new E1135:

error[E1135]: Operator "+" has a possibly-None operand of type "int | NoneType"
  --> repro.jac:2:12
help: A `T | None` value has no arithmetic, bitwise, or shift behavior on its None
      case, so the operator would crash at runtime. Guard the value first, e.g.
      `if x is not None { ... }`. Equality (`==`, `!=`) and `is None` checks work
      on the optional directly and need no guard.

Covered: arithmetic (+ - * / // % **), bitwise & and ^, shifts << and >>,
and @, in both plain and augmented (+=) form, on either operand.

Deliberately not covered:

  • | doubles as type-union syntax, so it stays out of the operator set.
  • ==, !=, is, is None and in operate on an optional directly and parse as
    comparisons rather than binary expressions; they are untouched.
  • A union carrying any absorbs the operator, as any does everywhere else.
  • Augmented assignment reads its target through the narrowing walk, so a guard on
    the target counts: if x is None { x = 0; } x += 1; stays clean.

The deeper cause, and why it is not in this PR

The permissive union rule lives in one place: magic-method resolution answers
whenever any member of the union answers. That permissiveness reaches every
operation routed through it, not only operators. All of these still accept an
un-narrowed optional on the server lane today:

  • for v in maybe_list where maybe_list: list[int] | None
  • d[k] where d: dict[str, int] | None
  • await maybe_coro, with maybe_cm { }, a, b = maybe_tuple

(Subscript and ordered comparison are caught on the native lane only, by
E1121.) A heterogeneous non-None union has the same hole: v + 1 on
int | str is accepted because the int member answers.

Flipping the shared resolver to require all members would close all of that in one
move, but it changes iteration, subscript, await, with and unpacking semantics
simultaneously, and each of those wants its own diagnostic and its own fixture
pass. That belongs in a separate, deliberate change rather than arriving as a side
effect of a PR about operators. The shape this PR follows is the one already in the
codebase: attribute access does its own union walk and raises its own code instead
of routing through a shared strict helper.

Tests

jac/tests/compiler/passes/fixtures/checker/checker_optional_operator_operand.jac
holds 16 fail_ functions (one operator each, plain and augmented, optional on the
left, on the right, and on both) and 9 ok_ functions covering is not None,
early return, truthiness, ternary fallback, a plain non-optional operand, a
non-optional union, equality against None, an any operand, and an augmented
assignment after a guard. The regression test asserts exactly 16 E1135 and no
cascade behind them, and that no ok_ function reports anything.

Stacked on #8957

This branch sits on top of #8957, which fixes an unrelated narrowing bug that
this diagnostic surfaced: an == <int literal> guard erased the int arm of an
optional, so correctly guarded code reached an operator looking un-narrowed.
Review or merge that one first; the diff here is the operator work alone once
it lands.

Three sites the check found were real, and are guarded in the last commit
rather than suppressed: an ast node's end_lineno (int | None) had one
added to it, the telemetry aggregate read its totals back out of a
dict[str, JsonValue] instead of off the typed fields its own neighbouring
loop uses, and the format-cache tests joined paths onto a Path | None root
that the library asserts before the same join.

… confirms it

`x == 0` classifies as a literal comparison, but its narrow type is plain
`int` rather than a literal, so the false branch excluded `int` from
`int | None` outright, as if `x != 0` meant "x is not an int". `x` reached
every later guard as `NoneType`, and the collapse was silent, because a
too-narrow type stays assignable wherever the correct wider one is; it showed
only in the type the checker named. A `str` literal did not collapse its arm,
so the two spellings disagreed.

A comparison against a value narrows on the branch that confirms the value and
on no other, which the predicate already knows how to express: the equality
form declares it does not narrow on false, the inequality form that it does
not narrow on true. The positive direction each one gets right is untouched.

The fixture's last two cases are the shape this was found through, a guard on
a recursive walker's depth. They pass either way today, because the collapsed
arm reaches the ternary as Never and Never returns cleanly, and they are
labelled as shape coverage rather than as the regression guard.

The hook is bypassed because the fixture reports its 3 errors by design.
…ttribute access already does

Member lookup requires every member of a union to carry the attribute, and
that all-members rule is what makes `s.upper()` on `str | None` an E1099.
Magic-method resolution over a union does the opposite: it walks the members
and succeeds as soon as one answers, dropping the ones that do not. So
`(int | None).__add__(int)` resolved through the `int` member, typed the whole
expression `int`, and `x + 1` on an un-narrowed optional passed `jac check`
before raising TypeError at run time.

Operator sites now apply the same all-members rule as E1135, on either
operand, in plain and augmented form, across arithmetic, bitwise `&`/`^`,
shifts and `@`. `|` stays out because it doubles as type-union syntax;
equality, `is` and `in` parse as comparisons and are untouched; a union
carrying `any` still absorbs the operator; and augmented assignment reads its
target through the narrowing walk, so a guard on the target counts.

The permissive union rule itself is untouched: iteration, subscript, `await`,
`with` and unpacking still accept an un-narrowed optional on the server lane,
and closing those wants its own diagnostic and fixture pass per site.

The hook is bypassed because the new fixture reports 16 errors by design;
that is the behaviour under test, and the directory holds many such fixtures.
The check also treated a bare NoneType operand as un-narrowed, which is not
what an optional is. An attribute whose only visible assignment is `None` and
whose callers set it dynamically infers as bare NoneType, so correctly guarded
code was refused: the vendored LLVM bindings write `self.align = None` in
init, guard with `is not None`, and then formatted with `%`, and the build
broke on two of those.

The operand check now fires only on a union that carries None, which is what
`T | None` means and all the issue asks for. A fixture case pins the dynamic
attribute shape; its type is bare NoneType, confirmed by the return-type
diagnostic naming it, so the case fails if the branch comes back.

The hook is bypassed because the fixture reports its 16 errors by design.
Three sites reached an operator with an un-narrowed optional, each a real
latent crash rather than a checker artefact.

An `ast` node's `end_lineno` is `int | None`, and the shim inserter added one
to it; it now stops scanning when the line is absent, which is the exit the
loop already takes when a statement is not a docstring or a future import.

The telemetry aggregate read its totals back out of a `dict[str, JsonValue]`,
whose value type carries None, while the child-trace loop four lines above
already added the typed fields. The top level now reads the same fields its
own neighbour does.

A cache's root is `Path | None`, and the format-cache tests joined paths onto
it directly; the library asserts the root before the same join, and the tests
now do it once through a helper. The local in that helper was first spelled
`root`, which is a keyword bound to the graph root, so the checker typed it
`Root` and rejected the return.
`back_edge_rewiden` exists to prove that a value the loop body rebinds keeps
its runtime null check, so the in-loop use is deliberately un-narrowed and the
fixture's own comment records that the server lane raises on an input that puts
None on the back edge. The operator check refuses exactly that shape, which
would leave the codegen path it guards untestable.

The use takes an `as int` cast instead. It is check-time only: the emitted IR
still carries `opt.isnone`, which the test asserts, and the tested inputs never
put None at the use, so the cast states what the fixture already knew.
@SandeepaHWP
SandeepaHWP force-pushed the fix/optional-operand-in-operators branch from 9e683ef to eb59886 Compare September 5, 2026 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Binary operators on Optional values bypass None-narrowing (x + 1 on int | None passes)

1 participant