fix(serialization): find the function's symtable child by type, not by count (Python 3.14) - #698
Merged
JadenFiotto-Kaufman merged 1 commit intoAug 26, 2026
Conversation
…y count
`_function_referenced_names` filters a `def`'s payload down to the names its
body actually needs from outside it, so a name the function only ever binds
does not drag in the enclosing scope's same-named object. Its own docstring
gives the failure it exists to prevent:
a helper doing ``with open(path) as f:`` would drag in whatever ``f`` a
notebook cell left lying around, and a closed file can't be pickled at
all ("Cannot pickle closed files")
It located the function's symtable entry by counting children:
children = top.get_children()
if len(children) != 1:
return _referenced_names(source) # block rule -- captures locals
return globals_of(children[0])
A single module-level `def` does not produce a single child table on every
Python. Under PEP 649 (3.14+) it also gets an `__annotate__` table of type
`annotation`, whether or not the `def` is annotated:
>>> [(c.get_name(), c.get_type()) for c in symtable.symtable(
... "def f(x):\n return g(x)\n", "<s>", "exec").get_children()]
[('__annotate__', 'annotation'), ('f', 'function')]
So on 3.14 the guard is true for *every* `def`, the fallback runs every time,
and the filter is inert -- reinstating exactly the capture it was written to
stop. PEP 695 generics add a `type_parameters` table for the same reason.
`children[0]` was also the wrong table by then, so removing the guard alone
would not have been enough.
Select the one child whose `get_type()` is `"function"` instead, which covers
`def` and `lambda` and is stable across versions.
`pyproject.toml` lists 3.14 as supported, and the two tests that already
covered this (`test_function_locals_are_not_shipped`,
`test_function_globals_still_ship`) fail on it today -- CI runs 3.12, so
nothing surfaced it.
Adds `test_scope_filter_survives_extra_symtable_children`, parametrised over
plain / annotated / defaulted `def`s and a lambda, asserting the parameter
never ships; and `test_function_local_shadowing_an_unpicklable_global`, which
pins the original symptom end to end by putting a closed file handle in the
enclosing scope.
Without the fix, 6 of the 15 TestScopeFiltering tests fail on 3.14. With it,
the full CPU suite is green: 856 passed, 7 skipped.
Member
|
Perfect. Thank you @Hotragn |
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
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.
Rebased onto
0.8as requested in #696. This one is a bug I hit because I moved over — it is specific to 0.8 and not present ondev.Summary
_function_referenced_namesfilters adef's serialized payload down to the names its body actually needs from outside it. Its own docstring gives the reason:It located the function's
symtableentry by counting children:A single module-level
defdoes not produce a single child table on every Python. Under PEP 649 (3.14+) it also gets an__annotate__table — whether or not thedefis annotated:So on 3.14 the guard is true for every
def, the fallback runs every time, and the filter is inert — reinstating exactly the capture it exists to stop. PEP 695 generics add atype_parameterstable for the same reason. Notechildren[0]is also the wrong table by then, so dropping the guard alone would not have been enough.Why CI didn't catch it
The two tests that already cover this —
test_function_locals_are_not_shippedandtest_function_globals_still_ship— fail on 3.14 today:.github/workflows/python-app.ymlruns Python 3.12, where PEP 649 is not active, so nothing surfaced it.pyproject.tomllists 3.14 as supported (Programming Language :: Python :: 3.14), so this is in scope rather than a future concern.Fix
Select the one child whose
get_type()is"function". That coversdefandlambda, and is stable across versions — it does not care how many non-function tables a given Python decides to emit.Tests
test_scope_filter_survives_extra_symtable_children— parametrised over plain / annotated / defaulteddefand a lambda.xis the parameter, so it is local no matter what; if it ships, the filter was bypassed. (The lambda case passes without the fix — lambdas still yield one child — which is why it is worth pinning alongside the others.)test_function_local_shadowing_an_unpicklable_global— the original symptom end to end: a closed file handle in the enclosing scope, shadowed by awith ... as f:local. Capturing it doesn't merely bloat the payload; serializing raises.Result
Without the fix, 6 of 15
TestScopeFilteringtests fail on 3.14. With it, the full CPU suite is green:0.8@ 1f974f0, Python 3.14.3,transformers5.15.1,torch2.9.1, CPU. Also confirmed the fix is a no-op on the 3.12 path (one function table either way).