Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion mypy/checkmember.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ class B(A): pass
info = itype.type # type: TypeInfo
if isinstance(t, CallableType):
# TODO: Should we propagate type variable values?
tvars = [TypeVarDef(n, i + 1, None, builtin_type('builtins.object'), tv.variance)
tvars = [TypeVarDef(n, i + 1, [], builtin_type('builtins.object'), tv.variance)
for (i, n), tv in zip(enumerate(info.type_vars), info.defn.type_vars)]
if is_classmethod:
t = bind_self(t, original_type)
Expand Down
6 changes: 3 additions & 3 deletions mypy/fixup.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from mypy.types import (
CallableType, EllipsisType, Instance, Overloaded, TupleType, TypedDictType,
TypeList, TypeVarType, UnboundType, UnionType, TypeVisitor,
TypeType
TypeType, NOT_READY
)
from mypy.visitor import NodeVisitor

Expand Down Expand Up @@ -156,7 +156,7 @@ def visit_instance(self, inst: Instance) -> None:
# TODO: Is this needed or redundant?
# Also fix up the bases, just in case.
for base in inst.type.bases:
if base.type is None:
if base.type is NOT_READY:
base.accept(self)
for a in inst.args:
a.accept(self)
Expand Down Expand Up @@ -233,7 +233,7 @@ def visit_type_type(self, t: TypeType) -> None:


def lookup_qualified(modules: Dict[str, MypyFile], name: str,
quick_and_dirty: bool) -> SymbolNode:
quick_and_dirty: bool) -> Optional[SymbolNode]:
stnode = lookup_qualified_stnode(modules, name, quick_and_dirty)
if stnode is None:
return None
Expand Down
10 changes: 2 additions & 8 deletions mypy/maptype.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ def map_instance_to_supertypes(instance: Instance,
# FIX: Currently we should only have one supertype per interface, so no
# need to return an array
result = [] # type: List[Instance]
typ = instance.type
assert typ is not None, 'Instance.type is not fixed after deserialization'
for path in class_derivation_paths(typ, supertype):
for path in class_derivation_paths(instance.type, supertype):
types = [instance]
for sup in path:
a = [] # type: List[Instance]
Expand Down Expand Up @@ -60,7 +58,6 @@ def class_derivation_paths(typ: TypeInfo,

for base in typ.bases:
btype = base.type
assert btype is not None, 'Instance.type is not fixed after deserialization'
if btype == supertype:
result.append([btype])
else:
Expand All @@ -75,7 +72,6 @@ def map_instance_to_direct_supertypes(instance: Instance,
supertype: TypeInfo) -> List[Instance]:
# FIX: There should only be one supertypes, always.
typ = instance.type
assert typ is not None, 'Instance.type is not fixed after deserialization'
result = [] # type: List[Instance]

for b in typ.bases:
Expand Down Expand Up @@ -103,6 +99,4 @@ def instance_to_type_environment(instance: Instance) -> Dict[TypeVarId, Type]:
arguments. The type variables are mapped by their `id`.

"""
typ = instance.type
assert typ is not None, 'Instance.type is not fixed after deserialization'
return {binder.id: arg for binder, arg in zip(typ.defn.type_vars, instance.args)}
return {binder.id: arg for binder, arg in zip(instance.type.defn.type_vars, instance.args)}
11 changes: 11 additions & 0 deletions mypy/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -2208,6 +2208,17 @@ def deserialize(cls, data: JsonDict) -> 'TypeInfo':
return ti


class FakeInfo(TypeInfo):
# types.py defines a single instance of this class, called types.NOT_READY.
# This instance is used as a temporary placeholder in the process of de-serialization
# of 'Instance' types. The de-serialization happens in two steps: In the first step,
# Instance.type is set to NOT_READY. In the second step (in fixup.py) it is replaced by
# an actual TypeInfo. If you see the assertion error below, then most probably something
# went wrong during the second step and an 'Instance' that raised this error was not fixed.
def __getattr__(self, attr: str) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ilevkivskyi @gvanrossum This should probably be __getattribute__ instead. Now attributes defined in the body of TypeInfo will not trigger an AssertionError, as far as I know, and thus FakeInfo can mask some actual errors.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In any case __getattribute__ looks more robust. Will make a PR soon.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, but then I suspect that #3281 will come back. So we should at least have a test in place to verify it doesn't. (Because that's the kind of error that's being masked here -- and since in that particular case it was just getting the wrong full name of some type for some error message I'd rather have a poor error than a crash -- our users really don't like crashes.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gvanrossum
Yes, #3281 comes back, I tried your crash scenario and it indeed crashes (although in a different place). But it looks like Jukka's idea works! At least I have found two actual bugs in fixup.py, both related to metaclass de-serialization. After fixing those, your scenario from #3281 doe snot crash anymore. I will make a PR now so that you can try it.

raise AssertionError('De-serialization failure: TypeInfo not fixed')


class SymbolTableNode:
# Kind of node. Possible values:
# - LDEF: local definition (of any kind)
Expand Down
6 changes: 3 additions & 3 deletions mypy/semanal.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class SemanticAnalyzer(NodeVisitor):
# Nested block depths of scopes
block_depth = None # type: List[int]
# TypeInfo of directly enclosing class (or None)
type = None # type: TypeInfo
type = None # type: Optional[TypeInfo]
# Stack of outer classes (the second tuple item contains tvars).
type_stack = None # type: List[TypeInfo]
# Type variables that are bound by the directly enclosing class
Expand Down Expand Up @@ -1745,10 +1745,10 @@ def build_newtype_typeinfo(self, name: str, old_type: Type, base_type: Instance)
info.is_newtype = True

# Add __init__ method
args = [Argument(Var('cls'), NoneTyp(), None, ARG_POS),
args = [Argument(Var('self'), NoneTyp(), None, ARG_POS),
self.make_argument('item', old_type)]
signature = CallableType(
arg_types=[cast(Type, None), old_type],
arg_types=[Instance(info, []), old_type],
arg_kinds=[arg.kind for arg in args],
arg_names=['self', 'item'],
ret_type=old_type,
Expand Down
8 changes: 5 additions & 3 deletions mypy/subtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -323,9 +323,11 @@ def is_callable_subtype(left: CallableType, right: CallableType,

if left.variables:
# Apply generic type variables away in left via type inference.
left = unify_generic_callable(left, right, ignore_return=ignore_return)
if left is None:
unified = unify_generic_callable(left, right, ignore_return=ignore_return)
if unified is None:
return False
else:
left = unified

# Check return types.
if not ignore_return and not is_compat(left.ret_type, right.ret_type):
Expand Down Expand Up @@ -493,7 +495,7 @@ def are_args_compatible(


def unify_generic_callable(type: CallableType, target: CallableType,
ignore_return: bool) -> CallableType:
ignore_return: bool) -> Optional[CallableType]:
"""Try to unify a generic callable type with another callable type.

Return unified CallableType if successful; otherwise, return None.
Expand Down
8 changes: 4 additions & 4 deletions mypy/test/testtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,18 +75,18 @@ def test_tuple_type(self) -> None:
assert_equal(str(TupleType([self.x, AnyType()], None)), 'Tuple[X?, Any]')

def test_type_variable_binding(self) -> None:
assert_equal(str(TypeVarDef('X', 1, None, self.fx.o)), 'X')
assert_equal(str(TypeVarDef('X', 1, [], self.fx.o)), 'X')
assert_equal(str(TypeVarDef('X', 1, [self.x, self.y], self.fx.o)),
'X in (X?, Y?)')

def test_generic_function_type(self) -> None:
c = CallableType([self.x, self.y], [ARG_POS, ARG_POS], [None, None],
self.y, self.function, name=None,
variables=[TypeVarDef('X', -1, None, self.fx.o)])
variables=[TypeVarDef('X', -1, [], self.fx.o)])
assert_equal(str(c), 'def [X] (X?, Y?) -> Y?')

v = [TypeVarDef('Y', -1, None, self.fx.o),
TypeVarDef('X', -2, None, self.fx.o)]
v = [TypeVarDef('Y', -1, [], self.fx.o),
TypeVarDef('X', -2, [], self.fx.o)]
c2 = CallableType([], [], [], NoneTyp(), self.function, name=None, variables=v)
assert_equal(str(c2), 'def [Y, X] ()')

Expand Down
2 changes: 1 addition & 1 deletion mypy/typefixture.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ def make_type_info(self, name: str,
variance = variances[id - 1]
else:
variance = COVARIANT
v.append(TypeVarDef(n, id, None, self.o, variance=variance))
v.append(TypeVarDef(n, id, [], self.o, variance=variance))
class_def.type_vars = v

info = TypeInfo(SymbolTable(), class_def, module_name)
Expand Down
Loading