Skip to content

Commit 375706b

Browse files
ericsnowcurrentlypull[bot]
ericsnowcurrently
authored and
pull[bot]
committed
gh-76785: Clean Up Interpreter ID Conversions (gh-117048)
Mostly we unify the two different implementations of the conversion code (from PyObject * to int64_t. We also drop the PyArg_ParseTuple()-style converter function, as well as rename and move PyInterpreterID_LookUp().
1 parent fc7eedc commit 375706b

File tree

8 files changed

+143
-178
lines changed

8 files changed

+143
-178
lines changed

‎Include/cpython/interpreteridobject.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,7 @@ PyAPI_DATA(PyTypeObject) PyInterpreterID_Type;
88

99
PyAPI_FUNC(PyObject *) PyInterpreterID_New(int64_t);
1010
PyAPI_FUNC(PyObject *) PyInterpreterState_GetIDObject(PyInterpreterState *);
11-
PyAPI_FUNC(PyInterpreterState *) PyInterpreterID_LookUp(PyObject *);
11+
12+
#ifdef Py_BUILD_CORE
13+
extern int64_t _PyInterpreterID_GetID(PyObject *);
14+
#endif

‎Include/internal/pycore_interp.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,8 +295,11 @@ _PyInterpreterState_SetFinalizing(PyInterpreterState *interp, PyThreadState *tst
295295
}
296296

297297

298+
extern int64_t _PyInterpreterState_ObjectToID(PyObject *);
299+
298300
// Export for the _xxinterpchannels module.
299301
PyAPI_FUNC(PyInterpreterState *) _PyInterpreterState_LookUpID(int64_t);
302+
PyAPI_FUNC(PyInterpreterState *) _PyInterpreterState_LookUpIDObject(PyObject *);
300303

301304
PyAPI_FUNC(int) _PyInterpreterState_IDInitref(PyInterpreterState *);
302305
PyAPI_FUNC(int) _PyInterpreterState_IDIncref(PyInterpreterState *);

‎Lib/test/test_capi/test_misc.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2303,7 +2303,7 @@ def test_equality(self):
23032303

23042304
def test_linked_lifecycle(self):
23052305
id1 = _interpreters.create()
2306-
_testcapi.unlink_interpreter_refcount(id1)
2306+
_testinternalcapi.unlink_interpreter_refcount(id1)
23072307
self.assertEqual(
23082308
_testinternalcapi.get_interpreter_refcount(id1),
23092309
0)
@@ -2319,7 +2319,7 @@ def test_linked_lifecycle(self):
23192319
_testinternalcapi.get_interpreter_refcount(id1),
23202320
0)
23212321

2322-
_testcapi.link_interpreter_refcount(id1)
2322+
_testinternalcapi.link_interpreter_refcount(id1)
23232323
self.assertEqual(
23242324
_testinternalcapi.get_interpreter_refcount(id1),
23252325
0)

‎Modules/_testcapimodule.c

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1455,30 +1455,6 @@ get_interpreterid_type(PyObject *self, PyObject *Py_UNUSED(ignored))
14551455
return Py_NewRef(&PyInterpreterID_Type);
14561456
}
14571457

1458-
static PyObject *
1459-
link_interpreter_refcount(PyObject *self, PyObject *idobj)
1460-
{
1461-
PyInterpreterState *interp = PyInterpreterID_LookUp(idobj);
1462-
if (interp == NULL) {
1463-
assert(PyErr_Occurred());
1464-
return NULL;
1465-
}
1466-
_PyInterpreterState_RequireIDRef(interp, 1);
1467-
Py_RETURN_NONE;
1468-
}
1469-
1470-
static PyObject *
1471-
unlink_interpreter_refcount(PyObject *self, PyObject *idobj)
1472-
{
1473-
PyInterpreterState *interp = PyInterpreterID_LookUp(idobj);
1474-
if (interp == NULL) {
1475-
assert(PyErr_Occurred());
1476-
return NULL;
1477-
}
1478-
_PyInterpreterState_RequireIDRef(interp, 0);
1479-
Py_RETURN_NONE;
1480-
}
1481-
14821458
static PyMethodDef ml;
14831459

