8000 New semantic analyzer: fix crash caused by leaking placeholders (#6845) · python/mypy@924de06 · GitHub
[go: up one dir, main page]

Skip to content

Commit 924de06

Browse files
authored
New semantic analyzer: fix crash caused by leaking placeholders (#6845)
This fixes a crash when processing the latest unittest stubs. Previously we didn't always defer a target when placeholder nodes were added to a symbol table. This could result in crashes.
1 parent c73c95c commit 924de06

File tree

4 files changed

+69
-13
lines changed

4 files changed

+69
-13
lines changed

mypy/newsemanal/semanal.py

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,6 @@ def add_implicit_module_attrs(self, file_node: MypyFile) -> None:
313313
self.add_symbol(name, var, None)
314314
else:
315315
self.add_symbol(name, PlaceholderNode(self.qualified_name(name), file_node), None)
316-
self.defer()
317316

318317
def prepare_file(self, file_node: MypyFile) -> None:
319318
"""Prepare a freshly parsed file for semantic analysis."""
@@ -848,7 +847,12 @@ def visit_class_def(self, defn: ClassDef) -> None:
848847
def analyze_class(self, defn: ClassDef) -> None:
849848
fullname = self.qualified_name(defn.name)
850849
if not defn.info and not self.is_core_builtin_class(defn):
851-
self.add_symbol(defn.name, PlaceholderNode(fullname, defn, True), defn)
850+
# Add placeholder so that self-references in base classes can be
851+
# resolved. We don't want this to cause a deferral, since if there
852+
# are no incomplete references, we'll replace this with a TypeInfo
853+
# before returning.
854+
self.add_symbol(defn.name, PlaceholderNode(fullname, defn, True), defn,
855+
can_defer=False)
852856

853857
tag = self.track_incomplete_refs()
854858

@@ -3900,7 +3904,11 @@ def lookup_qualified(self, name: str, ctx: Context,
39003904
return None
39013905

39023906
def defer(self) -> None:
3903-
"""Defer current analysis target to be analyzed again."""
3907+
"""Defer current analysis target to be analyzed again.
3908+
3909+
This must be called if something in the current target is
3910+
incomplete or has a placeholder node.
3911+
"""
39043912
self.deferred = True
39053913

39063914
def track_incomplete_refs(self) -> Tag:
@@ -4115,7 +4123,8 @@ def current_symbol_kind(self) -> int:
41154123
return kind
41164124

41174125
def add_symbol(self, name: str, node: SymbolNode, context: Optional[Context],
4118-
module_public: bool = True, module_hidden: bool = False) -> bool:
4126+
module_public: bool = True, module_hidden: bool = False,
4127+
can_defer: bool = True) -> bool:
41194128
"""Add symbol to the currently active symbol table.
41204129
41214130
Generally additions to symbol table should go through this method or
@@ -4124,6 +4133,8 @@ def add_symbol(self, name: str, node: SymbolNode, context: Optional[Context],
41244133
41254134
Return True if we actually added the symbol, or False if we refused to do so
41264135
(because something is not ready).
4136+
4137+
If can_defer is True, defer current target if adding a placeholder.
41274138
"""
41284139
if self.is_func_scope():
41294140
kind = LDEF
@@ -4135,7 +4146,7 @@ def add_symbol(self, name: str, node: SymbolNode, context: Optional[Context],
41354146
node,
41364147
module_public=module_public,
41374148
module_hidden=module_hidden)
4138-
return self.add_symbol_table_node(name, symbol, context)
4149+
return self.add_symbol_table_node(name, symbol, context, can_defer)
41394150

41404151
def add_symbol_skip_local(self, name: str, node: SymbolNode) -> None:
41414152
"""Same as above, but skipping the local namespace.
@@ -4175,21 +4186,26 @@ def is_global_or_nonlocal(self, name: str) -> bool:
41754186
or name in self.nonlocal_decls[-1]))
41764187

41774188
def add_symbol_table_node(self, name: str, symbol: SymbolTableNode,
4178-
context: Optional[Context] = None) -> bool:
4189+
context: Optional[Context] = None,
4190+
can_defer: bool = True) -> bool:
41794191
"""Add symbol table node to the currently active symbol table.
41804192
41814193
Return True if we actually added the symbol, or False if we refused to do so
41824194
(because something is not ready).
4195+
4196+
If can_defer is True, defer current target if adding a placeholder.
41834197
"""
41844198
names = self.current_symbol_table()
41854199
existing = names.get(name)
4200+
if isinstance(symbol.node, PlaceholderNode) and can_defer:
4201+
self.defer()
41864202
if (existing is not None
41874203
and context is not None
41884204
and (not isinstance(existing.node, PlaceholderNode)
4189-
or isinstance(symbol.node, PlaceholderNode) and
4190-
# Allow replacing becomes_typeinfo=False with becomes_typeinfo=True.
4191-
# This can happen for type aliases and NewTypes.
4192-
not symbol.node.becomes_typeinfo)):
4205+
or (isinstance(symbol.node, PlaceholderNode) and
4206+
# Allow replacing becomes_typeinfo=False with becomes_typeinfo=True.
4207+
# This can happen for type aliases and NewTypes.
4208+
not symbol.node.becomes_typeinfo))):
41934209
# There is an existing node, so this may be a redefinition.
41944210
# If the new node points to the same node as the old one,
41954211
# or if both old and new nodes are placeholders, we don't

mypy/newsemanal/semanal_newtype.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ def process_newtype_declaration(self, s: AssignmentStmt) -> bool:
5050
if (not call.analyzed or
5151
isinstance(call.analyzed, NewTypeExpr) and not call.analyzed.info):
5252
# Start from labeling this as a future class, as we do for normal ClassDefs.
53-
self.api.add_symbol(name, PlaceholderNode(fullname, s, becomes_typeinfo=True), s)
53+
self.api.add_symbol(name, PlaceholderNode(fullname, s, becomes_typeinfo=True), s,
54+
can_defer=False)
5455

5556
old_type, should_defer = self.check_newtype_args(name, call, s)
5657
if not call.analyzed:

mypy/newsemanal/semanal_shared.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,8 @@ def current_symbol_table(self) -> SymbolTable:
133133

134134
@abstractmethod
135135
def add_symbol(self, name: str, node: SymbolNode, context: Optional[Context],
136-
module_public: bool = True, module_hidden: bool = False) -> bool:
136+
module_public: bool = True, module_hidden: bool = False,
137+
can_defer: bool = True) -> bool:
137138
"""Add symbol to the current symbol table."""
138139
raise NotImplementedError
139140

test-data/unit/check-newsemanal.test

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1009,7 +1009,8 @@ class B: ...
10091009
import a
10101010
[file a.py]
10111011
C = 1
1012-
from b import * # E: Name 'C' already defined on line 1
1012+
from b import * # E: Name 'C' already defined on line 1 \
1013+
# E: Incompatible import of "C" (imported name has type "Type[C]", local name has type "int")
10131014

10141015
[file b.py]
10151016
import a
@@ -2200,3 +2201,40 @@ x = 1
22002201
# We want to check that the first definition creates the variable.
22012202
def x() -> None: ... # E: Name 'x' already defined on line 1
22022203
y = 2
2204+
2205+
[case testNewAnalyzerImportStarSpecialCase]
2206+
import unittest
2207+
[file unittest/__init__.pyi]
2208+
from unittest.suite import *
2209+
2210+
def load_tests() -> TestSuite: ...
2211+
[file unittest/suite.pyi]
2212+
from typing import Union # Iterable not imported
2213+
import unittest.case
2214+
2215+
_TestType = Union[unittest.case.TestCase]
2216+
2217+
class BaseTestSuite(Iterable[_TestType]):
2218+
...
2219+
2220+
class TestSuite(BaseTestSuite):
2221+
...
2222+
2223+
[file unittest/case.pyi]
2224+
class TestCase: ...
2225+
2226+
[out]
2227+
tmp/unittest/suite.pyi:6: error: Name 'Iterable' is not defined
2228+
tmp/unittest/suite.pyi:6: note: Did you forget to import it from "typing"? (Suggestion: "from typing import Iterable")
2229+
2230+
[case testNewAnalyzerNewTypeSpecialCase]
2231+
from typing import NewType
2232+
from typing_extensions import Final, Literal
2233+
2234+
X = NewType('X', int)
2235+
2236+
var1: Final = 1
2237+
2238+
def force1(x: Literal[1]) -> None: pass
2239+
2240+
force1(reveal_type(var1)) # E: Revealed type is 'Literal[1]'

0 commit comments

Comments
 (0)
0