Skip to content

Add a generator-expression bytecode disassembler - #725

Merged
jfeser merged 3 commits into
staging-weightedfrom
eb-disassembly
Jul 28, 2026
Merged

Add a generator-expression bytecode disassembler#725
jfeser merged 3 commits into
staging-weightedfrom
eb-disassembly

Conversation

@eb8680

@eb8680 eb8680 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Adds effectful/internals/disassembly.py, a standalone bytecode disassembler for generator expressions, plus its test suite.

What it does

The single public entry point is disassemble(gen). Given a generator object, it symbolically interprets the bytecode of the underlying code object and reconstructs an ast node for the comprehension that produced it — its targets, its iterables, its if guards, and the element expression — including lambdas and comprehensions nested inside the element expression.

>>> g = (x * 2 for x in range(10) if x % 2 == 0)
>>> node = disassemble(g)  # ast.Expression for the original comprehension

This lets a comprehension's source syntax be recovered from a code object at runtime, which is what the follow-up PR uses to give Monoid a comprehension syntax.

No coupling

disassembly.py imports nothing from effectful — only stdlib (ast, dis, types, inspect, ...). This PR adds two new files and changes no existing library code, so it is safe to land ahead of anything that consumes it. No packaging changes are needed: effectful/internals/ is already a package.

Version support

Bytecode layout is version-specific, so the module dispatches on sys.version_info.minor through a PythonVersion enum covering 3.12, 3.13 and 3.14, and raises NotImplementedError on an unrecognised version rather than guessing that the previous release's opcodes still mean what they used to.

Locally I could only exercise the 3.12 path (the dev interpreter here is 3.12.11); the 3.13 and 3.14 paths need CI to cover.

Tests

tests/test_internals_disassembler.py: 620 passed, 2 xfailed on 3.12.

The two xfails are known limitations of reconstructing from bytecode, not regressions:

  • test_conditional_expression_as_iterable[<genexpr>3] — an empty list literal is indistinguishable from the start of a list comprehension.
  • test_conditional_expression_as_iterable[<genexpr>6] — two conditional iterables in one comprehension leave paths that do not pairwise merge.

The full suite (effectful/ tests/, excluding the LLM handler tests) is unchanged and green: 18968 passed, 2 skipped, 2080 xfailed. ruff check and ruff format --diff are clean. mypy reports one pre-existing error in effectful/handlers/jax/monoid.py:382, which is present on staging-weighted and unrelated to this PR.

Split out of #724 for review. Part of a stack: based on #729 (the _jax_args
fix), and #727 (eb-comprehension) builds on this. This PR's own diff is just
the two new files.

🤖 Generated with Claude Code

`_jax_args` admitted `jax.typing.ArrayLike`, a union that includes `bool`,
`int`, `float` and `complex`, so the jax `Monoid.plus` handlers claimed
pure-Python scalar arithmetic. They extend `EvaluateIntp` after the scalar
implementations and so take precedence, silently narrowing a Python float
to a `float32` array and leaving downstream rules treating a scalar body as
array-valued. Require at least one genuine array.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@eb8680
eb8680 marked this pull request as ready for review July 28, 2026 15:27
@eb8680
eb8680 requested a review from jfeser July 28, 2026 15:27
@eb8680

eb8680 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Test failure is spurious and unrelated. The tests for this PR are part of the "Test / core" step, which passed for all Python versions in the build matrix. This is ready for review.

`effectful/internals/disassembly.py` symbolically interprets the bytecode
of a generator expression (and of lambdas and comprehensions nested inside
it) back into an `ast` node, so a comprehension's source syntax can be
recovered from the code object at runtime. Supports CPython 3.12 and 3.13.

Standalone: imports nothing from `effectful` and touches no existing code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@eb8680
eb8680 changed the base branch from staging-weighted to eb-jax-scalar-plus July 28, 2026 15:37
@eb8680
eb8680 marked this pull request as draft July 28, 2026 16:08
@eb8680
eb8680 marked this pull request as ready for review July 28, 2026 16:09
Base automatically changed from eb-jax-scalar-plus to staging-weighted July 28, 2026 16:19
@jfeser

