Repository: python/cpython
Pull request: gh-108991: replace _PyFrame_GetState by two simpler functions
Fixes #108991.

<!-- gh-issue-number: gh-108991 -->
* Issue: gh-108991
<!-- /gh-issue-number -->


Problem statement:
_PyFrame_GetState is misnamed and does more than it needs to
_PyFrame_GetState is a static function so can be renamed to reflect that.

More importantly, it calculates a lot more than what it is actually used for (it is called in two places, each of which checks for one of the states: CLEARED or SUSPENDED). The rest is neither used nor tested.

Some of the other states that this function checks for depend on the opcode at ``prev_instr``, which is going to be replaced soon by ``instr_ptr``. If we want to keep this function working we need to first cover all the states by tests. But there is no point doing this for code which is unused. 

So I will replace this function by two simpler functions for the two existing use cases: checking if the frame is in CLEARED or SUSPENDED state.

<!-- gh-linked-prs -->
### Linked PRs
* gh-108992
<!-- /gh-linked-prs -->

Hints from issue discussion:

Commit message: gh-108991: replace _PyFrame_GetState by two simpler functions

Patch to review:
diff --git a/Objects/frameobject.c b/Objects/frameobject.c
index 28f5a5a1222806..1370b120b42091 100644
--- a/Objects/frameobject.c
+++ b/Objects/frameobject.c
@@ -591,39 +591,29 @@ first_line_not_before(int *lines, int len, int line)
     return result;
 }
 
-static PyFrameState
-_PyFrame_GetState(PyFrameObject *frame)
+static bool
+frame_is_cleared(PyFrameObject *frame)
 {
     assert(!_PyFrame_IsIncomplete(frame->f_frame));
     if (frame->f_frame->stacktop == 0) {
-        return FRAME_CLEARED;
+        return true;
     }
-    switch(frame->f_frame->owner) {
-        case FRAME_OWNED_BY_GENERATOR:
-        {
-            PyGenObject *gen = _PyFrame_GetGenerator(frame->f_frame);
-            return gen->gi_frame_state;
-        }
-        case FRAME_OWNED_BY_THREAD:
-        {
-            if (_PyInterpreterFrame_LASTI(frame->f_frame) < 0) {
-                return FRAME_CREATED;
-            }
-            switch (frame->f_frame->prev_instr->op.code)
-            {
-                case COPY_FREE_VARS:
-                case MAKE_CELL:
-                case RETURN_GENERATOR:
-                    /* Frame not fully initialized */
-                    return FRAME_CREATED;
-                default:
-                    return FRAME_EXECUTING;
-            }
-        }
-        case FRAME_OWNED_BY_FRAME_OBJECT:
-            return FRAME_COMPLETED;
+    if (frame->f_frame->owner == FRAME_OWNED_BY_GENERATOR) {
+        PyGenObject *gen = _PyFrame_GetGenerator(frame->f_frame);
+        return gen->gi_frame_state == FRAME_CLEARED;
     }
-    Py_UNREACHABLE();
+    return false;
+}
+
+static bool frame_is_suspended(PyFrameObject *frame)
+{
+    assert(!_PyFrame_IsIncomplete(frame->f_frame));
+    if (frame->f_frame->owner == FRAME_OWNED_BY_GENERATOR) {
+        PyGenObject *gen = _PyFrame_GetGenerator(frame->f_frame);
+        return gen->gi_frame_state == FRAME_SUSPENDED;
+    }
+    return false;
+
 }
 
 /* Setter for f_lineno - you can set f_lineno from within a trace function in
@@ -655,7 +645,7 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno, void *Py_UNUSED(ignore
         return -1;
     }
 
-    PyFrameState state = _PyFrame_GetState(f);
+    bool is_suspended = frame_is_suspended(f);
     /*
      * This code preserves the historical restrictions on
      * setting the line number of a frame.
@@ -811,7 +801,7 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno, void *Py_UNUSED(ignore
         }
         assert(unbound == 0);
     }
-    if (state == FRAME_SUSPENDED) {
+    if (is_suspended) {
         /* Account for value popped by yield */
         start_stack = pop_value(start_stack);
     }
@@ -1455,7 +1445,7 @@ void
 PyFrame_LocalsToFast(PyFrameObject *f, int clear)
 {
     assert(!_PyFrame_IsIncomplete(f->f_frame));
-    if (f && f->f_fast_as_locals && _PyFrame_GetState(f) != FRAME_CLEARED) {
+    if (f && f->f_fast_as_locals && !frame_is_cleared(f)) {
         _PyFrame_LocalsToFast(f->f_frame, clear);
         f->f_fast_as_locals = 0;
     }


Code review comments:
diff_hunk: @@ -591,39 +591,29 @@ first_line_not_before(int *lines, int len, int line)
     return result;
 }
 
-static PyFrameState
-_PyFrame_GetState(PyFrameObject *frame)
+static bool
+frame_is_cleared(PyFrameObject *frame)
 {
     assert(!_PyFrame_IsIncomplete(frame->f_frame));
     if (frame->f_frame->stacktop == 0) {
-        return FRAME_CLEARED;
+        return true;
     }
-    switch(frame->f_frame->owner) {
-        case FRAME_OWNED_BY_GENERATOR:
-        {
-            PyGenObject *gen = _PyFrame_GetGenerator(frame->f_frame);
-            return gen->gi_frame_state;
-        }
-        case FRAME_OWNED_BY_THREAD:
-        {
-            if (_PyInterpreterFrame_LASTI(frame->f_frame) < 0) {
-                return FRAME_CREATED;
-            }
-            switch (frame->f_frame->prev_instr->op.code)
-            {
-                case COPY_FREE_VARS:
-                case MAKE_CELL:
-                case RETURN_GENERATOR:
-                    /* Frame not fully initialized */
-                    return FRAME_CREATED;
-                default:
-                    return FRAME_EXECUTING;
-            }
-        }
-        case FRAME_OWNED_BY_FRAME_OBJECT:
-            return FRAME_COMPLETED;
+    if (frame->f_frame->owner == FRAME_OWNED_BY_GENERATOR) {
+        PyGenObject *gen = _PyFrame_GetGenerator(frame->f_frame);
+        return gen->gi_frame_state == FRAME_CLEARED;
     }
-    Py_UNREACHABLE();
+    return false;
+}
+
+static bool frame_is_suspended(PyFrameObject *frame)
+{
+    assert(!_PyFrame_IsIncomplete(frame->f_frame));
+    if (frame->f_frame->owner == FRAME_OWNED_BY_GENERATOR) {
+        PyGenObject *gen = _PyFrame_GetGenerator(frame->f_frame);
+        return gen->gi_frame_state == FRAME_SUSPENDED;
+    }
+    return false;
+
path: Objects/frameobject.c
text: @author:
```suggestion
```