From 8573510f0ee7f6d3441f8d0b8356be08aab0350c Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Thu, 17 Feb 2022 01:15:07 +0900 Subject: [PATCH 1/2] bpo-46541: Remove usage of _Py_IDENTIFIER from array module --- Include/internal/pycore_global_strings.h | 1 + Include/internal/pycore_runtime_init.h | 1 + Modules/arraymodule.c | 19 +++++++------------ 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h index aa597bc8281a5a..49cce361643dee 100644 --- a/Include/internal/pycore_global_strings.h +++ b/Include/internal/pycore_global_strings.h @@ -194,6 +194,7 @@ struct _Py_global_strings { STRUCT_FOR_ID(__weakref__) STRUCT_FOR_ID(__xor__) STRUCT_FOR_ID(_abc_impl) + STRUCT_FOR_ID(_array_reconstructor) STRUCT_FOR_ID(_blksize) STRUCT_FOR_ID(_dealloc_warn) STRUCT_FOR_ID(_finalizing) diff --git a/Include/internal/pycore_runtime_init.h b/Include/internal/pycore_runtime_init.h index 04c1e671235eae..0e68c6ad3b7c6b 100644 --- a/Include/internal/pycore_runtime_init.h +++ b/Include/internal/pycore_runtime_init.h @@ -809,6 +809,7 @@ extern "C" { INIT_ID(__weakref__), \ INIT_ID(__xor__), \ INIT_ID(_abc_impl), \ + INIT_ID(_array_reconstructor), \ INIT_ID(_blksize), \ INIT_ID(_dealloc_warn), \ INIT_ID(_finalizing), \ diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index e516f54ab61e63..d5ae8c7ad2d081 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -6,12 +6,12 @@ #ifndef Py_BUILD_CORE_BUILTIN # define Py_BUILD_CORE_MODULE 1 #endif -#define NEEDS_PY_IDENTIFIER #define PY_SSIZE_T_CLEAN #include "Python.h" #include "pycore_floatobject.h" // _PyFloat_Unpack4() #include "pycore_moduleobject.h" // _PyModule_GetState() +#include "pycore_runtime.h" // _Py_ID() #include "structmember.h" // PyMemberDef #include // offsetof() #include @@ -1478,7 +1478,6 @@ array_array_fromfile_impl(arrayobject *self, PyObject *f, Py_ssize_t n) PyObject *b, *res; Py_ssize_t itemsize = self->ob_descr->itemsize; Py_ssize_t nbytes; - _Py_IDENTIFIER(read); int not_enough_bytes; if (n < 0) { @@ -1491,7 +1490,7 @@ array_array_fromfile_impl(arrayobject *self, PyObject *f, Py_ssize_t n) } nbytes = n * itemsize; - b = _PyObject_CallMethodId(f, &PyId_read, "n", nbytes); + b = _PyObject_CallMethod(f, &_Py_ID(read), "n", nbytes); if (b == NULL) return NULL; @@ -1546,14 +1545,13 @@ array_array_tofile(arrayobject *self, PyObject *f) char* ptr = self->ob_item + i*BLOCKSIZE; Py_ssize_t size = BLOCKSIZE; PyObject *bytes, *res; - _Py_IDENTIFIER(write); if (i*BLOCKSIZE + size > nbytes) size = nbytes - i*BLOCKSIZE; bytes = PyBytes_FromStringAndSize(ptr, size); if (bytes == NULL) return NULL; - res = _PyObject_CallMethodIdOneArg(f, &PyId_write, bytes); + res = PyObject_CallMethodOneArg(f, &_Py_ID(write), bytes); Py_DECREF(bytes); if (res == NULL) return NULL; @@ -2193,16 +2191,14 @@ array_array___reduce_ex__(arrayobject *self, PyObject *value) int mformat_code; static PyObject *array_reconstructor = NULL; long protocol; - _Py_IDENTIFIER(_array_reconstructor); - _Py_IDENTIFIER(__dict__); if (array_reconstructor == NULL) { PyObject *array_module = PyImport_ImportModule("array"); if (array_module == NULL) return NULL; - array_reconstructor = _PyObject_GetAttrId( + array_reconstructor = PyObject_GetAttr( array_module, - &PyId__array_reconstructor); + &_Py_ID(_array_reconstructor)); Py_DECREF(array_module); if (array_reconstructor == NULL) return NULL; @@ -2217,7 +2213,7 @@ array_array___reduce_ex__(arrayobject *self, PyObject *value) if (protocol == -1 && PyErr_Occurred()) return NULL; - if (_PyObject_LookupAttrId((PyObject *)self, &PyId___dict__, &dict) < 0) { + if (_PyObject_LookupAttr((PyObject *)self, &_Py_ID(__dict__), &dict) < 0) { return NULL; } if (dict == NULL) { @@ -2946,8 +2942,7 @@ static PyObject * array_arrayiterator___reduce___impl(arrayiterobject *self) /*[clinic end generated code: output=7898a52e8e66e016 input=a062ea1e9951417a]*/ { - _Py_IDENTIFIER(iter); - PyObject *func = _PyEval_GetBuiltinId(&PyId_iter); + PyObject *func = _PyEval_GetBuiltin(&_Py_ID(iter)); if (self->ao == NULL) { return Py_BuildValue("N(())", func); } From c9a28a2b718846306d923caef14981f79f7683be Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Thu, 17 Feb 2022 11:10:58 +0900 Subject: [PATCH 2/2] bpo-46541: Address code review --- Include/internal/pycore_global_strings.h | 1 - Include/internal/pycore_runtime_init.h | 1 - Modules/arraymodule.c | 82 ++++++++++++++++---- Modules/clinic/arraymodule.c.h | 95 +++++++++++++++++------- 4 files changed, 138 insertions(+), 41 deletions(-) diff --git a/Include/internal/pycore_global_strings.h b/Include/internal/pycore_global_strings.h index 49cce361643dee..aa597bc8281a5a 100644 --- a/Include/internal/pycore_global_strings.h +++ b/Include/internal/pycore_global_strings.h @@ -194,7 +194,6 @@ struct _Py_global_strings { STRUCT_FOR_ID(__weakref__) STRUCT_FOR_ID(__xor__) STRUCT_FOR_ID(_abc_impl) - STRUCT_FOR_ID(_array_reconstructor) STRUCT_FOR_ID(_blksize) STRUCT_FOR_ID(_dealloc_warn) STRUCT_FOR_ID(_finalizing) diff --git a/Include/internal/pycore_runtime_init.h b/Include/internal/pycore_runtime_init.h index 0e68c6ad3b7c6b..04c1e671235eae 100644 --- a/Include/internal/pycore_runtime_init.h +++ b/Include/internal/pycore_runtime_init.h @@ -809,7 +809,6 @@ extern "C" { INIT_ID(__weakref__), \ INIT_ID(__xor__), \ INIT_ID(_abc_impl), \ - INIT_ID(_array_reconstructor), \ INIT_ID(_blksize), \ INIT_ID(_dealloc_warn), \ INIT_ID(_finalizing), \ diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c index d5ae8c7ad2d081..2d6da1aaac85a9 100644 --- a/Modules/arraymodule.c +++ b/Modules/arraymodule.c @@ -11,7 +11,6 @@ #include "Python.h" #include "pycore_floatobject.h" // _PyFloat_Unpack4() #include "pycore_moduleobject.h" // _PyModule_GetState() -#include "pycore_runtime.h" // _Py_ID() #include "structmember.h" // PyMemberDef #include // offsetof() #include @@ -58,6 +57,12 @@ typedef struct { typedef struct { PyTypeObject *ArrayType; PyTypeObject *ArrayIterType; + + PyObject *str_read; + PyObject *str_write; + PyObject *str__array_reconstructor; + PyObject *str___dict__; + PyObject *str_iter; } array_state; static array_state * @@ -1464,6 +1469,7 @@ array_array_reverse_impl(arrayobject *self) /*[clinic input] array.array.fromfile + cls: defining_class f: object n: Py_ssize_t / @@ -1472,8 +1478,9 @@ Read n objects from the file object f and append them to the end of the array. [clinic start generated code]*/ static PyObject * -array_array_fromfile_impl(arrayobject *self, PyObject *f, Py_ssize_t n) -/*[clinic end generated code: output=ec9f600e10f53510 input=e188afe8e58adf40]*/ +array_array_fromfile_impl(arrayobject *self, PyTypeObject *cls, PyObject *f, + Py_ssize_t n) +/*[clinic end generated code: output=83a667080b345ebc input=3822e907c1c11f1a]*/ { PyObject *b, *res; Py_ssize_t itemsize = self->ob_descr->itemsize; @@ -1488,9 +1495,14 @@ array_array_fromfile_impl(arrayobject *self, PyObject *f, Py_ssize_t n) PyErr_NoMemory(); return NULL; } + + + array_state *state = get_array_state_by_class(cls); + assert(state != NULL); + nbytes = n * itemsize; - b = _PyObject_CallMethod(f, &_Py_ID(read), "n", nbytes); + b = _PyObject_CallMethod(f, state->str_read, "n", nbytes); if (b == NULL) return NULL; @@ -1521,6 +1533,7 @@ array_array_fromfile_impl(arrayobject *self, PyObject *f, Py_ssize_t n) /*[clinic input] array.array.tofile + cls: defining_class f: object / @@ -1528,8 +1541,8 @@ Write all items (as machine values) to the file object f. [clinic start generated code]*/ static PyObject * -array_array_tofile(arrayobject *self, PyObject *f) -/*[clinic end generated code: output=3a2cfa8128df0777 input=b0669a484aab0831]*/ +array_array_tofile_impl(arrayobject *self, PyTypeObject *cls, PyObject *f) +/*[clinic end generated code: output=4560c628d9c18bc2 input=5a24da7a7b407b52]*/ { Py_ssize_t nbytes = Py_SIZE(self) * self->ob_descr->itemsize; /* Write 64K blocks at a time */ @@ -1541,6 +1554,10 @@ array_array_tofile(arrayobject *self, PyObject *f) if (Py_SIZE(self) == 0) goto done; + + array_state *state = get_array_state_by_class(cls); + assert(state != NULL); + for (i = 0; i < nblocks; i++) { char* ptr = self->ob_item + i*BLOCKSIZE; Py_ssize_t size = BLOCKSIZE; @@ -1551,7 +1568,7 @@ array_array_tofile(arrayobject *self, PyObject *f) bytes = PyBytes_FromStringAndSize(ptr, size); if (bytes == NULL) return NULL; - res = PyObject_CallMethodOneArg(f, &_Py_ID(write), bytes); + res = PyObject_CallMethodOneArg(f, state->str_write, bytes); Py_DECREF(bytes); if (res == NULL) return NULL; @@ -2174,6 +2191,7 @@ array__array_reconstructor_impl(PyObject *module, PyTypeObject *arraytype, /*[clinic input] array.array.__reduce_ex__ + cls: defining_class value: object / @@ -2181,8 +2199,9 @@ Return state information for pickling. [clinic start generated code]*/ static PyObject * -array_array___reduce_ex__(arrayobject *self, PyObject *value) -/*[clinic end generated code: output=051e0a6175d0eddb input=c36c3f85de7df6cd]*/ +array_array___reduce_ex___impl(arrayobject *self, PyTypeObject *cls, + PyObject *value) +/*[clinic end generated code: output=4958ee5d79452ad5 input=19968cf0f91d3eea]*/ { PyObject *dict; PyObject *result; @@ -2192,13 +2211,16 @@ array_array___reduce_ex__(arrayobject *self, PyObject *value) static PyObject *array_reconstructor = NULL; long protocol; + array_state *state = get_array_state_by_class(cls); + assert(state != NULL); + if (array_reconstructor == NULL) { PyObject *array_module = PyImport_ImportModule("array"); if (array_module == NULL) return NULL; array_reconstructor = PyObject_GetAttr( array_module, - &_Py_ID(_array_reconstructor)); + state->str__array_reconstructor); Py_DECREF(array_module); if (array_reconstructor == NULL) return NULL; @@ -2213,7 +2235,7 @@ array_array___reduce_ex__(arrayobject *self, PyObject *value) if (protocol == -1 && PyErr_Occurred()) return NULL; - if (_PyObject_LookupAttr((PyObject *)self, &_Py_ID(__dict__), &dict) < 0) { + if (_PyObject_LookupAttr((PyObject *)self, state->str___dict__, &dict) < 0) { return NULL; } if (dict == NULL) { @@ -2935,14 +2957,20 @@ arrayiter_traverse(arrayiterobject *it, visitproc visit, void *arg) /*[clinic input] array.arrayiterator.__reduce__ + cls: defining_class + / + Return state information for pickling. [clinic start generated code]*/ static PyObject * -array_arrayiterator___reduce___impl(arrayiterobject *self) -/*[clinic end generated code: output=7898a52e8e66e016 input=a062ea1e9951417a]*/ +array_arrayiterator___reduce___impl(arrayiterobject *self, PyTypeObject *cls) +/*[clinic end generated code: output=4b032417a2c8f5e6 input=ac64e65a87ad452e]*/ { - PyObject *func = _PyEval_GetBuiltin(&_Py_ID(iter)); + + array_state *state = get_array_state_by_class(cls); + assert(state != NULL); + PyObject *func = _PyEval_GetBuiltin(state->str_iter); if (self->ao == NULL) { return Py_BuildValue("N(())", func); } @@ -3006,6 +3034,11 @@ array_traverse(PyObject *module, visitproc visit, void *arg) array_state *state = get_array_state(module); Py_VISIT(state->ArrayType); Py_VISIT(state->ArrayIterType); + Py_VISIT(state->str_read); + Py_VISIT(state->str_write); + Py_VISIT(state->str__array_reconstructor); + Py_VISIT(state->str___dict__); + Py_VISIT(state->str_iter); return 0; } @@ -3015,6 +3048,11 @@ array_clear(PyObject *module) array_state *state = get_array_state(module); Py_CLEAR(state->ArrayType); Py_CLEAR(state->ArrayIterType); + Py_CLEAR(state->str_read); + Py_CLEAR(state->str_write); + Py_CLEAR(state->str__array_reconstructor); + Py_CLEAR(state->str___dict__); + Py_CLEAR(state->str_iter); return 0; } @@ -3038,6 +3076,15 @@ do { \ } \ } while (0) +#define ADD_INTERNED(state, string) \ +do { \ + PyObject *tmp = PyUnicode_InternFromString(#string); \ + if (tmp == NULL) { \ + return -1; \ + } \ + state->str_ ## string = tmp; \ +} while (0) + static int array_modexec(PyObject *m) { @@ -3046,6 +3093,13 @@ array_modexec(PyObject *m) PyObject *typecodes; const struct arraydescr *descr; + /* Add interned strings */ + ADD_INTERNED(state, read); + ADD_INTERNED(state, write); + ADD_INTERNED(state, _array_reconstructor); + ADD_INTERNED(state, __dict__); + ADD_INTERNED(state, iter); + CREATE_TYPE(m, state->ArrayType, &array_spec); CREATE_TYPE(m, state->ArrayIterType, &arrayiter_spec); Py_SET_TYPE(state->ArrayIterType, &PyType_Type); diff --git a/Modules/clinic/arraymodule.c.h b/Modules/clinic/arraymodule.c.h index c46cc738de91b7..583ff28bf3287e 100644 --- a/Modules/clinic/arraymodule.c.h +++ b/Modules/clinic/arraymodule.c.h @@ -285,35 +285,26 @@ PyDoc_STRVAR(array_array_fromfile__doc__, "Read n objects from the file object f and append them to the end of the array."); #define ARRAY_ARRAY_FROMFILE_METHODDEF \ - {"fromfile", (PyCFunction)(void(*)(void))array_array_fromfile, METH_FASTCALL, array_array_fromfile__doc__}, + {"fromfile", (PyCFunction)(void(*)(void))array_array_fromfile, METH_METHOD|METH_FASTCALL|METH_KEYWORDS, array_array_fromfile__doc__}, static PyObject * -array_array_fromfile_impl(arrayobject *self, PyObject *f, Py_ssize_t n); +array_array_fromfile_impl(arrayobject *self, PyTypeObject *cls, PyObject *f, + Py_ssize_t n); static PyObject * -array_array_fromfile(arrayobject *self, PyObject *const *args, Py_ssize_t nargs) +array_array_fromfile(arrayobject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { PyObject *return_value = NULL; + static const char * const _keywords[] = {"", "", NULL}; + static _PyArg_Parser _parser = {"On:fromfile", _keywords, 0}; PyObject *f; Py_ssize_t n; - if (!_PyArg_CheckPositional("fromfile", nargs, 2, 2)) { + if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser, + &f, &n)) { goto exit; } - f = args[0]; - { - Py_ssize_t ival = -1; - PyObject *iobj = _PyNumber_Index(args[1]); - if (iobj != NULL) { - ival = PyLong_AsSsize_t(iobj); - Py_DECREF(iobj); - } - if (ival == -1 && PyErr_Occurred()) { - goto exit; - } - n = ival; - } - return_value = array_array_fromfile_impl(self, f, n); + return_value = array_array_fromfile_impl(self, cls, f, n); exit: return return_value; @@ -326,7 +317,28 @@ PyDoc_STRVAR(array_array_tofile__doc__, "Write all items (as machine values) to the file object f."); #define ARRAY_ARRAY_TOFILE_METHODDEF \ - {"tofile", (PyCFunction)array_array_tofile, METH_O, array_array_tofile__doc__}, + {"tofile", (PyCFunction)(void(*)(void))array_array_tofile, METH_METHOD|METH_FASTCALL|METH_KEYWORDS, array_array_tofile__doc__}, + +static PyObject * +array_array_tofile_impl(arrayobject *self, PyTypeObject *cls, PyObject *f); + +static PyObject * +array_array_tofile(arrayobject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +{ + PyObject *return_value = NULL; + static const char * const _keywords[] = {"", NULL}; + static _PyArg_Parser _parser = {"O:tofile", _keywords, 0}; + PyObject *f; + + if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser, + &f)) { + goto exit; + } + return_value = array_array_tofile_impl(self, cls, f); + +exit: + return return_value; +} PyDoc_STRVAR(array_array_fromlist__doc__, "fromlist($self, list, /)\n" @@ -544,7 +556,29 @@ PyDoc_STRVAR(array_array___reduce_ex____doc__, "Return state information for pickling."); #define ARRAY_ARRAY___REDUCE_EX___METHODDEF \ - {"__reduce_ex__", (PyCFunction)array_array___reduce_ex__, METH_O, array_array___reduce_ex____doc__}, + {"__reduce_ex__", (PyCFunction)(void(*)(void))array_array___reduce_ex__, METH_METHOD|METH_FASTCALL|METH_KEYWORDS, array_array___reduce_ex____doc__}, + +static PyObject * +array_array___reduce_ex___impl(arrayobject *self, PyTypeObject *cls, + PyObject *value); + +static PyObject * +array_array___reduce_ex__(arrayobject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) +{ + PyObject *return_value = NULL; + static const char * const _keywords[] = {"", NULL}; + static _PyArg_Parser _parser = {"O:__reduce_ex__", _keywords, 0}; + PyObject *value; + + if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser, + &value)) { + goto exit; + } + return_value = array_array___reduce_ex___impl(self, cls, value); + +exit: + return return_value; +} PyDoc_STRVAR(array_arrayiterator___reduce____doc__, "__reduce__($self, /)\n" @@ -553,15 +587,26 @@ PyDoc_STRVAR(array_arrayiterator___reduce____doc__, "Return state information for pickling."); #define ARRAY_ARRAYITERATOR___REDUCE___METHODDEF \ - {"__reduce__", (PyCFunction)array_arrayiterator___reduce__, METH_NOARGS, array_arrayiterator___reduce____doc__}, + {"__reduce__", (PyCFunction)(void(*)(void))array_arrayiterator___reduce__, METH_METHOD|METH_FASTCALL|METH_KEYWORDS, array_arrayiterator___reduce____doc__}, static PyObject * -array_arrayiterator___reduce___impl(arrayiterobject *self); +array_arrayiterator___reduce___impl(arrayiterobject *self, PyTypeObject *cls); static PyObject * -array_arrayiterator___reduce__(arrayiterobject *self, PyObject *Py_UNUSED(ignored)) +array_arrayiterator___reduce__(arrayiterobject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { - return array_arrayiterator___reduce___impl(self); + PyObject *return_value = NULL; + static const char * const _keywords[] = { NULL}; + static _PyArg_Parser _parser = {":__reduce__", _keywords, 0}; + + if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser + )) { + goto exit; + } + return_value = array_arrayiterator___reduce___impl(self, cls); + +exit: + return return_value; } PyDoc_STRVAR(array_arrayiterator___setstate____doc__, @@ -572,4 +617,4 @@ PyDoc_STRVAR(array_arrayiterator___setstate____doc__, #define ARRAY_ARRAYITERATOR___SETSTATE___METHODDEF \ {"__setstate__", (PyCFunction)array_arrayiterator___setstate__, METH_O, array_arrayiterator___setstate____doc__}, -/*[clinic end generated code: output=f130a994f98f1227 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=1db6decd8492bf91 input=a9049054013a1b77]*/