Fixes mypy crash on protocol with contravariant var #11135

sobolevn · 2021-09-18T09:44:16Z

JelleZijlstra · 2021-09-18T22:58:49Z

mypy/constraints.py

            inst = mypy.subtypes.find_member(member, instance, subtype)
            temp = mypy.subtypes.find_member(member, template, subtype)
-            assert inst is not None and temp is not None
+            if inst is None or temp is None:


I'm not familiar with this code. Do you know why it is safe to ignore it?

This is a pretty old piece from initial protocols commit c55e48b

I was not sure if this is safe to ignore or not. All code / tests I've tried showed that nothing has changed.

But, from personal experience finding bugs in constraints solver is pretty hard.

We can ask @ilevkivskyi if he knows some examples / cases where this can backfire.

This is a basic consistency check. One should never get here for something that can never be a protocol implementation. So I don't think this fixes the actual issue. Couple other comments:

Variance in the place where it is used in the original repro should be totally irrelevant. Variance is only relevant for class definition type variables. It should be irrelevant in generic function/method type variables (including self-types). Variance is a property of a class, not a property of a type variable. If the repro depends on variance we have a much bigger problem.

I think the actual culprit is overload on self-type. This is a relatively new feature (much newer than protocols), and it requires some special handling in bind_self() (and/or somewhere around). I bet find_member() doesn't handle this logic correctly. So you should either fix that, or better refactor find_member() somehow to use the same logic.

sobolevn · 2021-09-20T10:52:09Z

This also happens with:

from typing import overload, Protocol, TypeVar

I = TypeVar('I')
V = TypeVar('V')

class C(Protocol[I]):  # E: Invariant type variable "I" used in protocol where covariant one is expected
    def __abs__(self: 'C[V]') -> 'C[V]':
        ...

    @overload
    def f(self, q: int) -> int:
        ...
    @overload
    def f(self, q: float) -> 'C[float]':
        ...

Related traceback:

