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.
bpo-46409: Make generators in bytecode #30633
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
bpo-46409: Make generators in bytecode #30633
Changes from all commits
bf0394d
23877ec
381f3d3
c2b141e
8b73d0d
7cb3dad
79b6d11
039b14d
c33abbf
d105cac
e7c4bf2
c080089
1b177e3
243e441
a79e36b
7392593
17b453a
baa1009
44d3f47
06330b0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I have a question about gi_iframe. Below (L31) it is defined as an array of length 1 of object pointers. But everywhere it's used, it is cast to
InterpreterFrame *
. That's not an object or object pointer AFAICT. So what's going on here? Is the declaration wrong? An intentional lie?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.
It is a lie.
We don't want to expose
InterpreterFrame
in public headers, so we can't do the sensible thing and declaregi_frame
as:as C won't allow incomplete types in structs, even as the last member.
We could improve this by breaking up the header, so the public API sees a different definition. Still a lie, but a more elegant one.
Public header:
Private header:
Probably best done in a different PR, though.
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.
Gotcha. Any type other than PyObject* would be better though :-). And if you replace the casts with a macro it’s easier to fix later. I have an idea for the macro but it’s too painful to type on a phone.
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.
The first field of
InterpreterFrame
isPyFunctionObject *
and the first few fields are allPyObject *
, so declaringgi_frame
asPyObject * gi_frame[1]
seemed safe. Do let me know what your macro is, once you at a PC.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.
See https://bugs.python.org/issue40120#msg365465
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.