Skip to content

gh-127274: Defer nested methods #128012

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

Merged
merged 5 commits into from
Dec 19, 2024
Merged

gh-127274: Defer nested methods #128012

merged 5 commits into from
Dec 19, 2024

Conversation

mpage
Copy link
Contributor

@mpage mpage commented Dec 17, 2024

Methods (functions defined in class scope) are likely to be cleaned up by the GC anyway, so deferring nested methods will not prolong their lifetime.

Add a new code flag, CO_METHOD, that is set for functions defined in a class scope. Use that when deciding to defer functions.

Methods (functions defined in class scope) are likely to be cleaned
up by the GC anyway.

Add a new code flag, `CO_METHOD`, that is set for functions defined
in a class scope. Use that when deciding to defer functions.
Copy link
Contributor

@colesbury colesbury left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@JelleZijlstra
Copy link
Member

The other CO_* constants are documented and exposed in the inspect module (e.g. https://docs.python.org/3.14/library/inspect.html#inspect.CO_HAS_DOCSTRING); we'll have to do that for this one too.

@mpage
Copy link
Contributor Author

mpage commented Dec 17, 2024

The other CO_* constants are documented and exposed in the inspect module

Should that be done for CO_NO_MONITORING_EVENTS as well (in a separate PR)?

@mpage mpage removed the skip news label Dec 18, 2024
@JelleZijlstra
Copy link
Member

Should that be done for CO_NO_MONITORING_EVENTS as well (in a separate PR)?

Wasn't aware of that flag, but if we also added that one recently, then yes.

@mpage
Copy link
Contributor Author

mpage commented Dec 18, 2024

MacOS-13 build failure looks like an issue with homebrew that is unrelated to this PR.

@mpage
Copy link
Contributor Author

mpage commented Dec 18, 2024

@JelleZijlstra - would you please have another look?

Copy link
Member

@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Could also add tests that look directly at the flags of a code object, but flags are documented as internal and an implementation detail and CO_HAS_DOCSTRINGS doesn't have tests either, so I think it's fine not to test this.

@mpage mpage merged commit 255762c into python:main Dec 19, 2024
41 checks passed
@mpage mpage deleted the gh-127274-method-flag branch December 19, 2024 21:03
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Dec 23, 2024
Methods (functions defined in class scope) are likely to be cleaned
up by the GC anyway.

Add a new code flag, `CO_METHOD`, that is set for functions defined
in a class scope. Use that when deciding to defer functions.
srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull request Jan 8, 2025
Methods (functions defined in class scope) are likely to be cleaned
up by the GC anyway.

Add a new code flag, `CO_METHOD`, that is set for functions defined
in a class scope. Use that when deciding to defer functions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants