[mypyc] Support __pos__ and __abs__ dunders #13490
Conversation
ichard26
force-pushed
the
support-pos-and-abs-dunders
branch
from
b03734b
to
fa90af6
Compare
Calls to these dunders on native classes will be specialized to use a direct method call instead of using PyNumber_Absolute. Also calls to abs() on any types have been optimized. They no longer involve a builtins dictionary lookup. It's probably possible to write a C helper function for abs(int) to avoid the C-API entirely for native integers, but I don't feel skilled enough to do that yet.
ichard26
force-pushed
the
support-pos-and-abs-dunders
branch
from
fa90af6
to
05d6799
Compare
| arg_typ = builder.node_type(arg) | ||
| if isinstance(arg_typ, RInstance) and arg_typ.class_ir.has_method("__abs__"): | ||
| obj = builder.accept(arg) | ||
| return builder.gen_method_call(obj, "__abs__", [], None, expr.line) |
There was a problem hiding this comment.
What happens if __abs__ has been defined incorrectly and, for example, accepts an argument? Maybe guard against this.
There was a problem hiding this comment.
Wouldn't that fail type checking as abs() is annotated to only accept an object that implements __abs__ in accordance to:
@runtime_checkable
class SupportsAbs(Protocol[_T_co]):
@abstractmethod
def __abs__(self) -> _T_co: ...
abs(__x: SupportsAbs[_T]) -> _T: ...https://github.com/python/typeshed/blob/2c534dc2208191dc1ae1a6f51554fa5022cbfa0a/stdlib/typing.pyi#L313-L316
https://github.com/python/typeshed/blob/2c534dc2208191dc1ae1a6f51554fa5022cbfa0a/stdlib/builtins.pyi#L1199
class A:
def __abs__(self, arg: int) -> int:
return arg + 1
v = abs(A())❯ mypyc t.py
t.py:6: error: Need type annotation for "v"
t.py:6: error: Argument 1 to "abs" has incompatible type "A"; expected "SupportsAbs[<nothing>]"
There's also a if guard that checks for one positional argument before attempting specialization so even if this somehow type checks, the specialization will fail and return None. The return type shouldn't matter as abs() doesn't check the return type of __abs__ at all.
Am I missing something?
There was a problem hiding this comment.
I do see that mypyc doesn't complain if you don't ever call abs() on the native class with the invalid __abs__ dunder and simply generates bad code that isn't compatible with the C-API:
build/__native.c:39:20: error: initialization of ‘PyObject * (*)(PyObject *)’ {aka ‘struct _object * (*)(struct _object *)’} from incompatible pointer type ‘PyObject * (*)(PyObject *, PyObject *)’ {aka ‘struct _object * (*)(struct _object *, struct _object *)’} [-Werror=incompatible-pointer-types]
39 | .nb_absolute = CPyDunder___abs__A,
| ^~~~~~~~~~~~~~~~~~
build/__native.c:39:20: note: (near initialization for ‘A_as_number.nb_absolute’)
... but I don't think checking for invalid dunders (and raising an error) should happen here since it applies to all dunders, not just __abs__. It would also miss the error if abs() is never used in compiled code.
There was a problem hiding this comment.
Yeah, this is something that should be checked elsewhere. I'll create an issue about this -- no need to fix it here.
|
Hi, I'd appreciate if this could be looked at again soon, thanks in advance! |
| arg_typ = builder.node_type(arg) | ||
| if isinstance(arg_typ, RInstance) and arg_typ.class_ir.has_method("__abs__"): | ||
| obj = builder.accept(arg) | ||
| return builder.gen_method_call(obj, "__abs__", [], None, expr.line) |
There was a problem hiding this comment.
Yeah, this is something that should be checked elsewhere. I'll create an issue about this -- no need to fix it here.
Description
Towards mypyc/mypyc#553.
Add supports for these dunders:
__pos____abs__Calls to these dunders on native classes will be specialized to use a direct method call instead of using PyNumber_Absolute.
Also calls to abs() on any types have been optimized. They no longer involve a builtins dictionary lookup. It's probably possible to write a C helper function for abs(int) to avoid the C-API entirely for native integers, but I don't feel skilled enough to do that yet.
Test Plan