Skip to content

Inconsistent behavior depending on cache -- dict.get returning Union of key and value types #6509

Closed

Description

@pdeblanc

Python 3.7.2, mypy 0.670 (I also tried 70565cd, which was the most recent commit on master at the time of my testing).

File foo.py:

from typing import Dict, Generic, TypeVar

Key = TypeVar('Key')

class A(Generic[Key]):
    @staticmethod
    def lookup(d: Dict[Key, int], key: Key) -> None:
        x = d.get(key, 0) + 1

x = 1 + "a"

The first time running mypy foo.py produces the expected output:

foo.py:10: error: Unsupported operand types for + ("int" and "str")

Now running mypy foo.py a second time produces a spurious error:

foo.py:8: error: Unsupported operand types for + ("Key" and "int")
foo.py:8: note: Left operand is of type "Union[int, Key]"
foo.py:10: error: Unsupported operand types for + ("int" and "str")

Activity

  1. gvanrossum commented on Mar 5, 2019

    @gvanrossum
    Member

    Confirmed on latest master. @ilevkivskyi probably knows what this is a duplicate of, though. :-)

  2. ilevkivskyi commented on Mar 5, 2019

    @ilevkivskyi
    Member

    I think this is actually something new. (And it looks like a pure mystery.)

  3. gvanrossum commented on Mar 5, 2019

    @gvanrossum
    Member

    @pdeblanc Is there an earlier mypy version where this was not broken?

  4. pdeblanc commented on Mar 6, 2019

    @pdeblanc
    Author

    I'm not sure.

  5. joshuawu commented on Mar 22, 2019

    @joshuawu

    I performed a git bisect. In the context of the @pdeblanc's repro in the OP, the first "bad" commit was the one enabling the module cache by default:

    6b13652a466ccb102987a2ab1fa93d4b52fd8e3f is the first bad commit
    commit 6b13652a466ccb102987a2ab1fa93d4b52fd8e3f
    Author: Michael J. Sullivan <sully@msully.net>
    Date:   Thu Apr 5 10:12:12 2018 -0700
    
        Make incremental mode the default (#4800)
    
    :040000 040000 3ee965e31b88bd104fdd214e3c8dcba5aeb741ec 0809c2d4429a2998544b21816fa96de58673b4f4 M	docs
    :040000 040000 8f5d2fbe8e9311b0e02413f5ef5e6e5fe67c9a8e 58347b30d38225744f5c4d85b0cbdabf4b530ae4 M	mypy
    bisect run success
    

    So I turned on --incremental for the older commits. Now I found this:

    a2d654bbbbc94e6e3199001f9a0a95453f56fe85 is the first bad commit
    commit a2d654bbbbc94e6e3199001f9a0a95453f56fe85
    Author: Ivan Levkivskyi <levkivskyi@gmail.com>
    Date:   Tue Feb 7 13:31:02 2017 +0100
    
        Fix meta level on member access (#2809)
    
        Fixes #2804
    
        The problem is that analyze_member_access calls freshen_function_type_vars
        if the member is a function (i.e. method). This is necessary to not mix the type
        variables during type inference of generic methods inside generic functions.
        However, if the method does not participate in type inference, type variables
        are left in meta_level = 1 state. This causes the error, since the types could not
        be serialized in the middle of type inference.
    
        In this PR I propose to restore meta_level = 0 on member access, this is safe,
        since anyway, check_call always calls freshen_function_type_vars on callee (for
        generic functions, generic methods, etc).
    
    :040000 040000 8cfdb718a8acdd9e18a5df24c856bf3588851068 3e6ee42769347875a0863d3eb9fa14edf512bdf4 M	mypy
    :040000 040000 3c3d4a7330dace291e7dc037550a3297a8d9263b dc3a0a109c82ed6304929c2d81e1b7fd630fb026 M	test-data
    bisect run success
    

    I double-checked that these are indeed the offending lines:

    diff --git a/mypy/checkexpr.py b/mypy/checkexpr.py
    index 23ce56f..a5679f0 100644
    --- a/mypy/checkexpr.py
    +++ b/mypy/checkexpr.py
    @@ -993,6 +993,13 @@ class ExpressionChecker(ExpressionVisitor[Type]):
                     e.name, original_type, e, is_lvalue, False, False,
                     self.named_type, self.not_ready_callback, self.msg,
                     original_type=original_type, chk=self.chk)
    +            if isinstance(member_type, CallableType):
    +                for v in member_type.variables:
    +                    v.id.meta_level = 0
    +            if isinstance(member_type, Overloaded):
    +                for it in member_type.items():
    +                    for v in it.variables:
    +                        v.id.meta_level = 0    // This line causes the bug in the OP.
                 if is_lvalue:
                     return member_type
                 else:

    On master (a64278411d9c9cc6edc3e8a183714e726b0d2e05 as of this writing), the corresponding code has been moved to mypy/checkmember.py:539#freeze_type_vars, called once on :189 in the same file. If you delete mypy/checkmember.py:189, OP's bug disappears.

    However, the purpose of the original commit was to fix this issue from Feb 4, 2017. Deleting mypy/checkmember.py:189 resurrects the crash @gvanrossum described in the issue. (I was able to repro.)

    In the PR thread of this commit, @JukkaL expressed concerns that sound relevant to me, though I have no idea what's going on (I've never used mypy).

    @JukkaL: If we just switch the meta level, I think that we can get an id conflict between type variables in the 0 level if we happen to be in a generic function which has some additional type variables in scope.

    He changed his mind about this concern and approved the PR, but it seems like he was right after all. I suppose that a fix will need to deal with the id conflicts.

  6. gvanrossum commented on Mar 22, 2019

    @gvanrossum
    Member
  7. joshuawu commented on Mar 22, 2019

    @joshuawu

    No problem!

  8. joshuawu commented on Mar 23, 2019

    @joshuawu

    This is probably not directly related, but I noticed that these type definitions seem incorrect:

    test-data/unit/fixtures/dict.pyi:29:    def get(self, k: KT, default: Union[KT, T]) -> Union[VT, T]: pass
    test-data/unit/fixtures/floatdict.pyi:51:    def get(self, k: KT, default: Union[KT, T]) -> Union[VT, T]: pass
    test-data/unit/fixtures/floatdict_python2.pyi:51:    def get(self, k: KT, default: Union[KT, T]) -> Union[VT, T]: pass
    

    I think the default parameter should be T instead of Union[KT, T].

  9. gvanrossum commented on Mar 23, 2019

    @gvanrossum
    Member

    No, we tried that, and it broke stuff. See python/typeshed#2817

  10. joshuawu commented on Mar 23, 2019

    @joshuawu

    What about Union[VT, T] instead of Union[KT, T]? I’m confused as to why KT is in the type of default.

  11. gvanrossum commented on Mar 23, 2019

    @gvanrossum
    Member

    You must be misreading something. The code restored in python/typeshed#2817 uses (essentially) Union[VT, T].

  12. joshuawu commented on Mar 23, 2019

    @joshuawu

    Got it, perhaps I was looking at some older commit.

  13. erictraut commented on Aug 10, 2023

    @erictraut

    This bug appears to have been fixed at some point. The latest version of mypy produces consistent results when running multiple times on the code sample.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions