Fix crash on invalid starred assignment target - #21851
Open
TheMuffinMan1320 wants to merge 1 commit into
Open
Conversation
`*a = b` (a bare starred assignment target, not wrapped in a list or tuple) is invalid, and semantic analysis already reports "Starred assignment target must be in a list or tuple" for it. However, analyze_lvalue() returned without recursing into the inner name of the StarExpr, so if that name had earlier been registered as a PlaceholderNode (e.g. because the right-hand side referenced an as-yet-undefined name and analysis had to be deferred), the placeholder was never replaced with a real definition. This caused a later crash when writing the incremental cache: `NotImplementedError: Cannot serialize PlaceholderNode instance`. Fix by still analyzing the inner lvalue after reporting the error, so any placeholder gets resolved like it would for a valid nested star target. Fixes python#20796
Contributor
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
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.
Fixes #20796
Root cause
*a = bis invalid Python (a bare starred assignment target must bewrapped in a list or tuple, e.g.
*a, = b). Semantic analysisalready detects this and reports:
However, in
analyze_lvalue()(mypy/semanal.py), the branchhandling a non-nested
StarExprlvalue only calledself.fail(...)and returned, without analyzing the inner name expression
(
lval.expr).If the right-hand side of the assignment referenced an
as-yet-undefined name, semantic analysis has to defer the statement
for a later pass. As part of that deferral,
mark_incomplete()proactively registers the starred target's name (
a) in the symboltable as a
PlaceholderNode, expecting it to later be replaced witha real definition once the lvalue is fully analyzed.
Because the non-nested-
StarExprbranch never analyzedlval.expr,that placeholder was never resolved into a real
Var. Later, whenmypy tried to write the incremental cache, serialization walked the
module's symbol table and hit the leftover
PlaceholderNode,crashing with:
Fix
mypy/semanal.py: still analyze the inner lvalue (lval.expr) afterreporting the "Starred assignment target must be in a list or tuple"
error, matching what already happens for a valid nested star target.
This lets any pending placeholder for the name get resolved normally,
while the invalid-target error is still reported.
Test
Added
testInvalidBareStarAssignmentTargetNoCrashintest-data/unit/check-statements.test, using--debug-serialize(theexisting mechanism used by a few other crash-regression tests, e.g.
testNamedTupleDefaultValueDefer) to exercise serialization evenwithout writing an on-disk cache. Verified this test crashes with the
reported
NotImplementedErrorwhen the fix is reverted, and passeswith the fix.
Verification
No more crash, and the two errors are reported as expected.
python -m pytest -q mypy/test/testcheck.py -k check-statements— 172 passedpython -m pytest -q mypy/test/testsemanal.py— 577 passedpython -m mypy --config-file mypy_self_check.ini mypy/semanal.py— success