14841460
static PyObject *
@@ -3324,8 +3300,6 @@ static PyMethodDef TestMethods[] = {
33243300
{"test_current_tstate_matches", test_current_tstate_matches, METH_NOARGS},
33253301
{"run_in_subinterp", run_in_subinterp, METH_VARARGS},
33263302
{"get_interpreterid_type", get_interpreterid_type, METH_NOARGS},
3327-
{"link_interpreter_refcount", link_interpreter_refcount, METH_O},
3328-
{"unlink_interpreter_refcount", unlink_interpreter_refcount, METH_O},
33293303
{"create_cfunction", create_cfunction, METH_NOARGS},
33303304
{"call_in_temporary_c_thread", call_in_temporary_c_thread, METH_VARARGS,
33313305
PyDoc_STR("set_error_class(error_class) -> None")},

‎Modules/_testinternalcapi.c

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,6 @@
2929
#include "pycore_pyerrors.h" // _PyErr_ChainExceptions1()
3030
#include "pycore_pystate.h" // _PyThreadState_GET()
3131

32-
#include "interpreteridobject.h" // PyInterpreterID_LookUp()
33-
3432
#include "clinic/_testinternalcapi.c.h"
3533

3634
// Include test definitions from _testinternalcapi/
@@ -1112,7 +1110,7 @@ pending_identify(PyObject *self, PyObject *args)
11121110
if (!PyArg_ParseTuple(args, "O:pending_identify", &interpid)) {
11131111
return NULL;
11141112
}
1115-
PyInterpreterState *interp = PyInterpreterID_LookUp(interpid);
1113+
PyInterpreterState *interp = _PyInterpreterState_LookUpIDObject(interpid);
11161114
if (interp == NULL) {
11171115
if (!PyErr_Occurred()) {
11181116
PyErr_SetString(PyExc_ValueError, "interpreter not found");
@@ -1480,13 +1478,37 @@ run_in_subinterp_with_config(PyObject *self, PyObject *args, PyObject *kwargs)
14801478
static PyObject *
14811479
get_interpreter_refcount(PyObject *self, PyObject *idobj)
14821480
{
1483-
PyInterpreterState *interp = PyInterpreterID_LookUp(idobj);
1481+
PyInterpreterState *interp = _PyInterpreterState_LookUpIDObject(idobj);
14841482
if (interp == NULL) {
14851483
return NULL;
14861484
}
14871485
return PyLong_FromLongLong(interp->id_refcount);
14881486
}
14891487

1488+
static PyObject *
1489+
link_interpreter_refcount(PyObject *self, PyObject *idobj)
1490+
{
1491+
PyInterpreterState *interp = _PyInterpreterState_LookUpIDObject(idobj);
1492+
if (interp == NULL) {
1493+
assert(PyErr_Occurred());
1494+
return NULL;
1495+
}
1496+
_PyInterpreterState_RequireIDRef(interp, 1);
1497+
Py_RETURN_NONE;
1498+
}
1499+
1500+
static PyObject *
1501+
unlink_interpreter_refcount(PyObject *self, PyObject *idobj)
1502+
{
1503+
PyInterpreterState *interp = _PyInterpreterState_LookUpIDObject(idobj);
1504+
if (interp == NULL) {
1505+
assert(PyErr_Occurred());
1506+
return NULL;
1507+
}
1508+
_PyInterpreterState_RequireIDRef(interp, 0);
1509+
Py_RETURN_NONE;
1510+
}
1511+
14901512

14911513
static void
14921514
_xid_capsule_destructor(PyObject *capsule)
@@ -1728,6 +1750,8 @@ static PyMethodDef module_functions[] = {
17281750
_PyCFunction_CAST(run_in_subinterp_with_config),
17291751
METH_VARARGS | METH_KEYWORDS},
17301752
{"get_interpreter_refcount", get_interpreter_refcount, METH_O},
1753+
{"link_interpreter_refcount", link_interpreter_refcount, METH_O},
1754+
{"unlink_interpreter_refcount", unlink_interpreter_refcount, METH_O},
17311755
{"compile_perf_trampoline_entry", compile_perf_trampoline_entry, METH_VARARGS},
17321756
{"perf_trampoline_set_persist_after_fork", perf_trampoline_set_persist_after_fork, METH_VARARGS},
17331757
{"get_crossinterp_data", get_crossinterp_data, METH_VARARGS},

‎Modules/_xxsubinterpretersmodule.c

Lines changed: 4 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -35,83 +35,8 @@ _get_current_interp(void)
3535
return PyInterpreterState_Get();
3636
}
3737

38-
static int64_t
39-
pylong_to_interpid(PyObject *idobj)
40-
{
41-
assert(PyLong_CheckExact(idobj));
42-
43-
if (_PyLong_IsNegative((PyLongObject *)idobj)) {
44-
PyErr_Format(PyExc_ValueError,
45-
"interpreter ID must be a non-negative int, got %R",
46-
idobj);
47-
return -1;
48-
}
49-
50-
int overflow;
51-
long long id = PyLong_AsLongLongAndOverflow(idobj, &overflow);
52-
if (id == -1) {
53-
if (!overflow) {
54-
assert(PyErr_Occurred());
55-
return -1;
56-
}
57-
assert(!PyErr_Occurred());
58-
// For now, we don't worry about if LLONG_MAX < INT64_MAX.
59-
goto bad_id;
60-
}
61-
#if LLONG_MAX > INT64_MAX
62-
if (id > INT64_MAX) {
63-
goto bad_id;
64-
}
65-
#endif
66-
return (int64_t)id;
67-
68-
bad_id:
69-
PyErr_Format(PyExc_RuntimeError,
70-
"unrecognized interpreter ID %O", idobj);
71-
return -1;
72-
}
73-
74-
static int64_t
75-
convert_interpid_obj(PyObject *arg)
76-
{
77-
int64_t id = -1;
78-
if (_PyIndex_Check(arg)) {
79-
PyObject *idobj = PyNumber_Long(arg);
80-
if (idobj == NULL) {
81-
return -1;
82-
}
83-
id = pylong_to_interpid(idobj);
84-
Py_DECREF(idobj);
85-
if (id < 0) {
86-
return -1;
87-
}
88-
}
89-
else {
90-
PyErr_Format(PyExc_TypeError,
91-
"interpreter ID must be an int, got %.100s",
92-
Py_TYPE(arg)->tp_name);
93-
return -1;
94-
}
95-
return id;
96-
}
97-
98-
static PyInterpreterState *
99-
look_up_interp(PyObject *arg)
100-
{
101-
int64_t id = convert_interpid_obj(arg);
102-
if (id < 0) {
103-
return NULL;
104-
}
105-
return _PyInterpreterState_LookUpID(id);
106-
}
107-
38+
#define look_up_interp _PyInterpreterState_LookUpIDObject
10839

109-
static PyObject *
110-
interpid_to_pylong(int64_t id)
111-
{
112-
assert(id < LLONG_MAX);
113-
return PyLong_FromLongLong(id);
114-
}
11540

11641
static PyObject *
11742
get_interpid_obj(PyInterpreterState *interp)
@@ -123,7 +48,8 @@ get_interpid_obj(PyInterpreterState *interp)
12348
if (id < 0) {
12449
return NULL;
12550
}
126-
return interpid_to_pylong(id);
51+
assert(id < LLONG_MAX);
52+
return PyLong_FromLongLong(id);
12753
}
12854

12955
static PyObject *
@@ -699,7 +625,7 @@ interp_set___main___attrs(PyObject *self, PyObject *args)
699625
}
700626