jfeser commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Per ChatGPT:

  1. Dynamic dictionary displays are reconstructed in reverse order
    effectful/internals/disassembly.py:1172
    handle_build_map() reads key/value pairs from the top of the simulated stack but appends them directly, reversing source order. For example:

    ({x: "first", x: "second"} for x in range(1))

    is reconstructed as:

    ({x: "second", x: "first"} for x in range(1))

    and therefore yields {0: "first"} instead of {0: "second"}. Reversal also changes evaluation order when key or value expressions have side effects.
    Suggested fix: Reverse the collected pairs before constructing ast.Dict, or slice the stack in source order. Test duplicate dynamic keys, insertion order, and side-effecting key/value expressions.

  2. Legitimate tuples beginning with "dict_item" are silently corrupted
    effectful/internals/disassembly.py:3112
    _ensure_ast_tuple() treats every tuple whose first element equals "dict_item" as an internal marker and drops that element. Consequently:

    (x for x in (("dict_item", 1, 2),))

    reconstructs with ((1, 2),) as its iterable and yields the wrong value. No production code in this module creates this marker; only tests reference it.
    Suggested fix: Remove the string-based special case, or represent internal dictionary entries with a private typed wrapper that cannot collide with user data. Add a round-trip test for tuples beginning with "dict_item" in constants and outer iterables.

