Skip to content

fix[next]: collect closure vars from nested code objects - #2861

Open
havogt wants to merge 1 commit into
GridTools:sf_n_tracer_supportfrom
havogt:c1-closure-vars-repro
Open

fix[next]: collect closure vars from nested code objects#2861
havogt wants to merge 1 commit into
GridTools:sf_n_tracer_supportfrom
havogt:c1-closure-vars-repro

Conversation

@havogt

@havogt havogt commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Targets sf_n_tracer_support (#2833) rather than main, because the bug is only reachable through the tuple comprehensions that PR introduces. Follow-up to this review comment.

There is a parallel PR, #2864, fixing the same bug a different way — via the symtable module instead of code-object introspection. They are alternatives; pick one and close the other. A comparison is at the bottom.

Problem

A name referenced only inside a tuple comprehension body is not collected as a closure variable, so type deduction rejects it — naming a symbol that is visibly imported at module scope:

@gtx.field_operator
def testee(tracers: tuple[EField, ...]) -> tuple[CField, ...]:
    return tuple(neighbor_sum(t(C2E), axis=C2EDim) for t in tracers)
# UndefinedSymbolError: Undeclared symbol 'neighbor_sum'

This covers every module-level name a real stencil uses: gt4py builtins, FieldOffsets, Dimensions, and module-level field operators — so the natural factoring tuple(_inner(t, ...) for t in tracers) does not work either.

Two things make it more than a limitation: the diagnostic names a symbol that is plainly in scope, and the workaround is to also reference the name outside the comprehension — so whether a stencil compiles depends on an unrelated line elsewhere in the function.

Cause

get_closure_vars_from_function uses inspect.getclosurevars, which disassembles the enclosing function's own code object and collects the names loaded by its LOAD_GLOBAL instructions. A generator expression compiles to its own code object, so the names it uses are recorded there and nowhere else:

outer co_names            : ('tuple',)
nested <genexpr> co_names : ('neighbor_sum', 'C2E', 'C2EDim')
collected closure vars    : ['tuple']

Free variables are unaffected, because closing over one forces a cell whose name is recorded on the enclosing code object as co_freevars. Globals need no cell, so nothing links them back. That asymmetry is also why the existing comprehension tests are green — they all define their helpers inside the test function, putting them on the working side of the bug.

Fix

Walk the nested code objects and scan each the same way inspect.getclosurevars scans the outer one: LOAD_GLOBAL instructions only, so attribute names — which also appear in co_names — are not mistaken for references to the enclosing namespace. Resolve against __globals__ then the builtins namespace, merged at the same precedence so nonlocals still win. The comprehension target is not collected, being a local of the nested code object rather than a global reference.

Tests

  • test_tuples.py: three integration tests placed next to test_tuple_comprehension_other_fo, so the contrast is visible in one screen — that test defines its helper inside the test (free variable, always worked), the new ones use a module-level operator and a module-level builtin. Plus the used-outside-too variant that documents the workaround.
  • test_tuple_comprehension_closure_vars.py: mechanism-level unit tests pinning that the names live in the nested code object, that the comprehension target is not collected, and that an enclosing local shadowing a module-level name is not picked up as a global.

All new tests fail without the source change.

Validation

  • tests/next_tests/unit_tests/ffront_tests/ + type_system_tests/: 443 passed, 2 skipped, 3 xfailed.
  • Comprehension tests in test_tuples.py green on roundtrip/numpy and on gtfn CPU, including skip_value_mesh.
  • uv run mypy src/: clean (361 files). pre-commit: all hooks pass.
  • Not exercised: Python 3.13/3.14 (fix[next]: collect closure vars via the compiler's scope analysis #2864 has been verified on all three), and any GPU backend.

Versus #2864

this PR #2864 (symtable)
Source of truth code objects, LOAD_GLOBAL CPython's symbol table of the source
Mechanism same one inspect.getclosurevars already uses, applied recursively symtable.Function.get_globals() per scope
Reads source again no yes (~0.3 ms one-time per function)
Annotation names excluded for free (unevaluated annotations emit no bytecode) excluded via a from __future__ import annotations prefix
Version exposure dis opnames symtable.Function classification + PEP 563
Verified on 3.12 / 3.13 / 3.14 3.12 only all three

https://claude.ai/code/session_01VR1cyTQ4wysovMBAwPBAWh

`get_closure_vars_from_function` used `inspect.getclosurevars`, which disassembles
the enclosing function's own code object and collects the names loaded by its
`LOAD_GLOBAL` instructions. A generator expression compiles to a separate code
object, so a name referenced only inside a tuple comprehension body is recorded
there and nowhere else, and was never collected. Type deduction then failed with
`UndefinedSymbolError` on a name that is visibly imported:

    @gtx.field_operator
    def testee(tracers: tuple[EField, ...]) -> tuple[CField, ...]:
        return tuple(neighbor_sum(t(C2E), axis=C2EDim) for t in tracers)
    # UndefinedSymbolError: Undeclared symbol 'neighbor_sum'

This affected every module-level name: builtins, `FieldOffset`s, `Dimension`s
and module-level field operators, so `tuple(_inner(t, ...) for t in tracers)`
did not work either.

Free variables were unaffected, because closing over one forces a cell whose
name is recorded on the enclosing code object as `co_freevars`. Globals need no
cell, so nothing links them back. That is also why the existing comprehension
tests pass: they all define their helpers inside the test function.

Nested code objects are scanned the same way `inspect.getclosurevars` scans the
outer one -- `LOAD_GLOBAL` only, so attribute names in `co_names` are not
mistaken for references to the enclosing namespace -- and resolved against
`__globals__` then the builtins namespace, merged at the same precedence so
nonlocals still win. The comprehension target is not collected, being a local of
the nested code object.

Claude-Session: https://claude.ai/code/session_01VR1cyTQ4wysovMBAwPBAWh
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.

1 participant