701627
// Look up the interpreter.
702-
PyInterpreterState *interp = PyInterpreterID_LookUp(id);
628+
PyInterpreterState *interp = look_up_interp(id);
703629
if (interp == NULL) {
704630
return NULL;
705631
}

‎Objects/interpreteridobject.c

Lines changed: 23 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
/* InterpreterID object */
22

33
#include "Python.h"
4-
#include "pycore_abstract.h" // _PyIndex_Check()
5-
#include "pycore_interp.h" // _PyInterpreterState_LookUpID()
4+
#include "pycore_interp.h" // _PyInterpreterState_LookUpID()
65
#include "interpreteridobject.h"
76

87

@@ -11,6 +10,21 @@ typedef struct interpid {
1110
int64_t id;
1211
} interpid;
1312

13+
int64_t
14+
_PyInterpreterID_GetID(PyObject *self)
15+
{
16+
if (!PyObject_TypeCheck(self, &PyInterpreterID_Type)) {
17+
PyErr_Format(PyExc_TypeError,
18+
"expected an InterpreterID, got %R",
19+
self);
20+
return -1;
21+
22+
}
23+
int64_t id = ((interpid *)self)->id;
24+
assert(id >= 0);
25+
return id;
26+
}
27+
1428
static interpid *
1529
newinterpid(PyTypeObject *cls, int64_t id, int force)
1630
{
@@ -42,43 +56,19 @@ newinterpid(PyTypeObject *cls, int64_t id, int force)
4256
return self;
4357
}
4458

