Skip to content

Fix overload resolution on Tuples #1585

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

Closed
wants to merge 1 commit into from

Conversation

rwbarton
Copy link
Contributor

Fixes #1578, since list's setitem is overloaded.

def f(x: int, y: Tuple[str, ...]) -> None: pass
@overload
def f(x: int, y: str) -> None: pass
f(1, ('2', '3'))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also test calling with a Tuple[str, ...] arg and a Tuple[int, ...] argument.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, now it's actually added.

@rwbarton rwbarton force-pushed the rwbarton-overload-tuple branch from 1ec67ba to b162a4a Compare May 26, 2016 23:14
Fixes python#1578, since list's __setitem__ is overloaded.
@rwbarton rwbarton force-pushed the rwbarton-overload-tuple branch from b162a4a to ab2a7b2 Compare May 26, 2016 23:25
@JukkaL
Copy link
Collaborator

JukkaL commented Jun 10, 2016

@rwbarton Feel free to land this PR at any time if tests pass after fixing rebase conflicts. Sorry for the delay!

@rwbarton
Copy link
Contributor Author

Hmm, I thought I commented somewhere about this PR being unneeded after #1596, but now I can't find that comment...

@rwbarton
Copy link
Contributor Author

Oh right, I remember now.

    # Fall back to a conservative equality check for the remaining kinds of type.
    return 2 if is_same_type(erasetype.erase_type(actual), erasetype.erase_type(formal)) else 0

With the change to erase_type in #1596, this now correctly handles the case of calling a Tuple formal argument with a Tuple value, so the workaround in this PR is no longer needed.

I'll still merge the tests though since they are a bit different than the ones added in #1596.

@rwbarton
Copy link
Contributor Author

I added the tests in commit 838b4e0.

@rwbarton rwbarton closed this Jun 14, 2016
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