Skip to content

Commit dcf910e

Browse files
authored
Fix crash in daemon mode on new import cycle (#14508)
Fixes #14329 This fixes the second crash reported in the issue (other one is already fixed). This one is tricky, it looks like it happens only when we bring in a new import cycle in an incremental update with `--follow-import=normal`. To explain the reason, a little reminder of how semantic analyzer passes work: * Originally, we recorded progress automatically when some new symbol was resolved and added to symbol tables. * With implementation of recursive types, this mechanism was insufficient, as recursive types require modifying some symbols _in place_, this is why `force_progress` flag was added to `defer()`. * I was only careful with this flag for recursive type aliases (that were implemented first), for other things (like recursive TypedDicts, etc) I just always passed `force_progress=True` (without checking if we actually resolved some placeholder types). * The reasoning for that is if we ever add `becomes_typeinfo=True`, there is no way this symbol will later be unresolved (otherwise how would we know this is something that is a type). * It turns out this reasoning doesn't work in some edge cases in daemon mode, we do put some placeholders with `becomes_typeinfo=True` for symbols imported from modules that were not yet processed, thus causing a crash (see test cases). * There were two options to fix this: one is to stop creating placeholders with `becomes_typeinfo=True` for unimported symbols in daemon mode, other one is to always carefully check if in-place update of a symbol actually resulted in progress. * Ultimately I decided that the first way is too fragile (and I don't understand how import following works for daemon anyway), and the second way is something that is technically correct anyway, so here is this PR I didn't add test cases for each of the crash scenarios, since they are all very similar. I only added two that I encountered "in the wild", upper bound and tuple base caused actual crash in `trio` stubs, plus also randomly a test for a TypedDict crash. _EDIT:_ and one more thing, the "cannot resolve name" error should never appear in normal mode, only in daemon update (see reasoning above), so I don't make those error messages detailed, just add some minimal info if we will need to debug them.
1 parent 9bbb93c commit dcf910e

File tree

7 files changed

+126
-11
lines changed

7 files changed

+126
-11
lines changed

mypy/semanal.py

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1049,7 +1049,12 @@ def setup_self_type(self) -> None:
10491049
if info.self_type is not None:
10501050
if has_placeholder(info.self_type.upper_bound):
10511051
# Similar to regular (user defined) type variables.
1052-
self.defer(force_progress=True)
1052+
self.process_placeholder(
1053+
None,
1054+
"Self upper bound",
1055+
info,
1056+
force_progress=info.self_type.upper_bound != fill_typevars(info),
1057+
)
10531058
else:
10541059
return
10551060
info.self_type = TypeVarType("Self", f"{info.fullname}.Self", 0, [], fill_typevars(info))
@@ -2132,7 +2137,9 @@ def configure_tuple_base_class(self, defn: ClassDef, base: TupleType) -> Instanc
21322137
self.fail("Class has two incompatible bases derived from tuple", defn)
21332138
defn.has_incompatible_baseclass = True
21342139
if info.special_alias and has_placeholder(info.special_alias.target):
2135-
self.defer(force_progress=True)
2140+
self.process_placeholder(
2141+
None, "tuple base", defn, force_progress=base != info.tuple_type
2142+
)
21362143
info.update_tuple_type(base)
21372144
self.setup_alias_type_vars(defn)
21382145

@@ -3913,12 +3920,16 @@ def process_typevar_declaration(self, s: AssignmentStmt) -> bool:
39133920
type_var = TypeVarExpr(name, self.qualified_name(name), values, upper_bound, variance)
39143921
type_var.line = call.line
39153922
call.analyzed = type_var
3923+
updated = True
39163924
else:
39173925
assert isinstance(call.analyzed, TypeVarExpr)
3926+
updated = values != call.analyzed.values or upper_bound != call.analyzed.upper_bound
39183927
call.analyzed.upper_bound = upper_bound
39193928
call.analyzed.values = values
39203929
if any(has_placeholder(v) for v in values) or has_placeholder(upper_bound):
3921-
self.defer(force_progress=True)
3930+
self.process_placeholder(
3931+
None, f"TypeVar {'values' if values else 'upper bound'}", s, force_progress=updated
3932+
)
39223933

39233934
self.add_symbol(name, call.analyzed, s)
39243935
return True
@@ -5931,7 +5942,9 @@ def is_incomplete_namespace(self, fullname: str) -> bool:
59315942
"""
59325943
return fullname in self.incomplete_namespaces
59335944

5934-
def process_placeholder(self, name: str, kind: str, ctx: Context) -> None:
5945+
def process_placeholder(
5946+
self, name: str | None, kind: str, ctx: Context, force_progress: bool = False
5947+
) -> None:
59355948
"""Process a reference targeting placeholder node.
59365949
59375950
If this is not a final iteration, defer current node,
@@ -5943,10 +5956,11 @@ def process_placeholder(self, name: str, kind: str, ctx: Context) -> None:
59435956
if self.final_iteration:
59445957
self.cannot_resolve_name(name, kind, ctx)
59455958
else:
5946-
self.defer(ctx)
5959+
self.defer(ctx, force_progress=force_progress)
59475960

5948-
def cannot_resolve_name(self, name: str, kind: str, ctx: Context) -> None:
5949-
self.fail(f'Cannot resolve {kind} "{name}" (possible cyclic definition)', ctx)
5961+
def cannot_resolve_name(self, name: str | None, kind: str, ctx: Context) -> None:
5962+
name_format = f' "{name}"' if name else ""
5963+
self.fail(f"Cannot resolve {kind}{name_format} (possible cyclic definition)", ctx)
59505964
if not self.options.disable_recursive_aliases and self.is_func_scope():
59515965
self.note("Recursive types are not allowed at function scope", ctx)
59525966

mypy/semanal_namedtuple.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,9 @@ def build_namedtuple_typeinfo(
501501
info.is_named_tuple = True
502502
tuple_base = TupleType(types, fallback)
503503
if info.special_alias and has_placeholder(info.special_alias.target):
504-
self.api.defer(force_progress=True)
504+
self.api.process_placeholder(
505+
None, "NamedTuple item", info, force_progress=tuple_base != info.tuple_type
506+
)
505507
info.update_tuple_type(tuple_base)
506508
info.line = line
507509
# For use by mypyc.

mypy/semanal_newtype.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -249,10 +249,16 @@ def build_newtype_typeinfo(
249249
init_func = FuncDef("__init__", args, Block([]), typ=signature)
250250
init_func.info = info
251251
init_func._fullname = info.fullname + ".__init__"
252+
if not existing_info:
253+
updated = True
254+
else:
255+
previous_sym = info.names["__init__"].node
256+
assert isinstance(previous_sym, FuncDef)
257+
updated = old_type != previous_sym.arguments[1].variable.type
252258
info.names["__init__"] = SymbolTableNode(MDEF, init_func)
253259

254-
if has_placeholder(old_type) or info.tuple_type and has_placeholder(info.tuple_type):
255-
self.api.defer(force_progress=True)
260+
if has_placeholder(old_type):
261+
self.api.process_placeholder(None, "NewType base", info, force_progress=updated)
256262
return info
257263

258264
# Helpers

mypy/semanal_shared.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,12 @@ def qualified_name(self, n: str) -> str:
232232
def is_typeshed_stub_file(self) -> bool:
233233
raise NotImplementedError
234234

235+
@abstractmethod
236+
def process_placeholder(
237+
self, name: str | None, kind: str, ctx: Context, force_progress: bool = False
238+
) -> None:
239+
raise NotImplementedError
240+
235241

236242
def set_callable_name(sig: Type, fdef: FuncDef) -> ProperType:
237243
sig = get_proper_type(sig)

mypy/semanal_typeddict.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,9 @@ def build_typeddict_typeinfo(
535535
info = existing_info or self.api.basic_new_typeinfo(name, fallback, line)
536536
typeddict_type = TypedDictType(dict(zip(items, types)), required_keys, fallback)
537537
if info.special_alias and has_placeholder(info.special_alias.target):
538-
self.api.defer(force_progress=True)
538+
self.api.process_placeholder(
539+
None, "TypedDict item", info, force_progress=typeddict_type != info.typeddict_type
540+
)
539541
info.update_typeddict_type(typeddict_type)
540542
return info
541543

mypy/types.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2857,6 +2857,14 @@ def accept(self, visitor: TypeVisitor[T]) -> T:
28572857
assert isinstance(visitor, SyntheticTypeVisitor)
28582858
return cast(T, visitor.visit_placeholder_type(self))
28592859

2860+
def __hash__(self) -> int:
2861+
return hash((self.fullname, tuple(self.args)))
2862+
2863+
def __eq__(self, other: object) -> bool:
2864+
if not isinstance(other, PlaceholderType):
2865+
return NotImplemented
2866+
return self.fullname == other.fullname and self.args == other.args
2867+
28602868
def serialize(self) -> str:
28612869
# We should never get here since all placeholders should be replaced
28622870
# during semantic analysis.

test-data/unit/fine-grained-follow-imports.test

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -769,3 +769,80 @@ from . import mod3
769769
==
770770
main.py:1: error: Cannot find implementation or library stub for module named "pkg"
771771
main.py:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
772+
773+
[case testNewImportCycleTypeVarBound]
774+
# flags: --follow-imports=normal
775+
# cmd: mypy main.py
776+
# cmd2: mypy other.py
777+
778+
[file main.py]
779+
# empty
780+
781+
[file other.py.2]
782+
import trio
783+
784+
[file trio/__init__.py.2]
785+
from typing import TypeVar
786+
import trio
787+
from . import abc as abc
788+
789+
T = TypeVar("T", bound=trio.abc.A)
790+
791+
[file trio/abc.py.2]
792+
import trio
793+
class A: ...
794+
[out]
795+
==
796+
797+
[case testNewImportCycleTupleBase]
798+
# flags: --follow-imports=normal
799+
# cmd: mypy main.py
800+
# cmd2: mypy other.py
801+
802+
[file main.py]
803+
# empty
804+
805+
[file other.py.2]
806+
import trio
807+
808+
[file trio/__init__.py.2]
809+
from typing import TypeVar, Tuple
810+
import trio
811+
from . import abc as abc
812+
813+
class C(Tuple[trio.abc.A, trio.abc.A]): ...
814+
815+
[file trio/abc.py.2]
816+
import trio
817+
class A: ...
818+
[builtins fixtures/tuple.pyi]
819+
[out]
820+
==
821+
822+
[case testNewImportCycleTypedDict]
823+
# flags: --follow-imports=normal
824+
# cmd: mypy main.py
825+
# cmd2: mypy other.py
826+
827+
[file main.py]
828+
# empty
829+
830+
[file other.py.2]
831+
import trio
832+
833+
[file trio/__init__.py.2]
834+
from typing import TypeVar
835+
from typing_extensions import TypedDict
836+
import trio
837+
from . import abc as abc
838+
839+
class C(TypedDict):
840+
x: trio.abc.A
841+
y: trio.abc.A
842+
843+
[file trio/abc.py.2]
844+
import trio
845+
class A: ...
846+
[builtins fixtures/dict.pyi]
847+
[out]
848+
==

0 commit comments

Comments
 (0)