Comment thread effectful/internals/disassembly.py Outdated
cases. However, the semantic behavior of the reconstructed AST should
match the original comprehension.
"""
assert inspect.isgenerator(genexpr), "Input must be a generator expression"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should raise a ValueError instead of asserting.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in ccf7733. disassemble now raises ValueError for a non-generator, and also for a generator that has already been started — that check had been left to an assert further in, in _ensure_ast_genexpr. The docstring's Raises: section is updated, and test_error_handling covers all three cases.

@jfeser

jfeser commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

I used some LLM time to look at this, but I can't provide meaningful review given its size.

@eb8680

eb8680 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

I used some LLM time to look at this, but I can't provide meaningful review given its size.

I think reviewing the implementation details wouldn't be so useful anyway. This code has a very slim interface (just disassemble) and a precise specification that is the focus of all of the tests (materialize(genexpr) == materialize(eval(compile(disassemble(genexpr)))), roughly), so review should focus on test case coverage.

@jfeser

jfeser commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

A few more ChatGPT-discovered issues. The first one looks like it could lead to surprises depending on how exactly the disassembled generator is converted to a Term. The second is a straightforward bug.

  1. Captured closure variables become global lookups
    effectful/internals/disassembly.py:1453
    LOAD_DEREF is reconstructed as a plain ast.Name. For an external free variable, the original bytecode reads a captured cell, while the compiled reconstructed AST resolves the name globally:

    def make():
        value = 1
        return (value for _ in range(1))

    Evaluating the reconstructed AST with a global value = 2 yields 2; the original generator yields captured value 1. This also affects lambdas nested inside the generator.
    Suggested fix: Distinguish local cell variables from code.co_freevars and preserve free-variable bindings, possibly by returning an evaluation namespace alongside the AST or substituting frame-cell values where they can safely be represented. Test a closure whose free-variable name conflicts with a global.

  2. zip(strict=True) loses strictness
    effectful/internals/disassembly.py:3215
    _ensure_ast_iterator_adaptor() uses only the callable and arguments from __reduce__(). For a strict zip, the strict flag is stored in the reduction state and is ignored. The reconstructed zip therefore silently truncates mismatched iterables where the original raises ValueError.
    Suggested fix: Recognize the third reduction component for zip and emit strict=True. Add a mismatched-length outer zip(..., strict=True) test.

@jfeser

jfeser commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

I'm assuming we're ok with changing the semantics of generators with stateful predicates. We might make a documentation note about that though.

@jfeser

jfeser commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Lambda default values are not preserved: (fn() for fn in [lambda value=1: value]) raises but should yield 1.

Six fixes, each with tests that fail without them:

- `handle_build_map` read the key/value pairs of a dict display from the
  top of the stack down, reversing source order: a later duplicate key
  lost to an earlier one, and side effects ran backwards.

- `_ensure_ast_tuple` treated any tuple whose first element was the
  string "dict_item" as an internal marker and dropped that element.
  Nothing produced such a marker; user data holding that string was
  silently corrupted. The special case is gone.

- A free variable was reconstructed as a bare `ast.Name`, so evaluating
  the result resolved it against the evaluating namespace instead of the
  captured cell. The captured value is now written into the tree, for
  the generator itself, for lambdas reached as live objects, and for
  lambdas and comprehensions nested inside. A cell the comprehension
  creates -- a target captured by a nested lambda -- still stands as a
  name, since the reconstruction binds it too. A capture with no AST
  spelling, including an iterator, raises `TypeError` rather than
  reconstructing to a name that would answer differently.

- `_ensure_ast_iterator_adaptor` ignored the strictness a `zip` pickles
  as reduction state, so a strict zip silently truncated ragged input
  where the original raised.

- A lambda reached as a live object lost its default values, which live
  on the function rather than in its code object, leaving parameters
  with no way to be filled.

- `disassemble` asserted on its input; it now raises `ValueError`, and
  checks the generator has not been started rather than leaving that to
  an assert further in.

Also documents what reconstruction does and does not recover: evaluating
the result re-runs every expression in it, so a stateful filter answers
against state as it then stands.

663 passed, 2 xfailed on 3.12, 3.13 and 3.14.
@eb8680

eb8680 commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — all six are fixed in ccf7733. Each has a test that I confirmed fails against the pre-fix module.

1. Dict displays reconstructed in reverse order. handle_build_map now slices the pairs off the stack in source order. Tests pin the three things dict equality won't catch: which of two equal keys wins (test_dict_display_duplicate_keys), what items() yields (test_dict_display_insertion_order), and the order key and value subexpressions actually run in (test_dict_display_evaluation_order, which records each subexpression as it evaluates and asserts the original and the reconstruction agree).

2. Tuples beginning with "dict_item". The special case is gone — nothing in the module ever produced that marker, so there was no internal representation to replace it with. test_tuples_starting_with_dict_item round-trips such tuples as constants, in outer iterables, in element position, and as dict keys.

3. Captured closure variables. Substituting the value, per your second suggestion. The bindings come from the generator's frame (an unstarted generator has its cells copied in already) or from __closure__ for a lambda reached as a live object, and are threaded through ReconstructionState so lambdas and comprehensions nested inside the body resolve against the same cells.

The distinction you asked for is on co_freevars specifically: a cell the comprehension creates — a target captured by a nested lambda, as in ((lambda: x)() for x in range(3)) — still reconstructs as a name, because the reconstructed tree binds it too. Only a cell reaching outside gets its value written in.

Every closure test evaluates the reconstruction in a namespace binding the same name to something else, so a lookup that leaked out to globals fails the test — including the conflicting-global case you asked for, and both shadowing directions (a nested comprehension target and a lambda parameter that reuse a captured name).

One case I had to rule out rather than support: a captured iterator. ensure_ast spells an iterator as the elements it has left, which is right for the outermost iterable — it's about to be consumed anyway — but a captured one is a value the body can do anything with, and next([True, False]) is not next(iter([True, False])). That now raises TypeError, as does any other capture with no AST spelling, rather than reconstructing to a name that would quietly answer differently.

4. zip(strict=True). Read off the third reduction component and emitted as a keyword. Tests cover a ragged strict zip raising on both sides, a lax zip staying lax, strict zips in the round-trip parametrization, and a partly-consumed strict zip.

5. Lambda default values. Good catch — (fn() for fn in [lambda value=1: value]) raised. Defaults live on the function object, not the code object, so a lambda built inside the comprehension got them (from the stack, via MAKE_FUNCTION/SET_FUNCTION_ATTRIBUTE) but one arriving as a live object did not. _ensure_ast_lambda now attaches __defaults__ and __kwdefaults__; test_lambda_object_defaults covers positional, positional-only, keyword-only, variadic, and non-trivial default expressions.

6. Stateful predicates. Agreed, and documented. The module docstring has a new section on what reconstruction does and does not recover, covering all three ways the reconstruction can part company with the generator it came from: a stateful filter or element expression is re-run and can answer differently the second time; the outermost iterable is a snapshot of unconsumed elements rather than the expression that produced it; and a free variable is recovered by value. disassemble's own docstring points at it. test_stateful_filter_is_re_evaluated pins the behaviour rather than leaving it to prose.

Tests: 663 passed, 2 xfailed on 3.12, 3.13 and 3.14 locally — so the two version paths the PR description flagged as CI-only are covered here too. ruff check, ruff format --diff and mypy are clean on both changed files.

@jfeser
jfeser merged commit 0553c4a into staging-weighted Jul 28, 2026
23 of 29 checks passed
@jfeser
jfeser deleted the eb-disassembly branch July 28, 2026 19:54
jfeser pushed a commit that referenced this pull request Jul 29, 2026
* Don't route all-scalar monoid ops through jax

`_jax_args` admitted `jax.typing.ArrayLike`, a union that includes `bool`,
`int`, `float` and `complex`, so the jax `Monoid.plus` handlers claimed
pure-Python scalar arithmetic. They extend `EvaluateIntp` after the scalar
implementations and so take precedence, silently narrowing a Python float
to a `float32` array and leaving downstream rules treating a scalar body as
array-valued. Require at least one genuine array.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Add a generator-expression bytecode disassembler

`effectful/internals/disassembly.py` symbolically interprets the bytecode
of a generator expression (and of lambdas and comprehensions nested inside
it) back into an `ast` node, so a comprehension's source syntax can be
recovered from the code object at runtime. Supports CPython 3.12 and 3.13.

Standalone: imports nothing from `effectful` and touches no existing code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Address review comments on the generator-expression disassembler

Six fixes, each with tests that fail without them:

- `handle_build_map` read the key/value pairs of a dict display from the
  top of the stack down, reversing source order: a later duplicate key
  lost to an earlier one, and side effects ran backwards.

- `_ensure_ast_tuple` treated any tuple whose first element was the
  string "dict_item" as an internal marker and dropped that element.
  Nothing produced such a marker; user data holding that string was
  silently corrupted. The special case is gone.

- A free variable was reconstructed as a bare `ast.Name`, so evaluating
  the result resolved it against the evaluating namespace instead of the
  captured cell. The captured value is now written into the tree, for
  the generator itself, for lambdas reached as live objects, and for
  lambdas and comprehensions nested inside. A cell the comprehension
  creates -- a target captured by a nested lambda -- still stands as a
  name, since the reconstruction binds it too. A capture with no AST
  spelling, including an iterator, raises `TypeError` rather than
  reconstructing to a name that would answer differently.

- `_ensure_ast_iterator_adaptor` ignored the strictness a `zip` pickles
  as reduction state, so a strict zip silently truncated ragged input
  where the original raised.

- A lambda reached as a live object lost its default values, which live
  on the function rather than in its code object, leaving parameters
  with no way to be filled.

- `disassemble` asserted on its input; it now raises `ValueError`, and
  checks the generator has not been started rather than leaving that to
  an assert further in.

Also documents what reconstruction does and does not recover: evaluating
the result re-runs every expression in it, so a stateful filter answers
against state as it then stands.

663 passed, 2 xfailed on 3.12, 3.13 and 3.14.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
jfeser pushed a commit that referenced this pull request Jul 30, 2026
* Don't route all-scalar monoid ops through jax

`_jax_args` admitted `jax.typing.ArrayLike`, a union that includes `bool`,
`int`, `float` and `complex`, so the jax `Monoid.plus` handlers claimed
pure-Python scalar arithmetic. They extend `EvaluateIntp` after the scalar
implementations and so take precedence, silently narrowing a Python float
to a `float32` array and leaving downstream rules treating a scalar body as
array-valued. Require at least one genuine array.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Add a generator-expression bytecode disassembler

`effectful/internals/disassembly.py` symbolically interprets the bytecode
of a generator expression (and of lambdas and comprehensions nested inside
it) back into an `ast` node, so a comprehension's source syntax can be
recovered from the code object at runtime. Supports CPython 3.12 and 3.13.

Standalone: imports nothing from `effectful` and touches no existing code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Address review comments on the generator-expression disassembler

Six fixes, each with tests that fail without them:

- `handle_build_map` read the key/value pairs of a dict display from the
  top of the stack down, reversing source order: a later duplicate key
  lost to an earlier one, and side effects ran backwards.

- `_ensure_ast_tuple` treated any tuple whose first element was the
  string "dict_item" as an internal marker and dropped that element.
  Nothing produced such a marker; user data holding that string was
  silently corrupted. The special case is gone.

- A free variable was reconstructed as a bare `ast.Name`, so evaluating
  the result resolved it against the evaluating namespace instead of the
  captured cell. The captured value is now written into the tree, for
  the generator itself, for lambdas reached as live objects, and for
  lambdas and comprehensions nested inside. A cell the comprehension
  creates -- a target captured by a nested lambda -- still stands as a
  name, since the reconstruction binds it too. A capture with no AST
  spelling, including an iterator, raises `TypeError` rather than
  reconstructing to a name that would answer differently.

- `_ensure_ast_iterator_adaptor` ignored the strictness a `zip` pickles
  as reduction state, so a strict zip silently truncated ragged input
  where the original raised.

- A lambda reached as a live object lost its default values, which live
  on the function rather than in its code object, leaving parameters
  with no way to be filled.

- `disassemble` asserted on its input; it now raises `ValueError`, and
  checks the generator has not been started rather than leaving that to
  an assert further in.

Also documents what reconstruction does and does not recover: evaluating
the result re-runs every expression in it, so a stateful filter answers
against state as it then stands.

663 passed, 2 xfailed on 3.12, 3.13 and 3.14.

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.

2 participants