8000 bpo-46409: Make generators in bytecode by markshannon · Pull Request #30633 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

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.

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

Merged
merged 20 commits into from
Jan 20, 2022
Merged

bpo-46409: Make generators in bytecode #30633

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
bf0394d
Add new instruction to make generator or coroutine from current frame.
markshannon Jan 17, 2022
23877ec
Set line number for RETURN_GENERATOR instruction.
markshannon Jan 17, 2022
381f3d3
Fix magic number
markshannon Jan 17, 2022
c2b141e
Remove unused code.
markshannon Jan 17, 2022
8b73d0d
Rename YIELD_FROM_LOOP to JUMP_NO_INTERRUPT: describe what it does, n…
markshannon Jan 17, 2022
7cb3dad
Document new bytecodes.
markshannon Jan 17, 2022
79b6d11
Add news
markshannon Jan 17, 2022
039b14d
Merge branch 'main' into make-gen-in-bytecode
markshannon Jan 17, 2022
c33abbf
Fix whitespace in opcode.py
markshannon Jan 17, 2022
d105cac
Trim frame and generator by word each.
markshannon Jan 18, 2022
e7c4bf2
Minor refactor of frame.c
markshannon Jan 18, 2022
c080089
Update test.test_sys to account for smaller frames.
markshannon Jan 18, 2022
1b177e3
Skip clearing of old frame when returning a generator.
markshannon Jan 18, 2022
243e441
Treat generator functions as normal functions when evaluating and spe…
markshannon Jan 18, 2022
a79e36b
Attempt to streamline eval-breaker checks a bit.
markshannon Jan 18, 2022
7392593
Revert change in test.
markshannon Jan 19, 2022
17b453a
Remove unused macro.
markshannon Jan 19, 2022
baa1009
Add some clarifications and explanations.
markshannon Jan 19, 2022
44d3f47
Name new instructions in news item.
markshannon Jan 20, 2022
06330b0
Use Py_NewRef
markshannon Jan 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Trim frame and generator by word each.
  • Loading branch information
