GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — [mypyc] Fix vtable pointer with inherited dunder new by p-sawicki · Pull Request #20302 · python/mypy Conversation
|
| continue; | ||
| } | ||
|
|
||
| while (def->ml_name && strcmp(def->ml_name, "__internal_mypyc_setup")) { |
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.
This search may be slow, we know that this method should be first if it is there, so maybe only check the first method, and if it has a different name, move to next base?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
| 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)) | ||
| return builder.call_c(setup_object, [subtype], expr.line) |
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.
Add a comment here explaining why a dynamic method dispatch is needed here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
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.
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?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
| 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)) | ||
| return builder.call_c(setup_object, [subtype], expr.line) |
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.
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?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
| } | ||
| } | ||
| if (!def || !def->ml_name) { | ||
| PyErr_SetString(PyExc_LookupError, "Internal error: Unable to find object setup function"); |
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.
Add something to the error message that indicates that this error is related to mypyc, and not CPython.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
| 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) |
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?
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
All reactions
Uh oh!
There was an error while loading. Please reload this page.
Fixes an issue where a subclass would have its vtable pointer set to the base class' vtable when there is a `__new__` method defined in the base class. This resulted in the subclass constructor calling the setup function of the base class because mypyc transforms `object.__new__` into the setup function. The fix is to store the pointers to the setup functions in `tp_methods` of type objects and look them up dynamically when instantiating new objects. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Reviewers
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assignedLabels
Projects
Milestone
No milestoneDevelopment
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
3 participants
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.