Skip to content

Commit ada49c2

Browse files
JukkaLilevkivskyi
authored andcommitted
Refactoring: Make the state of type forward references explicit (#4092)
Forward references can be either unbound or resolved. Also ensure that each forward reference is only resolved at most once. As a semi-related change, be more careful about forward references leaking after semantic analysis.
1 parent b98ebf0 commit ada49c2

File tree

6 files changed

+53
-23
lines changed

6 files changed

+53
-23
lines changed

mypy/indirection.py

+4-1
Original file line numberDiff line numberDiff line change
@@ -103,4 +103,7 @@ def visit_type_type(self, t: types.TypeType) -> Set[str]:
103103
return self._visit(t.item)
104104

105105
def visit_forwardref_type(self, t: types.ForwardRef) -> Set[str]:
106-
return self._visit(t.link)
106+
if t.resolved:
107+
return self._visit(t.resolved)
108+
else:
109+
return set()

mypy/messages.py

+4-1
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,10 @@ def format_bare(self, typ: Type, verbosity: int = 0) -> str:
310310
elif isinstance(typ, TypeType):
311311
return 'Type[{}]'.format(self.format_bare(typ.item, verbosity))
312312
elif isinstance(typ, ForwardRef): # may appear in semanal.py
313-
return self.format_bare(typ.link, verbosity)
313+
if typ.resolved:
314+
return self.format_bare(typ.resolved, verbosity)
315+
else:
316+
return self.format_bare(typ.unbound, verbosity)
314317
elif isinstance(typ, FunctionLike):
315318
func = typ
316319
if func.is_type_obj():

mypy/semanal.py

+3-1
Original file line numberDiff line numberDiff line change
@@ -4836,7 +4836,9 @@ def visit_forwardref_type(self, t: ForwardRef) -> Type:
48364836
# its content is updated in ThirdPass, now we need to unwrap this type.
48374837
A = NewType('A', int)
48384838
"""
4839-
return t.link.accept(self)
4839+
assert t.resolved, 'Internal error: Unresolved forward reference: {}'.format(
4840+
t.unbound.name)
4841+
return t.resolved.accept(self)
48404842

48414843
def visit_instance(self, t: Instance, from_fallback: bool = False) -> Type:
48424844
"""This visitor method tracks situations like this:

mypy/server/deps.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ def visit_type_type(self, typ: TypeType) -> List[str]:
213213
return []
214214

215215
def visit_forwardref_type(self, typ: ForwardRef) -> List[str]:
216-
return get_type_dependencies(typ.link)
216+
assert False, 'Internal error: Leaked forward reference object {}'.format(typ)
217217

218218
def visit_type_var(self, typ: TypeVarType) -> List[str]:
219219
# TODO: replace with actual implementation

mypy/typeanal.py

+12-8
Original file line numberDiff line numberDiff line change
@@ -704,8 +704,8 @@ def visit_instance(self, t: Instance) -> None:
704704
arg_values = [arg]
705705
self.check_type_var_values(info, arg_values, tvar.name, tvar.values, i + 1, t)
706706
# TODO: These hacks will be not necessary when this will be moved to later stage.
707-
arg = self.update_type(arg)
708-
bound = self.update_type(tvar.upper_bound)
707+
arg = self.resolve_type(arg)
708+
bound = self.resolve_type(tvar.upper_bound)
709709
if not is_subtype(arg, bound):
710710
self.fail('Type argument "{}" of "{}" must be '
711711
'a subtype of "{}"'.format(
@@ -719,9 +719,10 @@ def visit_instance(self, t: Instance) -> None:
719719
def check_type_var_values(self, type: TypeInfo, actuals: List[Type], arg_name: str,
720720
valids: List[Type], arg_number: int, context: Context) -> None:
721721
for actual in actuals:
722-
actual = self.update_type(actual)
722+
actual = self.resolve_type(actual)
723723
if (not isinstance(actual, AnyType) and
724-
not any(is_same_type(actual, self.update_type(value)) for value in valids)):
724+
not any(is_same_type(actual, self.resolve_type(value))
725+
for value in valids)):
725726
if len(actuals) > 1 or not isinstance(actual, Instance):
726727
self.fail('Invalid type argument value for "{}"'.format(
727728
type.name()), context)
@@ -731,11 +732,13 @@ def check_type_var_values(self, type: TypeInfo, actuals: List[Type], arg_name: s
731732
self.fail(messages.INCOMPATIBLE_TYPEVAR_VALUE.format(
732733
arg_name, class_name, actual_type_name), context)
733734

734-
def update_type(self, tp: Type) -> Type:
735+
def resolve_type(self, tp: Type) -> Type:
735736
# This helper is only needed while is_subtype and is_same_type are
736737
# called in third pass. This can be removed when TODO in visit_instance is fixed.
737738
if isinstance(tp, ForwardRef):
738-
tp = tp.link
739+
if tp.resolved is None:
740+
return tp.unbound
741+
tp = tp.resolved
739742
if isinstance(tp, Instance) and tp.type.replaced:
740743
replaced = tp.type.replaced
741744
if replaced.tuple_type:
@@ -799,8 +802,9 @@ def visit_type_type(self, t: TypeType) -> None:
799802

800803
def visit_forwardref_type(self, t: ForwardRef) -> None:
801804
self.indicator['forward'] = True
802-
if isinstance(t.link, UnboundType):
803-
t.link = self.anal_type(t.link)
805+
if t.resolved is None:
806+
resolved = self.anal_type(t.unbound)
807+
t.resolve(resolved)
804808

805809
def anal_type(self, tp: UnboundType) -> Type:
806810
tpan = TypeAnalyser(self.lookup_func,

mypy/types.py

+29-11
Original file line numberDiff line numberDiff line change
@@ -1390,21 +1390,33 @@ class ForwardRef(Type):
13901390
So that ForwardRefs are temporary and will be completely replaced with the linked types
13911391
or Any (to avoid cyclic references) before the type checking stage.
13921392
"""
1393-
link = None # type: Type # The wrapped type
1393+
_unbound = None # type: UnboundType # The original wrapped type
1394+
_resolved = None # type: Optional[Type] # The resolved forward reference (initially None)
13941395

1395-
def __init__(self, link: Type) -> None:
1396-
self.link = link
1396+
def __init__(self, unbound: UnboundType) -> None:
1397+
self._unbound = unbound
1398+
self._resolved = None
1399+
1400+
@property
1401+
def unbound(self) -> UnboundType:
1402+
# This is read-only to make it clear that resolution happens through resolve().
1403+
return self._unbound
1404+
1405+
@property
1406+
def resolved(self) -> Optional[Type]:
1407+
# Similar to above.
1408+
return self._resolved
1409+
1410+
def resolve(self, resolved: Type) -> None:
1411+
"""Resolve an unbound forward reference to point to a type."""
1412+
assert self._resolved is None
1413+
self._resolved = resolved
13971414

13981415
def accept(self, visitor: 'TypeVisitor[T]') -> T:
13991416
return visitor.visit_forwardref_type(self)
14001417

14011418
def serialize(self):
1402-
if isinstance(self.link, UnboundType):
1403-
name = self.link.name
1404-
if isinstance(self.link, Instance):
1405-
name = self.link.type.name()
1406-
else:
1407-
name = self.link.__class__.__name__
1419+
name = self.unbound.name
14081420
# We should never get here since all forward references should be resolved
14091421
# and removed during semantic analysis.
14101422
assert False, "Internal error: Unresolved forward reference to {}".format(name)
@@ -1749,7 +1761,10 @@ def visit_type_type(self, t: TypeType) -> str:
17491761
return 'Type[{}]'.format(t.item.accept(self))
17501762

17511763
def visit_forwardref_type(self, t: ForwardRef) -> str:
1752-
return '~{}'.format(t.link.accept(self))
1764+
if t.resolved:
1765+
return '~{}'.format(t.resolved.accept(self))
1766+
else:
1767+
return '~{}'.format(t.unbound.accept(self))
17531768

17541769
def list_str(self, a: List[Type]) -> str:
17551770
"""Convert items of an array to strings (pretty-print types)
@@ -1831,7 +1846,10 @@ def visit_type_type(self, t: TypeType) -> T:
18311846
return t.item.accept(self)
18321847

18331848
def visit_forwardref_type(self, t: ForwardRef) -> T:
1834-
return t.link.accept(self)
1849+
if t.resolved:
1850+
return t.resolved.accept(self)
1851+
else:
1852+
return t.unbound.accept(self)
18351853

18361854
def visit_ellipsis_type(self, t: EllipsisType) -> T:
18371855
return self.strategy([])

0 commit comments

Comments
 (0)