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.

Sign up for GitHub

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

Jump to bottom

Allow the f_func field of the _PyInterpreterFrame struct to be any object (and rename it) #96237

Closed
markshannon opened this issue Aug 24, 2022 · 5 comments
Closed

Allow the f_func field of the _PyInterpreterFrame struct to be any object (and rename it) #96237

markshannon opened this issue Aug 24, 2022 · 5 comments
Labels
performance Performance or resource usage

Comments

@markshannon
Copy link
Member

In order to better support shim frames, trampolines, Cython frames and other such bits and pieces, we need to relax the requirement that the f_func field is a function.
The only reason for it to be a function is that it needs to hold a strong reference to builtins and globals, as the frame only has a borrowed reference to those. Any object with those references will do.
In fact, for code that has no builtins or globals (many shims and trampolines) there is no need for those references and f_func could even be None.
@markshannon markshannon added the performance Performance or resource usage label Aug 24, 2022
@bedevere-bot bedevere-bot mentioned this issue Aug 24, 2022
Merged
@iritkatriel
Copy link
Member

Does it really need to allow any object? Could you instead define a small number of object types that it can be?

@markshannon
Copy link
Member Author

I don't see any advantage to restricting the possible types. Allowing any object is no worse for any code reading from f_funcobj and more flexible for anything storing to it.

Given the values of f_globals and f_builtins can no longer be trusted, I think I'll make them void * just to make it clear that if you didn't set them, then you can't trust them.

markshannon added a commit that referenced this issue Aug 25, 2022
@markshannon
GH-96237: Allow non-functions as reference-holder in frames. (GH-96238)
c09fa75
@carljm
Copy link
Member

carljm commented Feb 14, 2023 •
edited
Loading

Currently whatever is in f_funcobj also needs to have a func_closure. #101310 demonstrates one way to remove this requirement.

@carljm
Copy link
Member

carljm commented Feb 14, 2023

Should this issue be closed, given the attached PR was merged? Or is there more planned here?

@markshannon
Copy link
Member Author

Superseded by #100987

@markshannon markshannon closed this as not planned Won't fix, can't repro, duplicate, stale Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
performance Performance or resource usage
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@carljm @iritkatriel @markshannon