-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
[mypyc] Fix vtable pointer with inherited dunder new #20302
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
Changes from 4 commits
3e8d126
be8f0ff
98d7abb
499389f
54dc884
2fd137e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,7 @@ | |
| Var, | ||
| ) | ||
| from mypy.types import AnyType, TypeOfAny | ||
| from mypyc.ir.class_ir import all_concrete_classes | ||
| from mypyc.ir.ops import ( | ||
| BasicBlock, | ||
| Call, | ||
|
|
@@ -99,7 +100,7 @@ | |
| isinstance_dict, | ||
| ) | ||
| from mypyc.primitives.float_ops import isinstance_float | ||
| from mypyc.primitives.generic_ops import generic_setattr | ||
| from mypyc.primitives.generic_ops import generic_setattr, setup_object | ||
| from mypyc.primitives.int_ops import isinstance_int | ||
| from mypyc.primitives.list_ops import isinstance_list, new_list_set_item_op | ||
| from mypyc.primitives.misc_ops import isinstance_bool | ||
|
|
@@ -1103,7 +1104,14 @@ def translate_object_new(builder: IRBuilder, expr: CallExpr, callee: RefExpr) -> | |
| method_args = fn.fitem.arg_names | ||
| if isinstance(typ_arg, NameExpr) and len(method_args) > 0 and method_args[0] == typ_arg.name: | ||
| subtype = builder.accept(expr.args[0]) | ||
| return builder.add(Call(ir.setup, [subtype], expr.line)) | ||
| classes = all_concrete_classes(ir) | ||
| if classes and len(classes) == 1: | ||
| return builder.add(Call(ir.setup, [subtype], expr.line)) | ||
| # Call a function that dynamically resolves the setup function of extension classes from the type object. | ||
| # This is necessary because the setup involves default attribute initialization and setting up | ||
| # the vtable which are specific to a given type and will not work if a subtype is created using | ||
| # the setup function of its base. | ||
| return builder.call_c(setup_object, [subtype], expr.line) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add a comment here explaining why a dynamic method dispatch is needed here.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a fast path if class is known to not have subclasses -- we would still be able to use the old direct call, right? |
||
|
|
||
| return None | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ir.subclasses()seems like the correct way -- we need to use the slow path for an ABC that has only a single concrete subclass, I think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok that makes sense. i didn't add a test for this because for now
__new__cannot be called in an abstract class anyway as it inherits from a python type.