@markshannon
markshannon committed Jan 18, 2022
commit d105cac6d54aca87f14a261642b82fa7b66402a9
8000
1 change: 0 additions & 1 deletion Include/cpython/genobject.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ extern "C" {
and coroutine objects. */
#define _PyGenObject_HEAD(prefix) \
PyObject_HEAD \
/* Note: gi_frame can be NULL if the generator is "finished" */ \
Copy link
Member

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?

Copy link
Member Author

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 declare gi_frame as:

    InterpreterFrame gi_frame;

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:

typedef struct {
    _PyGenObject_HEAD(gi)
} PyGenObject;

Private header:

typedef struct {
    _PyGenObject_HEAD(gi)
    InterpreterFrame gi_frame;
} PyGenObject;

Probably best done in a different PR, though.

Copy link
Member

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.

Copy link
Member Author

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 is PyFunctionObject * and the first few fields are all PyObject *, so declaring gi_frame as PyObject * gi_frame[1] seemed safe. Do let me know what your macro is, once you at a PC.

Copy link
Member

Choose a reason for hiding this comment

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

Do let me know what your macro is, once you at a PC.

See https://bugs.python.org/issue40120#msg365465

/* The code object backing the generator */ \
PyCodeObject *prefix##_code; \
/* List of weak reference. */ \
Expand Down
4 changes: 2 additions & 2 deletions Include/internal/pycore_frame.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ typedef struct _interpreter_frame {
PyObject *f_locals; /* Strong reference, may be NULL */
PyCodeObject *f_code; /* Strong reference */
PyFrameObject *frame_obj; /* Strong reference, may be NULL */
PyObject *generator; /* Borrowed reference, may be NULL */
struct _interpreter_frame *previous;
int f_lasti; /* Last instruction if called */
int stacktop; /* Offset of TOS from localsplus */
PyFrameState f_state; /* What state the f 8000 rame is in */
bool is_entry; // Whether this is the "root" frame for the current CFrame.
bool is_generator;
PyObject *localsplus[1];
} InterpreterFrame;

Expand Down Expand Up @@ -100,10 +100,10 @@ _PyFrame_InitializeSpecials(
frame->f_locals = Py_XNewRef(locals);
frame->stacktop = nlocalsplus;
frame->frame_obj = NULL;
frame->generator = NULL;
frame->f_lasti = -1;
frame->f_state = FRAME_CREATED;
frame->is_entry = false;
frame->is_generator = false;
}

/* Gets the pointer to the locals array
Expand Down
8 changes: 5 additions & 3 deletions Objects/frameobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ frame_dealloc(PyFrameObject *f)
{
/* It is the responsibility of the owning generator/coroutine
* to have cleared the generator pointer */
assert(f->f_frame->generator == NULL);
assert(!f->f_frame->is_generator);

if (_PyObject_GC_IS_TRACKED(f)) {
_PyObject_GC_UNTRACK(f);
Expand Down Expand Up @@ -699,8 +699,10 @@ frame_clear(PyFrameObject *f, PyObject *Py_UNUSED(ignored))
"cannot clear an executing frame");
return NULL;
}
if (f->f_frame->generator) {
_PyGen_Finalize(f->f_frame->generator);
if (f->f_frame->is_generator) {
assert(!f->f_owns_frame);
PyObject *gen = (PyObject *)(((char *)f->f_frame)-offsetof(PyGenObject, gi_iframe));
_PyGen_Finalize(gen);
}
(void)frame_tp_clear(f);
Py_RETURN_NONE;
Expand Down
22 changes: 9 additions & 13 deletions Objects/genobject.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ gen_dealloc(PyGenObject *gen)
if (gen->gi_frame_valid) {
InterpreterFrame *frame = (InterpreterFrame *)gen->gi_iframe;
gen->gi_frame_valid = 0;
frame->generator = NULL;
frame->is_generator = false;
frame->previous = NULL;
_PyFrame_Clear(frame);
}
Expand Down Expand Up @@ -265,7 +265,7 @@ gen_send_ex2(PyGenObject *gen, PyObject *arg, PyObject **presult,
/* first clean reference cycle through stored exception traceback */
_PyErr_ClearExcState(&gen->gi_exc_state);

frame->generator = NULL;
frame->is_generator = false;
gen->gi_frame_valid = 0;
_PyFrame_Clear(frame);
*presult = result;
Expand Down Expand Up @@ -896,16 +896,12 @@ make_gen(PyTypeObject *type, PyFunctionObject *func)
gen->gi_weakreflist = NULL;
gen->gi_exc_state.exc_value = NULL;
gen->gi_exc_state.previous_item = NULL;
if (func->func_name != NULL)
gen->gi_name = func->func_name;
else
gen->gi_name = gen->gi_code->co_name;
Py_INCREF(gen->gi_name);
if (func->func_qualname != NULL)
gen->gi_qualname = func->func_qualname;
else
gen->gi_qualname = gen->gi_name;
Py_INCREF(gen->gi_qualname);
assert(func->func_name != NULL);
Py_INCREF(func->func_name);
gen->gi_name = func->func_name;
assert(func->func_qualname != NULL);
Py_INCREF(func->func_qualname);
gen->gi_qualname = func->func_qualname;
_PyObject_GC_TRACK(gen);
return (PyObject *)gen;
}
Expand Down Expand Up @@ -976,7 +972,7 @@ gen_new_with_qualname(PyTypeObject *type, PyFrameObject *f,
assert(frame->frame_obj == f);
f->f_owns_frame = 0;
f->f_frame = frame;
frame->generator = (PyObject *) gen;
frame->is_generator = true;
assert(PyObject_GC_IsTracked((PyObject *)f));
gen->gi_code = PyFrame_GetCode(f);
Py_INCREF(gen->gi_code);
Expand Down
2 changes: 1 addition & 1 deletion Python/ceval.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5056,7 +5056,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
assert(frame->frame_obj == NULL);
frame->f_locals = NULL;
gen->gi_frame_valid = 1;
gen_frame->generator = (PyObject *)gen;
gen_frame->is_generator = true;
gen_frame->f_state = FRAME_CREATED;
_Py_LeaveRecursiveCall(tstate);
if (!frame->is_entry) {
Expand Down
5 changes: 2 additions & 3 deletions Python/frame.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ _PyFrame_Copy(InterpreterFrame *src, InterpreterFrame *dest)
static inline void
clear_specials(InterpreterFrame *frame)
{
frame->generator = NULL;
Py_XDECREF(frame->frame_obj);
Py_XDECREF(frame->f_locals);
Py_DECREF(frame->f_func);
Expand Down Expand Up @@ -95,8 +94,8 @@ void
_PyFrame_Clear(InterpreterFrame * frame)
{
/* It is the responsibility of the owning generator/coroutine
* to have cleared the generator pointer */
assert(frame->generator == NULL);
* to have cleared the enclosing generator, if any. */
assert(!frame->is_generator);
if (frame->frame_obj) {
PyFrameObject *f = frame->frame_obj;
frame->frame_obj = NULL;
Expand Down
0