fix[next]: resolve closure variable references by value - #2865
Draft
havogt wants to merge 4 commits into
Draft
Conversation
References to builtins and field operators are recognized by their source-level name in the later frontend passes, so a builtin referenced through a module (`gtx.where`) or under an alias (`my_where = where`), an operator referenced through a module (`helpers.helper`) or under an alias (`helper as h2`), and a scalar module attribute (`np.pi`) all failed, most of them with internal errors late in the pipeline. Extend `ClosureVarFolding` to resolve such references by value: scalar module attributes are folded into constants, builtins are rewritten to their builtin name, and module-prefixed operators to a name derived from the operator's definition, registered as additional closure variables. The program lowering registers each callable under every name it is referenced by, which makes aliases work without rewriting and retires the misnamed-function lint. `typing.Final` scalars and program-level module-prefixed calls are left as they are. Claude-Session: https://claude.ai/code/session_01Sr9xdMdZgG4wjLLAKbweYb
…irst Direct references to a field operator were rewritten to the synthesized, definition-derived name as well, which changed every call site's name in the FOAST and IR. Only references reached through a module get a synthesized name. The shadowing check ran after the closure-variable lookup, so a name already in the closure variables was accepted even when a local of the same name existed. On Python 3.12.3 `inspect.getclosurevars` still collects attribute names from `co_names`, which is how `gtx.where` put `where` there in CI. Check for a shadowing local or parameter first. Claude-Session: https://claude.ai/code/session_01Sr9xdMdZgG4wjLLAKbweYb
…ames Review follow-ups: a FOAST-level test that direct and aliased operator references are left alone and that module-prefixed references resolve as intended; the two FrozenNamespace/enum integration tests that the rewrite of test_closure_vars.py had dropped are restored. The synthesized name for a module-prefixed operator is now the definition name plus a hash of its source file and qualified name: the previous scheme mapped `.` and `_` to the same character, so distinct modules could collide, and it embedded `__module__`, which differs between running and importing a script. The builtin lookup is a linear identity scan instead of an id-keyed dict. Claude-Session: https://claude.ai/code/session_01Sr9xdMdZgG4wjLLAKbweYb
…t the file path Hashing `co_filename` made the symbol for a module-prefixed operator depend on where the checkout lives, and the symbol reaches the IR and the on-disk cache keys. Hash `__module__.__qualname__` instead; a test pins that a definition moved to another path keeps its name. Claude-Session: https://claude.ai/code/session_01Sr9xdMdZgG4wjLLAKbweYb
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.
References to builtins and field operators are recognized by their source-level name in the later frontend passes, so a builtin referenced through a module (
gtx.where) or under an alias (my_where = where), an operator referenced through a module (helpers.helper) or under an alias (helper as h2), and a scalar module attribute (np.pi) all failed, most of them with internal errors late in the pipeline.Extend
ClosureVarFoldingto resolve such references by value: scalar module attributes are folded into constants, builtins are rewritten to their builtin name, and module-prefixed operators to a name derived from the operator's definition, registered as additional closure variables. The program lowering registers each callable under every name it is referenced by, which makes aliases work without rewriting and retires the misnamed-function lint.