Various fixes to codegen and generator runtime#23
Merged
Conversation
…type The Try-handler codegen stored the matched exception *type* into the bound name (`store_name(handler->name(), handler->type()->codegen(), …)`), so `except ValueError as e:` left `e` as the `ValueError` class instead of the raised instance — `isinstance(e, ValueError)` was false, `type(e)` was `type`, and any use of `e`'s value (e.g. `str(e)`) was wrong. (This is the real bug behind a "miscompile" that looked layout-fragile only because of a separate wrong-traceback-line issue.) Add a `py.load_exception` op (lowered to a new LOAD_EXCEPTION bytecode instruction, opcode 84) that reads the active exception instance from `execution_frame()->exception_info()->exception`, modeled on LoadAssertionError across the pipeline (Python + EmitPythonBytecode dialect ops, ControlFlow lowering, bytecode emitter, instruction decoder). Bind the name to that instance at the start of the matched handler block instead of to the type in the cond block.
Each builtin exception is a distinct C++ class with its own create(), so
each must define __new__ to allocate its own type; the inherited
Exception::__new__ both hardcodes the Exception class and asserts
`type == types::exception()`. RuntimeError and NameError were missing
__new__, so `raise RuntimeError(...)` / `raise NameError(...)` aborted at
that assert. ModuleNotFoundError::__new__ dereferenced `kwargs->map()`
unconditionally, so `raise ModuleNotFoundError("m")` (no kwargs)
segfaulted.
- Add RuntimeError::__new__ and NameError::__new__ (mirroring ValueError).
- Guard the null kwargs in ModuleNotFoundError::__new__.
- BaseException.args now returns an empty tuple (not None) when the
exception was constructed without args, matching CPython.
Regression test integration/tests/exception_types.py raises every builtin
exception and checks isinstance/type/args. Full ctest (194) + integration
suite green.
(Note: the test calls its per-type check via a helper rather than inline in
the loop, to avoid the separate FOR_ITER iterator-register clobber bug that
a heavy loop body triggers — see integration/minimal_foriter_bug.py.)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XZ4THY2jXpZnNoxVTzDn32
A try body lowers to blocks whose only explicit CFG successors are the normal-flow ones (e.g. RAISE_VARARGS -> exit); the edge from a faulting op in the try body to the handler is implicit (established by SETUP_EXC_HANDLE in the *predecessor*). mlir::Liveness therefore never sees body-op -> handler, so a value that is live at the handler — or live after it — was considered dead inside the try body, and the register allocator happily reused its register there. When an exception actually unwound, that register had been overwritten. The clearest case: a `for` loop whose body contains `try/except`. The FOR_ITER iterator is read again after the handler (next iteration), but its register was reused for a value inside the try body, so the resumed loop read a clobbered iterator (abort in ForIter / "object is not an iterator"). The same bug corrupted exception args across sequential try/except blocks. Fix: in LiveAnalysis, for every block in a try body (those dominated by the SETUP_EXC_HANDLE / SETUP_WITH try-entry successor), add the handler's live-in set to the block's live-out before computing per-op liveness. Any op in the try body can transfer to the handler, so values live at the handler are live across the whole try body. Dominance keeps it bounded and handles nesting; the pass is skipped entirely when a function has no handlers. This makes the iterator (and any value live past the handler) interfere with try-body temporaries, so they no longer share a register.
…_context__ `raise X from Y` already stored the cause on the exception (RaiseVarargs::execute), but BaseException exposed none of the chaining attributes, so __cause__ was unreadable. - Add context()/set_context() and suppress_context()/set_suppress_context() accessors on BaseException (m_context/m_cause/m_suppress_context already existed and are GC-visited). - Register __cause__, __context__ and __suppress_context__ as read/write properties. Setting __cause__ also sets __suppress_context__ = True, and __suppress_context__ coerces via truthiness, matching the data model. - `raise X from Y` now also sets __suppress_context__ = True (and `from None` keeps the suppression with a None cause). Implicit __context__ chaining (auto-setting the new exception's context to the one being handled) is intentionally left out: the frame exception stack is not reliably popped — internally-consumed StopIterations linger (the same pre-existing bug that makes a bare `raise` outside a handler re-raise a stale exception instead of erroring), so reading it would attach a spurious context.
…ext__
The frame exception stack is shared across a call chain, and the eval loop
pushed every raised exception onto it (so handlers can read it) but only
popped on handler completion. Exceptions that propagated out of a frame
uncaught — including a generator/genexpr's completion StopIteration consumed
by the FOR_ITER that resumed it — were pushed and never popped, so they
accumulated. A later bare `raise` re-raised that stale StopIteration instead
of erroring, and implicit __context__ would pick it up.
Balance the lifecycle: when an exception propagates out of a frame that has
no handler, the eval loop now pops the entry it just pushed (the caller's
eval loop re-pushes it). With the stack kept clean:
- ReRaise returns RuntimeError("No active exception to reraise") on an empty
stack instead of asserting (bare `raise` outside a handler).
- The top-level driver uses the propagated result value rather than popping
the exception off the (now-empty) stack.
- from_iterable's manual StopIteration cleanup is removed — it is handled by
the eval loop now, and keeping it would wrongly pop a pre-existing
exception when iterating during exception handling.
- RaiseVarargs re-enables implicit __context__ chaining (set the new
exception's context to the one being handled); now reliable since the
stack no longer holds stale state.
exception_chaining.py now covers implicit __context__, bare `raise` ->
RuntimeError, and iterating generators/comprehensions while handling an
exception.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.