Skip to content

[mypyc] Skip recursive setup for frame-private nested functions - #22187

Merged
p-sawicki merged 2 commits into
python:masterfrom
p-sawicki:fix-recursive-setup-private-frame
Oct 13, 2026
Merged

p-sawicki merged 2 commits into
python:masterfrom
p-sawicki:fix-recursive-setup-private-frame

Conversation

@p-sawicki

@p-sawicki p-sawicki commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

The recursive setup could cause AttributeError at runtime when a function contains both a frame-private as well as a captured nested function. The setup would add the frame-private function object as an attribute to the env class and access it even though it was never initialized.

Fix by omitting the recursive setup since it's not needed - a frame-private nested function can't call itself.

Also fix possibly inconsistent storage for a redefined nested function that is captured in one branch and not captured in another by using original_def in capture/free variable checks.

@p-sawicki p-sawicki changed the title [mypyc] Fix recursive setup for frame-private nested functions [mypyc] Skip recursive setup for frame-private nested functions Oct 12, 2026

@JukkaL JukkaL left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

AI found a regression:

Regression: a nested function stops seeing updates to its own name when original_def is a Var

If a nested function conditionally redefines a variable, get_func_target_symbol() returns that Var. setup_func_for_recursive_call() then copies the Var into a local register when the nested function starts. From then on, every read of the name inside the function uses that copy instead of the environment attribute. If the variable is rebound later (for example through nonlocal in another nested function), the nested function still calls the old value.

Before this PR, the copy was registered under the FuncDef. Name references resolve to the Var, so they kept going through the environment and saw the current value.

The copy is only made when the nested function itself contains a self-referencing nested function (add_nested_funcs_to_env), which is why k appears below:

from typing import Callable

def outer(flag: bool) -> list[int]:
    out: list[int] = []
    f: Callable[[int], None] = lambda n: out.append(-1)
    def swap() -> None:
        nonlocal f
        f = lambda n: out.append(100 + n)
    if flag:
        def f(n: int) -> None:
            def k(m: int) -> int:
                return k(m - 1) if m else 0
            out.append(n + k(2))
            if n == 0:
                swap()
                f(1)   # should call the lambda installed by swap()
    f(0)
    return out
outer(True)
Interpreted [0, 101]
master, compiled [0, 101]
this PR, compiled [0, 1]

If outer is a generator, master raises AttributeError: attribute 'f' of 'outer_env' undefined, and this PR gives the same wrong [0, 1].

Suggested fix: only make the copy when the storage symbol is a FuncDef. Reads of a Var already go through the environment:

     target_symbol = get_func_target_symbol(fdef)
     target = builder.symtables[-2].get(target_symbol)
     if (
-        not isinstance(target, AssignmentTargetAttr)
+        not isinstance(target_symbol, FuncDef)
+        or not isinstance(target, AssignmentTargetAttr)
         or not isinstance(target.obj.type, RInstance)
         or target.obj.type.class_ir is not prev_env
     ):
         return

With this change, the example gives [0, 101] for both the regular and the generator outer function. test_generator_spills.py, run-generators.test, run-functions.test and test_irbuild.py pass with it. The example above would make a good run test.

@p-sawicki
p-sawicki merged commit a9928e3 into python:master Oct 13, 2026
18 checks passed
@p-sawicki
p-sawicki deleted the fix-recursive-setup-private-frame branch October 13, 2026 11:45
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