self._visit_overloaded_func_def(defn)
  File "/Users/sobolev/Desktop/mypy/mypy/checker.py", line 469, in _visit_overloaded_func_def
    self.check_overlapping_overloads(defn)
  File "/Users/sobolev/Desktop/mypy/mypy/checker.py", line 533, in check_overlapping_overloads
    if is_unsafe_overlapping_overload_signatures(sig1, sig2):
  File "/Users/sobolev/Desktop/mypy/mypy/checker.py", line 5582, in is_unsafe_overlapping_overload_signatures
    return (is_callable_compatible(signature, other,
  File "/Users/sobolev/Desktop/mypy/mypy/subtypes.py", line 858, in is_callable_compatible
    unified = unify_generic_callable(left, right, ignore_return=ignore_return)
  File "/Users/sobolev/Desktop/mypy/mypy/subtypes.py", line 1087, in unify_generic_callable
    c = mypy.constraints.infer_constraints(
  File "/Users/sobolev/Desktop/mypy/mypy/constraints.py", line 101, in infer_constraints
    return _infer_constraints(template, actual, direction)
  File "/Users/sobolev/Desktop/mypy/mypy/constraints.py", line 174, in _infer_constraints
    return template.accept(ConstraintBuilderVisitor(actual, direction))
  File "/Users/sobolev/Desktop/mypy/mypy/types.py", line 855, in accept
    return visitor.visit_instance(self)
  File "/Users/sobolev/Desktop/mypy/mypy/constraints.py", line 409, in visit_instance
    mypy.subtypes.is_protocol_implementation(erased, instance)):
  File "/Users/sobolev/Desktop/mypy/mypy/subtypes.py", line 556, in is_protocol_implementation
    supertype = get_proper_type(find_member(member, right, left))
  File "/Users/sobolev/Desktop/mypy/mypy/subtypes.py", line 623, in find_member
    return find_node_type(method, itype, subtype)
  File "/Users/sobolev/Desktop/mypy/mypy/subtypes.py", line 713, in find_node_type
    signature = bind_self(typ, subtype)
  File "/Users/sobolev/Desktop/mypy/mypy/typeops.py", line 239, in bind_self
    typeargs = infer_type_arguments(all_ids, self_param_type, original_type,
  File "/Users/sobolev/Desktop/mypy/mypy/infer.py", line 45, in infer_type_arguments
    constraints = infer_constraints(template, actual,
  File "/Users/sobolev/Desktop/mypy/mypy/constraints.py", line 101, in infer_constraints
    return _infer_constraints(template, actual, direction)
  File "/Users/sobolev/Desktop/mypy/mypy/constraints.py", line 174, in _infer_constraints
    return template.accept(ConstraintBuilderVisitor(actual, direction))
  File "/Users/sobolev/Desktop/mypy/mypy/types.py", line 855, in accept
    return visitor.visit_instance(self)
  File "/Users/sobolev/Desktop/mypy/mypy/constraints.py", line 399, in visit_instance
    mypy.subtypes.is_protocol_implementation(instance, erased)):
  File "/Users/sobolev/Desktop/mypy/mypy/subtypes.py", line 556, in is_protocol_implementation
    supertype = get_proper_type(find_member(member, right, left))
  File "/Users/sobolev/Desktop/mypy/mypy/subtypes.py", line 623, in find_member
    return find_node_type(method, itype, subtype)
  File "/Users/sobolev/Desktop/mypy/mypy/subtypes.py", line 713, in find_node_type
    signature = bind_self(typ, subtype)
  File "/Users/sobolev/Desktop/mypy/mypy/typeops.py", line 239, in bind_self
    typeargs = infer_type_arguments(all_ids, self_param_type, original_type,
  File "/Users/sobolev/Desktop/mypy/mypy/infer.py", line 45, in infer_type_arguments
    constraints = infer_constraints(template, actual,
  File "/Users/sobolev/Desktop/mypy/mypy/constraints.py", line 101, in infer_constraints
    return _infer_constraints(template, actual, direction)
  File "/Users/sobolev/Desktop/mypy/mypy/constraints.py", line 174, in _infer_constraints
    return template.accept(ConstraintBuilderVisitor(actual, direction))
  File "/Users/sobolev/Desktop/mypy/mypy/types.py", line 855, in accept
    return visitor.visit_instance(self)
  File "/Users/sobolev/Desktop/mypy/mypy/constraints.py", line 401, in visit_instance
    self.infer_constraints_from_protocol_members(res, instance, template,
  File "/Users/sobolev/Desktop/mypy/mypy/constraints.py", line 449, in infer_constraints_from_protocol_members
    assert inst is not None and temp is not None
AssertionError: 
ex.py:10: : note: use --pdb to drop into pdb

sobolevn · 2021-09-20T11:14:23Z

This also fails:

from typing import overload, Protocol, TypeVar

I = TypeVar('I')
V = TypeVar('V')

class C(Protocol[I]): 
    def __abs__(self: 'C[V]') -> 'C[V]':
        ...

class D:
    @overload
    def f(self, q: int) -> int:
        ...
    @overload
    def f(self, q: float) -> 'C[float]':
        ...

x: C = D()

Traceback is similar:

  File "/Users/sobolev/Desktop/mypy/mypy/constraints.py", line 174, in _infer_constraints
    return template.accept(ConstraintBuilderVisitor(actual, direction))
  File "/Users/sobolev/Desktop/mypy/mypy/types.py", line 855, in accept
    return visitor.visit_instance(self)
  File "/Users/sobolev/Desktop/mypy/mypy/constraints.py", line 401, in visit_instance
    self.infer_constraints_from_protocol_members(res, instance, template,
  File "/Users/sobolev/Desktop/mypy/mypy/constraints.py", line 449, in infer_constraints_from_protocol_members
    assert inst is not None and temp is not None
AssertionError:

ilevkivskyi · 2021-09-20T11:27:25Z

Yeah, it looks like it fails for generic self-types in general:

from typing import Protocol, TypeVar

I = TypeVar('I', covariant=True)
V = TypeVar('V')

class C(Protocol[I]): 
    def g(self: 'C[V]') -> 'C[V]':
        ...

class D:
    pass

x: C = D()

Btw IIRC they were implemented approx. at the same time as overload on self-types. So the problems happen with all "tricky" self-types.

ilevkivskyi · 2021-09-20T12:06:41Z

I think I understand the problem: find_member() calls bind_self() (the latter requires inference), and the former call in is_protocol_implementation() occurs already under newly-added assumption. So after all the proposed "naive" solution in this PR may be actually OK. I would go even further, and say that maybe we should drop all inferred constraints (instead of skipping) if there is at least one None, but not 100% sure about this.

sobolevn · 2021-09-20T12:09:01Z

@ilevkivskyi you are ahead of me 🙂
That's exactly the PR I'm making right now.

Copy paste from my commit comment:

 Fixes how infer_constraints_from_protocol_members works.

Here's what was wrong:
1. When checking if some type is a subtype of a protocol, we iterate over all its members
2. When we are inside `is_protocol_implementation` we set `.assuming` or `.assuming_proper` attributes and continue to check the other members
3. While checking methods with annotated `self` type with a protocol itself, we recurse into `is_protocol_implementation` once again
4. It always returns `True` in this case, because `assuming` is set in the context above
5. Because `is_protocol_implementation` is set to `True`, we dive into `infer_constraints_from_protocol_members` with a type, which is not really a protocol implementation
6. Sanity check breaks!

Solution: return empty constraints instead of raising `AssertionError`

Here's what was wrong: 1. When checking if some type is a subtype of a protocol, we iterate over all its members 2. When we are inside `is_protocol_implementation` we set `.assuming` or `.assuming_proper` attributes and continue to check the other members 3. While checking methods with annotated `self` type with a protocol itself, we recurse into `is_protocol_implementation` once again 4. It always returns `True` in this case, because `assuming` is set in the context above 5. Because `is_protocol_implementation` is set to `True`, we dive into `infer_constraints_from_protocol_members` with a type, which is not really a protocol implementation 6. Sanity check breaks! Solution: return empty constraints instead of raising `AssertionError`

sobolevn · 2021-09-20T12:55:16Z

Thanks, @ilevkivskyi! Your help helped a lot 🙂

Fixes mypy crash on protocol with contravariant var

2b8617b

JelleZijlstra reviewed Sep 18, 2021

View reviewed changes

sobolevn added 2 commits September 20, 2021 15:14

Adds positive case

e5f80ca

ilevkivskyi approved these changes Sep 20, 2021

View reviewed changes

JukkaL merged commit 23e02e2 into python:master Sep 21, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fixes mypy crash on protocol with contravariant var #11135

Fixes mypy crash on protocol with contravariant var #11135

Uh oh!

sobolevn commented Sep 18, 2021

Uh oh!

JelleZijlstra Sep 18, 2021

Uh oh!

sobolevn Sep 18, 2021 •

edited

Loading

Uh oh!

ilevkivskyi Sep 19, 2021

Uh oh!

sobolevn commented Sep 20, 2021 •

edited

Loading

Uh oh!

sobolevn commented Sep 20, 2021 •

edited

Loading

Uh oh!

ilevkivskyi commented Sep 20, 2021

Uh oh!

ilevkivskyi commented Sep 20, 2021

Uh oh!

sobolevn commented Sep 20, 2021 •

edited

Loading

Uh oh!

sobolevn commented Sep 20, 2021 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Fixes mypy crash on protocol with contravariant var #11135

Fixes mypy crash on protocol with contravariant var #11135

Uh oh!

Conversation

sobolevn commented Sep 18, 2021

Uh oh!

JelleZijlstra Sep 18, 2021

Choose a reason for hiding this comment

Uh oh!

sobolevn Sep 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ilevkivskyi Sep 19, 2021

Choose a reason for hiding this comment

Uh oh!

sobolevn commented Sep 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sobolevn commented Sep 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ilevkivskyi commented Sep 20, 2021

Uh oh!

ilevkivskyi commented Sep 20, 2021

Uh oh!

sobolevn commented Sep 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sobolevn commented Sep 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

sobolevn Sep 18, 2021 •

edited

Loading

sobolevn commented Sep 20, 2021 •

edited

Loading

sobolevn commented Sep 20, 2021 •

edited

Loading

sobolevn commented Sep 20, 2021 •

edited

Loading

sobolevn commented Sep 20, 2021 •

edited

Loading