From dc8230d2417d8484684a790a2239e8c5bd3f38cd Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Thu, 24 Apr 2025 18:22:58 -0600 Subject: [PATCH 1/5] Add _PyCode_Returns(). --- Include/internal/pycore_code.h | 4 ++++ Objects/codeobject.c | 40 ++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h index 2839b9b7ebe0fb..e7967ad43881d3 100644 --- a/Include/internal/pycore_code.h +++ b/Include/internal/pycore_code.h @@ -561,6 +561,10 @@ extern void _Py_ClearTLBCIndex(_PyThreadStateImpl *tstate); extern int _Py_ClearUnusedTLBC(PyInterpreterState *interp); #endif + +PyAPI_FUNC(int) _PyCode_Returns(PyCodeObject *); + + #ifdef __cplusplus } #endif diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 226c64f717dc82..be63fce6fb0947 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -1689,6 +1689,46 @@ PyCode_GetFreevars(PyCodeObject *code) return _PyCode_GetFreevars(code); } + +int +_PyCode_Returns(PyCodeObject *co) +{ + // Look up None in co_consts. + Py_ssize_t nconsts = PyTuple_Size(co->co_consts); + int none_index = 0; + for (; none_index < nconsts; none_index++) { + if (PyTuple_GET_ITEM(co->co_consts, none_index) == Py_None) { + break; + } + } + if (none_index == nconsts) { + // None wasn't there, which means there was no implicit return, + // "return", or "return None". That means there must be + // an explicit return (non-None). + return 1; + } + + // Walk the bytecode, looking for RETURN_VALUE. + Py_ssize_t len = Py_SIZE(co); + for (int i = 0; i < len; i++) { + _Py_CODEUNIT inst = _Py_GetBaseCodeUnit(co, i); + if (inst.op.code == RETURN_VALUE) { + assert(i != 0); + // Ignore it if it returns None. + _Py_CODEUNIT prev = _Py_GetBaseCodeUnit(co, i-1); + if (prev.op.code == LOAD_CONST) { + // We don't worry about EXTENDED_ARG for now. + if (prev.op.arg == none_index) { + continue; + } + } + return 1; + } + } + return 0; +} + + #ifdef _Py_TIER2 static void From 076b060ffa318fb12d4812eceee253722ed0deb3 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 28 Apr 2025 19:07:16 -0600 Subject: [PATCH 2/5] _PyCode_Returns() -> _PyCode_ReturnsValue(). --- Include/internal/pycore_code.h | 2 +- Objects/codeobject.c | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h index e7967ad43881d3..093ca9d1f27c28 100644 --- a/Include/internal/pycore_code.h +++ b/Include/internal/pycore_code.h @@ -562,7 +562,7 @@ extern int _Py_ClearUnusedTLBC(PyInterpreterState *interp); #endif -PyAPI_FUNC(int) _PyCode_Returns(PyCodeObject *); +PyAPI_FUNC(int) _PyCode_ReturnsValue(PyCodeObject *); #ifdef __cplusplus diff --git a/Objects/codeobject.c b/Objects/codeobject.c index be63fce6fb0947..fca6b8054bc8c3 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -1690,8 +1690,11 @@ PyCode_GetFreevars(PyCodeObject *code) } +/* Here "value" means a non-None value, since a bare return is identical + * to returning None explicitly. Likewise a missing return statement + * at the end of the function is turned into "return None". */ int -_PyCode_Returns(PyCodeObject *co) +_PyCode_ReturnsValue(PyCodeObject *co) { // Look up None in co_consts. Py_ssize_t nconsts = PyTuple_Size(co->co_consts); From 479bedde516270c9ef89df0105f56921d3a95444 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 28 Apr 2025 19:14:05 -0600 Subject: [PATCH 3/5] Factor out IS_RETURN_OPCODE(). --- Include/internal/pycore_opcode_utils.h | 3 +++ Objects/codeobject.c | 2 +- Python/flowgraph.c | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/Include/internal/pycore_opcode_utils.h b/Include/internal/pycore_opcode_utils.h index b3056e7bb84c69..62af06dc01c125 100644 --- a/Include/internal/pycore_opcode_utils.h +++ b/Include/internal/pycore_opcode_utils.h @@ -54,6 +54,9 @@ extern "C" { (opcode) == RAISE_VARARGS || \ (opcode) == RERAISE) +#define IS_RETURN_OPCODE(opcode) \ + (opcode == RETURN_VALUE) + /* Flags used in the oparg for MAKE_FUNCTION */ #define MAKE_FUNCTION_DEFAULTS 0x01 diff --git a/Objects/codeobject.c b/Objects/codeobject.c index fca6b8054bc8c3..73d76e7fd820fb 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -1715,7 +1715,7 @@ _PyCode_ReturnsValue(PyCodeObject *co) Py_ssize_t len = Py_SIZE(co); for (int i = 0; i < len; i++) { _Py_CODEUNIT inst = _Py_GetBaseCodeUnit(co, i); - if (inst.op.code == RETURN_VALUE) { + if (IS_RETURN_OPCODE(inst.op.code)) { assert(i != 0); // Ignore it if it returns None. _Py_CODEUNIT prev = _Py_GetBaseCodeUnit(co, i-1); diff --git a/Python/flowgraph.c b/Python/flowgraph.c index a0d5690250cffb..423904672e8b11 100644 --- a/Python/flowgraph.c +++ b/Python/flowgraph.c @@ -295,7 +295,7 @@ dump_instr(cfg_instr *i) static inline int basicblock_returns(const basicblock *b) { cfg_instr *last = basicblock_last_instr(b); - return last && last->i_opcode == RETURN_VALUE; + return last && IS_RETURN_OPCODE(last->i_opcode); } static void From 2c801b69bdf2d7fe1ac5551a528ff5596e47a9e7 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 28 Apr 2025 19:42:06 -0600 Subject: [PATCH 4/5] Add tests. --- Lib/test/test_code.py | 59 +++++++++++++++++++++++++++++++++++++ Modules/_testinternalcapi.c | 13 ++++++++ 2 files changed, 72 insertions(+) diff --git a/Lib/test/test_code.py b/Lib/test/test_code.py index 2f459a46b5ad70..a593cd78c13151 100644 --- a/Lib/test/test_code.py +++ b/Lib/test/test_code.py @@ -216,6 +216,10 @@ from test.support.bytecode_helper import instructions_with_positions from opcode import opmap, opname from _testcapi import code_offset_to_line +try: + import _testinternalcapi +except ModuleNotFoundError: + _testinternalcapi = None COPY_FREE_VARS = opmap['COPY_FREE_VARS'] @@ -425,6 +429,61 @@ def func(): with self.assertWarns(DeprecationWarning): func.__code__.co_lnotab + @unittest.skipIf(_testinternalcapi is None, '_testinternalcapi is missing') + def test_returns_value(self): + value = True + + def spam1(): + pass + def spam2(): + return + def spam3(): + return None + def spam4(): + if not value: + return + ... + def spam5(): + if not value: + return None + ... + lambda1 = (lambda: None) + for func in [ + spam1, + spam2, + spam3, + spam4, + spam5, + lambda1, + ]: + with self.subTest(func): + res = _testinternalcapi.code_returns_value(func.__code__) + self.assertFalse(res) + + def spam6(): + return True + def spam7(): + return value + def spam8(): + if value: + return None + return True + def spam9(): + if value: + return True + return None + lambda2 = (lambda: True) + for func in [ + spam6, + spam7, + spam8, + spam9, + lambda2, + ]: + with self.subTest(func): + res = _testinternalcapi.code_returns_value(func.__code__) + self.assertTrue(res) + def test_invalid_bytecode(self): def foo(): pass diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 575e55a0e9c45a..2dbaf235e55d72 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -945,6 +945,18 @@ iframe_getlasti(PyObject *self, PyObject *frame) return PyLong_FromLong(PyUnstable_InterpreterFrame_GetLasti(f)); } +static PyObject * +code_returns_value(PyObject *self, PyObject *arg) +{ + if (!PyCode_Check(arg)) { + PyErr_SetString(PyExc_TypeError, "argument must be a code object"); + return NULL; + } + PyCodeObject *code = (PyCodeObject *)arg; + int res = _PyCode_ReturnsValue(code); + return PyBool_FromLong(res); +} + static PyObject * get_co_framesize(PyObject *self, PyObject *arg) { @@ -2030,6 +2042,7 @@ static PyMethodDef module_functions[] = { {"iframe_getcode", iframe_getcode, METH_O, NULL}, {"iframe_getline", iframe_getline, METH_O, NULL}, {"iframe_getlasti", iframe_getlasti, METH_O, NULL}, + {"code_returns_value", code_returns_value, METH_O, NULL}, {"get_co_framesize", get_co_framesize, METH_O, NULL}, {"jit_enabled", jit_enabled, METH_NOARGS, NULL}, #ifdef _Py_TIER2 From 8482297939ed6035c699a3d485c0a6bcdf22de86 Mon Sep 17 00:00:00 2001 From: Eric Snow Date: Mon, 28 Apr 2025 19:20:02 -0600 Subject: [PATCH 5/5] _PyCode_ReturnsValue() -> _PyCode_ReturnsOnlyNone(). --- Include/internal/pycore_code.h | 2 +- Lib/test/test_code.py | 10 +++++----- Modules/_testinternalcapi.c | 6 +++--- Objects/codeobject.c | 8 ++++---- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h index 093ca9d1f27c28..eced772493ee30 100644 --- a/Include/internal/pycore_code.h +++ b/Include/internal/pycore_code.h @@ -562,7 +562,7 @@ extern int _Py_ClearUnusedTLBC(PyInterpreterState *interp); #endif -PyAPI_FUNC(int) _PyCode_ReturnsValue(PyCodeObject *); +PyAPI_FUNC(int) _PyCode_ReturnsOnlyNone(PyCodeObject *); #ifdef __cplusplus diff --git a/Lib/test/test_code.py b/Lib/test/test_code.py index a593cd78c13151..0c76d38feaae87 100644 --- a/Lib/test/test_code.py +++ b/Lib/test/test_code.py @@ -430,7 +430,7 @@ def func(): func.__code__.co_lnotab @unittest.skipIf(_testinternalcapi is None, '_testinternalcapi is missing') - def test_returns_value(self): + def test_returns_only_none(self): value = True def spam1(): @@ -457,8 +457,8 @@ def spam5(): lambda1, ]: with self.subTest(func): - res = _testinternalcapi.code_returns_value(func.__code__) - self.assertFalse(res) + res = _testinternalcapi.code_returns_only_none(func.__code__) + self.assertTrue(res) def spam6(): return True @@ -481,8 +481,8 @@ def spam9(): lambda2, ]: with self.subTest(func): - res = _testinternalcapi.code_returns_value(func.__code__) - self.assertTrue(res) + res = _testinternalcapi.code_returns_only_none(func.__code__) + self.assertFalse(res) def test_invalid_bytecode(self): def foo(): diff --git a/Modules/_testinternalcapi.c b/Modules/_testinternalcapi.c index 2dbaf235e55d72..643bc90bd4332e 100644 --- a/Modules/_testinternalcapi.c +++ b/Modules/_testinternalcapi.c @@ -946,14 +946,14 @@ iframe_getlasti(PyObject *self, PyObject *frame) } static PyObject * -code_returns_value(PyObject *self, PyObject *arg) +code_returns_only_none(PyObject *self, PyObject *arg) { if (!PyCode_Check(arg)) { PyErr_SetString(PyExc_TypeError, "argument must be a code object"); return NULL; } PyCodeObject *code = (PyCodeObject *)arg; - int res = _PyCode_ReturnsValue(code); + int res = _PyCode_ReturnsOnlyNone(code); return PyBool_FromLong(res); } @@ -2042,7 +2042,7 @@ static PyMethodDef module_functions[] = { {"iframe_getcode", iframe_getcode, METH_O, NULL}, {"iframe_getline", iframe_getline, METH_O, NULL}, {"iframe_getlasti", iframe_getlasti, METH_O, NULL}, - {"code_returns_value", code_returns_value, METH_O, NULL}, + {"code_returns_only_none", code_returns_only_none, METH_O, NULL}, {"get_co_framesize", get_co_framesize, METH_O, NULL}, {"jit_enabled", jit_enabled, METH_NOARGS, NULL}, #ifdef _Py_TIER2 diff --git a/Objects/codeobject.c b/Objects/codeobject.c index 73d76e7fd820fb..bf24a4af445356 100644 --- a/Objects/codeobject.c +++ b/Objects/codeobject.c @@ -1694,7 +1694,7 @@ PyCode_GetFreevars(PyCodeObject *code) * to returning None explicitly. Likewise a missing return statement * at the end of the function is turned into "return None". */ int -_PyCode_ReturnsValue(PyCodeObject *co) +_PyCode_ReturnsOnlyNone(PyCodeObject *co) { // Look up None in co_consts. Py_ssize_t nconsts = PyTuple_Size(co->co_consts); @@ -1708,7 +1708,7 @@ _PyCode_ReturnsValue(PyCodeObject *co) // None wasn't there, which means there was no implicit return, // "return", or "return None". That means there must be // an explicit return (non-None). - return 1; + return 0; } // Walk the bytecode, looking for RETURN_VALUE. @@ -1725,10 +1725,10 @@ _PyCode_ReturnsValue(PyCodeObject *co) continue; } } - return 1; + return 0; } } - return 0; + return 1; }