45-
static int
46-
interp_id_converter(PyObject *arg, void *ptr)
47-
{
48-
int64_t id;
49-
if (PyObject_TypeCheck(arg, &PyInterpreterID_Type)) {
50-
id = ((interpid *)arg)->id;
51-
}
52-
else if (_PyIndex_Check(arg)) {
53-
id = PyLong_AsLongLong(arg);
54-
if (id == -1 && PyErr_Occurred()) {
55-
return 0;
56-
}
57-
if (id < 0) {
58-
PyErr_Format(PyExc_ValueError,
59-
"interpreter ID must be a non-negative int, got %R", arg);
60-
return 0;
61-
}
62-
}
63-
else {
64-
PyErr_Format(PyExc_TypeError,
65-
"interpreter ID must be an int, got %.100s",
66-
Py_TYPE(arg)->tp_name);
67-
return 0;
68-
}
69-
*(int64_t *)ptr = id;
70-
return 1;
71-
}
72-
7359
static PyObject *
7460
interpid_new(PyTypeObject *cls, PyObject *args, PyObject *kwds)
7561
{
7662
static char *kwlist[] = {"id", "force", NULL};
77-
int64_t id;
63+
PyObject *idobj;
7864
int force = 0;
7965
if (!PyArg_ParseTupleAndKeywords(args, kwds,
80-
"O&|$p:InterpreterID.__init__", kwlist,
81-
interp_id_converter, &id, &force)) {
66+
"O|$p:InterpreterID.__init__", kwlist,
67+
&idobj, &force)) {
68+
return NULL;
69+
}
70+
int64_t id = _PyInterpreterState_ObjectToID(idobj);
71+
if (id < 0) {
8272
return NULL;
8373
}
8474

@@ -282,13 +272,3 @@ PyInterpreterState_GetIDObject(PyInterpreterState *interp)
282272
}
283273
return (PyObject *)newinterpid(&PyInterpreterID_Type, id, 0);
284274
}
285-
286-
PyInterpreterState *
287-
PyInterpreterID_LookUp(PyObject *requested_id)
288-
{
289-
int64_t id;
290-
if (!interp_id_converter(requested_id, &id)) {
291-
return NULL;
292-
}
293-
return _PyInterpreterState_LookUpID(id);
294-
}

0 commit comments

Comments
 (0)