2574 StackOverflow Exception On Super Method Call Fix #2628
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
3ddd76a to
bf389cf
Compare
Member
|
Thanks a lot for this. I will try my best to get the CI working again on the weekend and will "enqueue" this one for merging. |
Contributor
Author
|
I have added the actual fix for #2574. In this case, it did not find the correct method to call the base class variant of the virtual method called. Instead it decided to call the top function, causing a stack overflow exception. This has been fixed by using the correct name for the base class function invoke method I think this is ready for merge, but I will continue testing it. |
…ning a method and some not. Also fixed an issue causing the finalizer to be called twice. Added unit tests to verify the fix.
a3fbc84 to
98a740e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Update: Does not actually fix #2574, but a similar problem(#2629). I'll provide a fix for 2574 as well.Update 2: Now this also fixes #2574
This fixed an issue with a StackOverflow exception being generated when a virtual method is being called, but a class in the middle of the inheritance tree does not override the method.
This happens because the CallVirtual method calls the method on the python object if the wrapper got generated, causing everything to be evaluated again from the top. Doing it this way, causes the the python method only to get invoked if there is an implementation on the class, which can then correctly call
super().Method().For the finalizer fix, we now skip generating the call to PyFinalize if the base class is already a python class, because if that is the case the finalize method has already been added.
Additionally it simplifies how the generated code for virtual methods look. Now, a virtual wrapper method is not automatically generated. Only if the method is actually being overwritten by the class it will prompt the generation of a virtual method wrapper.
Does this close any currently open issues?
This closes #2574, #2627, #2629
Any other comments?
...
Checklist
Check all those that are applicable and complete.
AUTHORSCHANGELOG