Repository navigation
[mypyc] Skip recursive setup for frame-private nested functions - #22187
Conversation
JukkaL
left a comment
There was a problem hiding this comment.
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 outouter(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
):
returnWith 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.
The recursive setup could cause
AttributeErrorat 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_defin capture/free variable checks.