Skip to content

Fixes type inference of type var from context #447

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

Merged
merged 1 commit into from
Sep 15, 2014

Conversation

spkersten
Copy link
Contributor

For type vars, ConstraintBuilderVisitor always created a constraint in the SUPERTYPE_OF direction for type vars, irrespective of its direction field, which is wrong. This fixes both issue #360 as well as the errors reported by Travis in #443.

Currently, 15 tests are failing because the error messages changed due to the fix. Sometimes they became more accurate; sometimes they became wrong (reporting incompatible types of an assignment instead of arguments). Before I fix the messages and test cases, I'd like someone to look at the fix and confirm that it is indeed correct now.

The following example (extension of the one from #360) shows that this pull request fixes that issue and correctly reports a type error in the last line.

from typing import AnyStr

def f(x: AnyStr) -> AnyStr:
    if isinstance(x, str):
        return 'foo'
    else:
        return b'zar'

def g(s: str): pass

f('')  # OK
repr(f(''))   # OK: repr expect object and get str. Used to give an incorrect type error

g(f('string'))
g(f(b'bytes'))   # E: Argument 1 to "g" has incompatible type "bytes"; expected "str"

@spkersten
Copy link
Contributor Author

Apparently, there is some problem with travis.sh. The failing tests are not reflected in the status.

@JukkaL
Copy link
Collaborator

JukkaL commented Sep 14, 2014

Yeah, travis.sh is clearly broken for unit tests. Added #454. I will look into it.

@JukkaL
Copy link
Collaborator

JukkaL commented Sep 14, 2014

travis.sh should work now.

@JukkaL
Copy link
Collaborator

JukkaL commented Sep 15, 2014

Your fix was good, but it actually solved a different issue. There is actually a third bug (that was hidden by the bug you fixed) that causes the test failures and also seems to fix the original issue (but it just hid it). I spent a while figuring out what is going on, but I think I was able to finally fix it. I also fixed the other issue surfaced by your fix.

@JukkaL JukkaL merged commit 5585f8f into python:master Sep 15, 2014
@spkersten spkersten deleted the context360 branch September 15, 2014 19:35
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