Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds support for basic union math with overloads #4842

Conversation

Michael0x2a
Copy link
Collaborator

This commit adds support for very basic and simple union math when calling overloaded functions, resolving #4576.

One thing led to another, and this ended up accidentally fixing or touching on several different overload-related issues. In particular, I believe this pull request:

  1. Fixes the bug (?) where calling overloaded functions can sometimes silently infer a return type of 'Any'
  2. Changes the semantics of how mypy handles overlapping functions, which I believe is currently under discussion in Clarify what @overload means typing#253

Although this change is functional and mergeable, I was planning on polishing it more -- adding more tests, fleshing out the union math behavior, etc.

However, I think these are sort of big changes and wanted to check in and make sure this pull request is actually welcome/is a good idea. If not, let me know, and I'd be happy to abandon it.


Details on specific changes made:

  1. The new algorithm works by modifying checkexpr.overload_call_targets to return all possible matches, rather then just one.

    We start by trying the first matching signature. If there was some error, we (conservatively) attempt to union all of the matching signatures together and repeat the typechecking process.

    If it doesn't seem like it's possible to combine the matching signatures in a sound way, we end and just output the errors we obtained from typechecking the first match.

    The "signature-unioning" code is currently deliberately very conservative. I figured it was better to start small and attempt to handle only basic cases like os.open doesn't accept Union[bytes, Text] #1943 and relax the restrictions later as needed. For more details on this algorithm, see the comments in checkexpr.union_overload_matches.

  2. This change incidentally resolves any bugs related to how calling an overloaded function can sometimes silently infer a return type of Any. Previously, if a function call caused an overload to be
    less precise then a previous one, we gave up and returned a silent Any.

    This change removes this case altogether and only infers Any if either (a) the caller arguments explicitly contains Any or (b) if there was some error.

    For example, see Should have a way to annotate an Optional carryover method. #3295 and Any incorrectly inferred for a call to an overloaded function #1322 -- I believe this pull request touches on and maybe resolves (??) those two issues.

  3. As a result, this caused a few errors in mypy where code was relying on this "silently infer Any" behavior -- see the changes in checker.py and semanal.py. Both files were using expressions of the form zip(*iterable), which ended up having a type of Any under the old algorithm. The new algorithm will instead infer Iterable[Tuple[Any, ...]] which actually matches the stubs in typeshed.

  4. Many of the attrs tests were also relying on the same behavior. It seemed that expressions of the form a = attr.ib() were evaluated to 'Any' not because of the definitions in the stubs, but because of the 'silent Any' bug.

    I couldn't find a clean way of fixing the stubs to infer the correct thing under this new behavior, so just gave up and removed the overloads altogether. I think this is fine though -- it seems like the attrs plugin infers the correct type for us anyways, regardless of what the stubs say.

    If this pull request is accepted, I plan on submitting a similar pull request to the stubs in typeshed.

  5. This pull request also probably touches on Clarify what @overload means typing#253. We still require the overloads to be written from the most narrow to general and disallow overlapping signatures.

    However, if a call now causes overlaps, we try the "union" algorithm described above and default to selecting the first matching overload instead of giving up.

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

It is a good piece of work, thanks! It would be god to have more time to review this, but now I have just one brief comment about tests: You don't have any tests wth generic functions and methods of generic classes. There might be some corner cases when type variables are involved.

@Michael0x2a
Copy link
Collaborator Author

@ilevkivskyi -- yep, will do. I plan on adding more tests either later today or over the weekend -- since this pull request does change the semantics of overloads slightly, I just wanted to make sure it wasn't going to be rejected out-of-hand first :)

No rush on reviewing -- it looks like people are pretty busy so I don't mind waiting.

@Michael0x2a Michael0x2a force-pushed the modify-overloads-to-support-basic-union-math branch from 1ebd4c4 to 4edaacb Compare April 11, 2018 15:30
This commit adds support for very basic and simple union math when
calling overloaded functions, resolving python#4576.

As a side effect, this change also fixes a bug where calling overloaded
functions can sometimes silently infer a return type of 'Any' and
slightly modifies the semantics of how mypy handles overlaps in
overloaded functions.

Details on specific changes made:

1.  The new algorithm works by modifying checkexpr.overload_call_targets
    to return all possible matches, rather then just one.

    We start by trying the first matching signature. If there was some
    error, we (conservatively) attempt to union all of the matching
    signatures together and repeat the typechecking process.

    If it doesn't seem like it's possible to combine the matching
    signatures in a sound way, we end and just output the errors we
    obtained from typechecking the first match.

    The "signature-unioning" code is currently deliberately very
    conservative. I figured it was better to start small and attempt to
    handle only basic cases like python#1943 and relax the restrictions later
    as needed. For more details on this algorithm, see the comments in
    checkexpr.union_overload_matches.

2.  This change incidentally resolves any bugs related to how calling
    an overloaded function can sometimes silently infer a return type
    of Any. Previously, if a function call caused an overload to be
    less precise then a previous one, we gave up and returned a silent
    Any.

    This change removes this case altogether and only infers Any if
    either (a) the caller arguments explicitly contains Any or (b) if
    there was some error.

    For example, see python#3295 and python#1322 -- I believe this pull request touches
    on and maybe resolves (??) those two issues.

3.  As a result, this caused a few errors in mypy where code was
    relying on this "silently infer Any" behavior -- see the changes in
    checker.py and semanal.py. Both files were using expressions of the
    form `zip(*iterable)`, which ended up having a type of `Any` under
    the old algorithm. The new algorithm will instead infer
    `Iterable[Tuple[Any, ...]]` which actually matches the stubs in
    typeshed.

4.  Many of the attrs tests were also relying on the same behavior.
    Specifically, these changes cause the attr stubs in
    `test-data/unit/lib-stub` to no longer work. It seemed that expressions
    of the form `a = attr.ib()` were evaluated to 'Any' not because of a
    stub, but because of the 'silent Any' bug.

    I couldn't find a clean way of fixing the stubs to infer the correct
    thing under this new behavior, so just gave up and removed the
    overloads altogether. I think this is fine though -- it seems like
    the attrs plugin infers the correct type for us anyways, regardless
    of what the stubs say.

    If this pull request is accepted, I plan on submitting a similar
    pull request to the stubs in typeshed.

4.  This pull request also probably touches on
    python/typing#253. We still require the
    overloads to be written from the most narrow to general and disallow
    overlapping signatures.

    However, if a *call* now causes overlaps, we try the "union"
    algorithm described above and default to selecting the first
    matching overload instead of giving up.
@Michael0x2a Michael0x2a force-pushed the modify-overloads-to-support-basic-union-math branch from 4edaacb to a5d181b Compare April 23, 2018 22:13
@ilevkivskyi ilevkivskyi self-assigned this May 14, 2018
@Michael0x2a
Copy link
Collaborator Author

Superseded by #5084.

@Michael0x2a Michael0x2a deleted the modify-overloads-to-support-basic-union-math branch June 15, 2018 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants