Skip to content
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

Remove redundant _PyFrame_GetCode #113217

Closed
wants to merge 1 commit into from

Conversation

WolframAlph
Copy link
Contributor

@WolframAlph WolframAlph commented Dec 16, 2023

No need to call _PyFrame_GetCode(cframe), it is already passed as co:

int res = super_init_without_args(frame, _PyFrame_GetCode(frame), &type, &obj);

@carljm
Copy link
Member

carljm commented Dec 16, 2023

Hi! Thanks for the pull request. As a general rule, CPython doesn't accept "code cleanup" pull requests that don't fix an actual bug or add a feature; it's not worth the code churn and risk of breakage. I do think this change is correct and safe, and there could in principle be a performance impact, though I doubt its detectable (especially since in Python 3.12 with the LOAD_SUPER_ATTR opcode, this function is rarely used anymore.)

Also, per the dev guide, all CPython PRs (unless they are fixing e.g. just a doc typo) should be linked to a separate issue describing the problem.

@WolframAlph
Copy link
Contributor Author

WolframAlph commented Dec 17, 2023

Thanks for review! For unoptimised build it produces less assembly while behaviour is the same, but it's true that for optimised build, generated assembly is the same. Came across this while investigating #113212, so decided maybe it makes sense to modify it, but I get your point. Closing this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants