[mypyc] Add initial support for compiling singledispatch functions #10753
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Refactor the code for loading the arguments of the current function into a new function, which will make it easier to generate the IR for singledispatch functions.
Collect a list of registered singledispatch implementations in the prebuild pass so that we can use that list of registered implementations when we're generating IR for singledispatch functions.
Name the slower version of the isinstance primitive so that we can use it to check if the object passed to the singledispatch function is an instance of a non-native type.
Even though registered singledispatch implementations are decorated functions, we treat them as regular non-decorated functions because we remove all register decorators and don't add them to the fdefs_to_decorators dict, so we shouldn't try loading them in the top level code.
Special case singledispatch by generating special IR for functions marked with the singledispatch decorator that dispatches to the correct registered implementation based on the type of the first argument.
Fix flake8 'expected 2 blank lines, found 1' errors
Move the code for getting the singledispatch function associated with a register call from an expression like `fun.register` to a new function.
Add support for registering functions which use type annotations for the dispatch type, instead of passing an argument to the register call.
Several singledispatch related tests that were previously marked as xfails are able to pass now, so those tests are changed to regular tests.
Make sure that we don't crash with an IndexError when trying to get the arguments for the current function, which happens when we try to check if a decorated function that has no arguments is a register call.
msullivan
reviewed
Jul 3, 2021
There was a problem hiding this comment.
This looks really solid, though we should find a way to avoid mutating the AST.
Also, it just occured to me that something like the following test case might be tricky:
from functools import singledispatch
from typing import *
@singledispatch
def fun(arg: Any) -> Iterable[int]:
yield 1
@fun.register(int)
def fun_specialized(arg: str) -> Iterable[int]:
return [0]
Could you add something like that? You can xfail it if it fails and come back to it
| for arg in rt_args] | ||
| arg_kinds = [concrete_arg_kind(arg.kind) for arg in rt_args] | ||
| arg_info = get_args(builder, rt_args, line) | ||
| args, arg_kinds, arg_names = arg_info.args, arg_info.arg_kinds, arg_info.arg_names |
There was a problem hiding this comment.
You could also just do args, arg_kinds, arg_names = get_args(builder, rt_args, line), if you prefer.
There was a problem hiding this comment.
I mainly did that because I didn't want to accidentally mess up the order of the fields in the assignment.
| self.singledispatch_impls[impl.singledispatch_func].append( | ||
| (impl.dispatch_type, dec.func)) | ||
| removed.append(i) | ||
| for i in reversed(removed): |
There was a problem hiding this comment.
I don't think that we should mutate the AST in mypyc. Could you track this in some other way?
Instead of loading arguments based on the names of arguments of registered functions, load arguments based on the names of the arguments of the current function when we're generating IR that checks the first argument's type and dispatches to the correct implementation. This also adds a test to make sure that we're using the argument names of the current function, not the registered implementation.
I think having a Other than that, I think that would be a good test to add. I'll add it to this PR. |
|
Oh, sorry, that part was just a copy/paste screw up. I meant to have no argument to register |
Add 2 xfailed tests: - the first one checks if we handle keyword arguments to singledispatch correctly - the second one checks if we handle generators correctly, and whether we handle registered implementations returning a different type of iterable than the main singledispatch function - suggested at python#10753 (review)
The testTypeAnnotationsDisagreeWithRegisterArgument test fails on CI runs because the compiled code raises a TypeError when trying to pass a value with the incorrect type. We could try and support this by skipping the type check in this case or changing the type of the first argument to the type of the register argument, but this will probably turn into a mypy error at some point anyway, so it's not really worth the effort to keep this test around.
The keyword arguments test should have gotten marked as xfail in the last commit when it was added, but it was accidentally not marked as xfail.
pranavrajpal
added a commit
to pranavrajpal/mypy
that referenced
this pull request
Jul 7, 2021
Instead of modifying dec.decorators directly, copy it and only modify the copied version. That prevents us from modifying the AST in mypyc, which we should avoid according to python#10753 (comment).
msullivan
pushed a commit
that referenced
this pull request
Jul 7, 2021
Instead of modifying dec.decorators directly, copy it and only modify the copied version. That prevents us from modifying the AST in mypyc, which we should avoid according to #10753 (comment).
msullivan
pushed a commit
that referenced
this pull request
Jul 12, 2021
This makes several improvements to the support for compiling singledispatch that was introduced in #10753 by: * Making sure registered implementations defined later in a file take precedence when multiple overlap * Using non-native calls to registered implementations to allow for adding other decorators to registered functions (099b047) * Creating a separate function that dispatches to the correct implementation instead of adding code to dispatch to one of the registered implementations directly into the main singledispatch function, allowing the main singledispatch function to be a generator (59555e4) * Avoiding a compilation error when trying to dispatch on an ABC (2d40421)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds initial support for compiling functions marked with singledispatch by generating IR that checks the type of the first argument and calls the correct implementation, falling back to the main singledispatch function if none of the registered implementations have a dispatch type that matches the argument.
Currently, this only supports both one-argument versions of
register(passing a type as an argument to register or using type annotations), and only works ifregisteris used as a decorator.Test Plan
Several of the tests in run-singledispatch.test that were previously marked as xfail are now able to pass, so those are changed to regular tests.