Skip to content

Skip arg count checks for dynamic functions. #1433

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 4 commits into from
Apr 25, 2016
Merged

Conversation

gvanrossum
Copy link
Member

Another followup for #1415 (fixing #1334).

I'm not at all sure this is the right thing to do. But we should honor the simple promise implied by the docs and by PEP 484 (even if not made very explicit) that the body of dynamic (unannotated) functions is not checked.

There are probably many other errors that are produced by the type checker for dynamic functions. But suppressing this one and #1431 are on at the top of the wishlist of our internal customers.

@JukkaL
Copy link
Collaborator

JukkaL commented Apr 24, 2016

Yeah, this is probably a reasonable thing to do, as type checking calls is prone to false positives. We can revisit this later if/when stubs are in good enough shape that false positives are no longer a significant problem.

The code that causes mypy to always type check calls is this fragment in visit_call_expr (checkexpr.py):

        self.accept(e.callee)
        # Access callee type directly, since accept may return the Any type
        # even if the type is known (in a dynamically typed function). This
        # way we get a more precise callee in dynamically typed functions.
        callee_type = self.chk.type_map[e.callee]

Modifying it to this should result in calls not being type checked, and it would be cleaner than this PR:

        callee_type = self.accept(e.callee)

@gvanrossum
Copy link
Member Author

@JukkaL Can you look at the failures? They're all from testtypegen, and seem regular fallout from the lesser checking in dynamic functions. I'll see if I can brute-force fix them and push another, but I'd like you to review that.

@JukkaL
Copy link
Collaborator

JukkaL commented Apr 25, 2016

I'll look at the failures later today. testtypegen is expected to fail, as one of the things that it tests is that types of functions are exported from unannotated functions, which no longer happens after this PR. We can just remove or update those test cases, as we don't care about the types any more.

@JukkaL
Copy link
Collaborator

JukkaL commented Apr 25, 2016

After the fix, the exported types aren't correct for generic functions in unannotated code. However, we don't rely on these types anywhere, so I'd suggest merging this PR and creating a new issue for fixing the exported types that refers to this PR.

@gvanrossum gvanrossum merged commit 4a4c415 into master Apr 25, 2016
@gvanrossum gvanrossum deleted the too-many-few-args branch April 25, 2016 17:59
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