From c1c4a6099c4a7e80d38aea16846f5b7e6827a57e Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Thu, 23 Jul 2020 15:34:56 -0500 Subject: [PATCH 01/11] ENH,API: Store exported buffer info on the array This speeds up array deallocation and buffer exports, since it removes the need to global dictionary lookups. It also somewhat simplifies the logic. The main advantage is prossibly less the speedup itself (which is not large compared to most things that happen in the livetime of an array), but rather that no unnecessary work is done for shortlived arrays, which never export a buffer. The downside of this approach is that the ABI changes for anyone who would be subclassing ndarray in C. --- doc/release/upcoming_changes/16938.c_api.rst | 17 + numpy/core/include/numpy/arrayscalars.h | 1 + numpy/core/include/numpy/ndarraytypes.h | 3 +- numpy/core/src/multiarray/arrayobject.c | 9 +- numpy/core/src/multiarray/buffer.c | 345 ++++++++----------- numpy/core/src/multiarray/ctors.c | 2 + numpy/core/src/multiarray/npy_buffer.h | 16 +- numpy/core/src/multiarray/scalartypes.c.src | 25 +- 8 files changed, 211 insertions(+), 207 deletions(-) create mode 100644 doc/release/upcoming_changes/16938.c_api.rst diff --git a/doc/release/upcoming_changes/16938.c_api.rst b/doc/release/upcoming_changes/16938.c_api.rst new file mode 100644 index 000000000000..260046991fe0 --- /dev/null +++ b/doc/release/upcoming_changes/16938.c_api.rst @@ -0,0 +1,17 @@ +Size of ``np.ndarray`` and ``np.void_`` changed +----------------------------------------------- +The size of the ``PyArrayObject`` and ``PyVoidScalarObject`` +structures have changed. This also modifies the header +definition:: + + #define NPY_SIZEOF_PYARRAYOBJECT (PyArray_Type.tp_basicsize) + +since the size should not be considered a compile time constant. + +In the case that your code subclasses any of these on the C-level, +it will have to be recompiled and should add guards against such +size changes for the future. NumPy will attempt to give a graceful +warning, but in many cases crashes are likely if this is done. + +We are not aware of any user impacted by this change. If your +project is, please get in touch with the NumPy developers. diff --git a/numpy/core/include/numpy/arrayscalars.h b/numpy/core/include/numpy/arrayscalars.h index b282a2cd40f5..14a31988fe42 100644 --- a/numpy/core/include/numpy/arrayscalars.h +++ b/numpy/core/include/numpy/arrayscalars.h @@ -149,6 +149,7 @@ typedef struct { PyArray_Descr *descr; int flags; PyObject *base; + void *_buffer_info; /* private buffer info, tagged to allow warning */ } PyVoidScalarObject; /* Macros diff --git a/numpy/core/include/numpy/ndarraytypes.h b/numpy/core/include/numpy/ndarraytypes.h index 6bf54938f2b2..ef219d6e1079 100644 --- a/numpy/core/include/numpy/ndarraytypes.h +++ b/numpy/core/include/numpy/ndarraytypes.h @@ -701,6 +701,7 @@ typedef struct tagPyArrayObject_fields { int flags; /* For weak references */ PyObject *weakreflist; + void *_buffer_info; /* private buffer info, tagged to allow warning */ } PyArrayObject_fields; /* @@ -720,7 +721,7 @@ typedef struct tagPyArrayObject { } PyArrayObject; #endif -#define NPY_SIZEOF_PYARRAYOBJECT (sizeof(PyArrayObject_fields)) +#define NPY_SIZEOF_PYARRAYOBJECT (PyArray_Type.tp_basicsize) /* Array Flags Object */ typedef struct PyArrayFlagsObject { diff --git a/numpy/core/src/multiarray/arrayobject.c b/numpy/core/src/multiarray/arrayobject.c index 5da1b5f2902e..2638d17e2812 100644 --- a/numpy/core/src/multiarray/arrayobject.c +++ b/numpy/core/src/multiarray/arrayobject.c @@ -432,9 +432,11 @@ WARN_IN_DEALLOC(PyObject* warning, const char * msg) { static void array_dealloc(PyArrayObject *self) { - PyArrayObject_fields *fa = (PyArrayObject_fields *)self; + PyArrayObject_fields *fa = (PyArrayObject_fields *) self; - _dealloc_cached_buffer_info((PyObject*)self); + if (_buffer_info_free(fa->_buffer_info, (PyObject *)self) < 0) { + PyErr_WriteUnraisable(NULL); + } if (fa->weakreflist != NULL) { PyObject_ClearWeakRefs((PyObject *)self); @@ -1731,6 +1733,7 @@ array_alloc(PyTypeObject *type, Py_ssize_t NPY_UNUSED(nitems)) /* nitems will always be 0 */ PyObject *obj = PyObject_Malloc(type->tp_basicsize); PyObject_Init(obj, type); + return obj; } @@ -1745,7 +1748,7 @@ array_free(PyObject * v) NPY_NO_EXPORT PyTypeObject PyArray_Type = { PyVarObject_HEAD_INIT(NULL, 0) .tp_name = "numpy.ndarray", - .tp_basicsize = NPY_SIZEOF_PYARRAYOBJECT, + .tp_basicsize = sizeof(PyArrayObject_fields), /* methods */ .tp_dealloc = (destructor)array_dealloc, .tp_repr = (reprfunc)array_repr, diff --git a/numpy/core/src/multiarray/buffer.c b/numpy/core/src/multiarray/buffer.c index 1f4f676ba5ce..d3c6bfe0f6d3 100644 --- a/numpy/core/src/multiarray/buffer.c +++ b/numpy/core/src/multiarray/buffer.c @@ -428,31 +428,23 @@ _buffer_format_string(PyArray_Descr *descr, _tmp_string_t *str, /* - * Global information about all active buffers + * Information about all active buffers is stored as a linked list on + * the ndarray. The initial pointer is currently tagged to have a chance of + * detecting incompatible subclasses. * * Note: because for backward compatibility we cannot define bf_releasebuffer, * we must manually keep track of the additional data required by the buffers. */ /* Additional per-array data required for providing the buffer interface */ -typedef struct { +typedef struct _buffer_info_t_tag { char *format; int ndim; Py_ssize_t *strides; Py_ssize_t *shape; + struct _buffer_info_t_tag *next; } _buffer_info_t; -/* - * { id(array): [list of pointers to _buffer_info_t, the last one is latest] } - * - * Because shape, strides, and format can be different for different buffers, - * we may need to keep track of multiple buffer infos for each array. - * - * However, when none of them has changed, the same buffer info may be reused. - * - * Thread-safety is provided by GIL. - */ -static PyObject *_buffer_info_cache = NULL; /* Fill in the info structure */ static _buffer_info_t* @@ -564,6 +556,7 @@ _buffer_info_new(PyObject *obj, int flags) Py_DECREF(descr); info->format = NULL; } + info->next = NULL; return info; fail: @@ -596,144 +589,141 @@ _buffer_info_cmp(_buffer_info_t *a, _buffer_info_t *b) return 0; } + +/* + * We tag the pointer by adding 2, to ensure that we are most likely to + * notice if a C defined subclass extends our structure and does not take + * into account that it may grow. + */ +static int +_buffer_info_untag( + void *tagged_buffer_info, _buffer_info_t **buffer_info, PyObject *obj) +{ + if (NPY_UNLIKELY(((uintptr_t)tagged_buffer_info & 0x7) != 2)) { + PyErr_Format(PyExc_RuntimeError, + "Object of type %S appears to be C subclassed NumPy array, " + "void scalar, or allocated in a non-standard way." + "NumPy reserves the right to change the size of these " + "structures. Projects are required to take this into account " + "by either recompiling against a specific NumPy version or " + "padding the struct and enforcing a maximum NumPy version.", + Py_TYPE(obj)); + return -1; + } + *buffer_info = (void *)((uintptr_t)tagged_buffer_info - 2); + return 0; +} + + +/* + * NOTE: for backward compatibility (esp. with PyArg_ParseTuple("s#", ...)) + * we do *not* define bf_releasebuffer at all. + * + * Instead, any extra data allocated with the buffer is released only in + * array_dealloc. + * + * Ensuring that the buffer stays in place is taken care by refcounting; + * ndarrays do not reallocate if there are references to them, and a buffer + * view holds one reference. + * + * This is stored in the array's _buffer_info slot (currently as a void *). + */ static void -_buffer_info_free(_buffer_info_t *info) +_buffer_info_free_untagged(void *_buffer_info) { - if (info->format) { - PyObject_Free(info->format); + _buffer_info_t *next = _buffer_info; + while (next != NULL) { + _buffer_info_t *curr = next; + next = curr->next; + if (curr->format) { + PyObject_Free(curr->format); + } + /* Shape is allocated as part of info */ + PyObject_Free(curr); } - PyObject_Free(info); } -/* Get buffer info from the global dictionary */ + +/* + * Checks whether the pointer is tagged, and then frees the cache list. + * (The tag check is only for transition due to changed structure size in 1.20) + */ +NPY_NO_EXPORT int +_buffer_info_free(void *buffer_info, PyObject *obj) +{ + _buffer_info_t *untagged_buffer_info; + if (_buffer_info_untag(buffer_info, &untagged_buffer_info, obj) < 0) { + return -1; + } + _buffer_info_free_untagged(untagged_buffer_info); + return 0; +} + + +/* + * Get the buffer info returning either the old one (passed in) or a new + * buffer info which adds holds on to (and thus replaces) the old one. + */ static _buffer_info_t* -_buffer_get_info(PyObject *obj, int flags) +_buffer_get_info(void **buffer_info_cache_ptr, PyObject *obj, int flags) { - PyObject *key = NULL, *item_list = NULL, *item = NULL; - _buffer_info_t *info = NULL, *old_info = NULL; + _buffer_info_t *info = NULL; + _buffer_info_t *stored_info; /* First currently stored buffer info */ - if (_buffer_info_cache == NULL) { - _buffer_info_cache = PyDict_New(); - if (_buffer_info_cache == NULL) { - return NULL; - } + if (_buffer_info_untag(*buffer_info_cache_ptr, &stored_info, obj) < 0) { + return NULL; } + _buffer_info_t *old_info = stored_info; - /* Compute information */ + /* Compute information (it would be nice to skip this in simple cases) */ info = _buffer_info_new(obj, flags); if (info == NULL) { return NULL; } - /* Check if it is identical with an old one; reuse old one, if yes */ - key = PyLong_FromVoidPtr((void*)obj); - if (key == NULL) { - goto fail; + if (old_info != NULL && _buffer_info_cmp(info, old_info) != 0) { + _buffer_info_t *next_info = old_info->next; + old_info = NULL; /* Can't use this one, but possibly next */ + + if (info->ndim > 1 && next_info != NULL) { + /* + * Some arrays are C- and F-contiguous and if they have more + * than one dimension, the buffer-info may differ between + * the two due to RELAXED_STRIDES_CHECKING. + * If we export both buffers, the first stored one may be + * the one for the other contiguity, so check both. + * This is generally very unlikely in all other cases, since + * in all other cases the first one will match unless array + * metadata was modified in-place (which is discouraged). + */ + if (_buffer_info_cmp(info, next_info) == 0) { + old_info = next_info; + } + } } - item_list = PyDict_GetItem(_buffer_info_cache, key); - - if (item_list != NULL) { - Py_ssize_t item_list_length = PyList_GET_SIZE(item_list); - Py_INCREF(item_list); - if (item_list_length > 0) { - item = PyList_GetItem(item_list, item_list_length - 1); - old_info = (_buffer_info_t*)PyLong_AsVoidPtr(item); - if (_buffer_info_cmp(info, old_info) != 0) { - old_info = NULL; /* Can't use this one, but possibly next */ - - if (item_list_length > 1 && info->ndim > 1) { - /* - * Some arrays are C- and F-contiguous and if they have more - * than one dimension, the buffer-info may differ between - * the two due to RELAXED_STRIDES_CHECKING. - * If we export both buffers, the first stored one may be - * the one for the other contiguity, so check both. - * This is generally very unlikely in all other cases, since - * in all other cases the first one will match unless array - * metadata was modified in-place (which is discouraged). - */ - item = PyList_GetItem(item_list, item_list_length - 2); - old_info = (_buffer_info_t*)PyLong_AsVoidPtr(item); - if (_buffer_info_cmp(info, old_info) != 0) { - old_info = NULL; - } - } - } - - if (old_info != NULL) { - /* - * The two info->format are considered equal if one of them - * has no format set (meaning the format is arbitrary and can - * be modified). If the new info has a format, but we reuse - * the old one, this transfers the ownership to the old one. - */ - if (old_info->format == NULL) { - old_info->format = info->format; - info->format = NULL; - } - _buffer_info_free(info); - info = old_info; - } + if (old_info != NULL) { + /* + * The two info->format are considered equal if one of them + * has no format set (meaning the format is arbitrary and can + * be modified). If the new info has a format, but we reuse + * the old one, this transfers the ownership to the old one. + */ + if (old_info->format == NULL) { + old_info->format = info->format; + info->format = NULL; } + _buffer_info_free_untagged(info); + info = old_info; } else { - item_list = PyList_New(0); - if (item_list == NULL) { - goto fail; - } - if (PyDict_SetItem(_buffer_info_cache, key, item_list) != 0) { - goto fail; - } - } - - if (info != old_info) { - /* Needs insertion */ - item = PyLong_FromVoidPtr((void*)info); - if (item == NULL) { - goto fail; - } - PyList_Append(item_list, item); - Py_DECREF(item); + /* Insert new info as first item in the linked buffer-info list. */ + info->next = stored_info; + *buffer_info_cache_ptr = buffer_info_tag(info); } - Py_DECREF(item_list); - Py_DECREF(key); return info; - -fail: - if (info != NULL && info != old_info) { - _buffer_info_free(info); - } - Py_XDECREF(item_list); - Py_XDECREF(key); - return NULL; } -/* Clear buffer info from the global dictionary */ -static void -_buffer_clear_info(PyObject *arr) -{ - PyObject *key, *item_list, *item; - _buffer_info_t *info; - int k; - - if (_buffer_info_cache == NULL) { - return; - } - - key = PyLong_FromVoidPtr((void*)arr); - item_list = PyDict_GetItem(_buffer_info_cache, key); - if (item_list != NULL) { - for (k = 0; k < PyList_GET_SIZE(item_list); ++k) { - item = PyList_GET_ITEM(item_list, k); - info = (_buffer_info_t*)PyLong_AsVoidPtr(item); - _buffer_info_free(info); - } - PyDict_DelItem(_buffer_info_cache, key); - } - - Py_DECREF(key); -} /* * Retrieving buffers for ndarray @@ -779,8 +769,9 @@ array_getbuffer(PyObject *obj, Py_buffer *view, int flags) goto fail; } - /* Fill in information */ - info = _buffer_get_info(obj, flags); + /* Fill in information (and add it to _buffer_info if necessary) */ + info = _buffer_get_info( + &((PyArrayObject_fields *)self)->_buffer_info, obj, flags); if (info == NULL) { goto fail; } @@ -830,90 +821,48 @@ array_getbuffer(PyObject *obj, Py_buffer *view, int flags) } /* - * Retrieving buffers for scalars + * Retrieving buffers for void scalar (which can contain any complex types), + * defined in buffer.c since it requires the complex format building logic. */ -int +NPY_NO_EXPORT int void_getbuffer(PyObject *self, Py_buffer *view, int flags) { - _buffer_info_t *info = NULL; - PyArray_Descr *descr = NULL; - int elsize; + PyVoidScalarObject *scalar = (PyVoidScalarObject *)self; if (flags & PyBUF_WRITABLE) { PyErr_SetString(PyExc_BufferError, "scalar buffer is readonly"); - goto fail; - } - - /* Fill in information */ - info = _buffer_get_info(self, flags); - if (info == NULL) { - goto fail; - } - - view->ndim = info->ndim; - view->shape = info->shape; - view->strides = info->strides; - - if ((flags & PyBUF_FORMAT) == PyBUF_FORMAT) { - view->format = info->format; - } else { - view->format = NULL; - } - - descr = PyArray_DescrFromScalar(self); - view->buf = (void *)scalar_value(self, descr); - elsize = descr->elsize; - view->len = elsize; - if (PyArray_IsScalar(self, Datetime) || PyArray_IsScalar(self, Timedelta)) { - elsize = 1; /* descr->elsize,char is 8,'M', but we return 1,'B' */ + return -1; } - view->itemsize = elsize; - - Py_DECREF(descr); + view->ndim = 0; + view->shape = NULL; + view->strides = NULL; + view->suboffsets = NULL; + view->len = scalar->descr->elsize; + view->itemsize = scalar->descr->elsize; view->readonly = 1; view->suboffsets = NULL; - view->obj = self; Py_INCREF(self); - return 0; - -fail: - view->obj = NULL; - return -1; -} - -/* - * NOTE: for backward compatibility (esp. with PyArg_ParseTuple("s#", ...)) - * we do *not* define bf_releasebuffer at all. - * - * Instead, any extra data allocated with the buffer is released only in - * array_dealloc. - * - * Ensuring that the buffer stays in place is taken care by refcounting; - * ndarrays do not reallocate if there are references to them, and a buffer - * view holds one reference. - */ - -NPY_NO_EXPORT void -_dealloc_cached_buffer_info(PyObject *self) -{ - int reset_error_state = 0; - PyObject *ptype, *pvalue, *ptraceback; - - /* This function may be called when processing an exception -- - * we need to stash the error state to avoid confusing PyDict - */ + view->obj = self; + view->buf = scalar->obval; - if (PyErr_Occurred()) { - reset_error_state = 1; - PyErr_Fetch(&ptype, &pvalue, &ptraceback); + if (((flags & PyBUF_FORMAT) != PyBUF_FORMAT)) { + /* It is unnecessary to find the correct format */ + view->format = NULL; + return 0; } - _buffer_clear_info(self); - - if (reset_error_state) { - PyErr_Restore(ptype, pvalue, ptraceback); + /* + * If a format is being exported, we need to use _buffer_get_info + * to find the correct format. This format must also be stored, since + * at least in theory it can change (in practice it should never change). + */ + _buffer_info_t *info = _buffer_get_info(&scalar->_buffer_info, self, flags); + if (info == NULL) { + return -1; } + view->format = info->format; + return 0; } diff --git a/numpy/core/src/multiarray/ctors.c b/numpy/core/src/multiarray/ctors.c index 2426076b98ef..e0830dddd4e3 100644 --- a/numpy/core/src/multiarray/ctors.c +++ b/numpy/core/src/multiarray/ctors.c @@ -752,6 +752,8 @@ PyArray_NewFromDescr_int( } fa = (PyArrayObject_fields *) subtype->tp_alloc(subtype, 0); + ((PyArrayObject_fields *)fa)->_buffer_info = buffer_info_tag(NULL); + if (fa == NULL) { Py_DECREF(descr); return NULL; diff --git a/numpy/core/src/multiarray/npy_buffer.h b/numpy/core/src/multiarray/npy_buffer.h index 5ff8b6c2c600..b4c655ce3674 100644 --- a/numpy/core/src/multiarray/npy_buffer.h +++ b/numpy/core/src/multiarray/npy_buffer.h @@ -3,8 +3,20 @@ extern NPY_NO_EXPORT PyBufferProcs array_as_buffer; -NPY_NO_EXPORT void -_dealloc_cached_buffer_info(PyObject *self); +NPY_NO_EXPORT int +_buffer_info_free(void *buffer_info, PyObject *obj); + +/* + * Tag the buffer info pointer. This was appended to the array struct + * in NumPy 1.20, tagging the pointer gives us a chance to raise/print + * a useful error message when a user modifies fields that should belong to + * us. + */ +static NPY_INLINE void * +buffer_info_tag(void *buffer_info) +{ + return (void *)((uintptr_t)buffer_info + 2); +} NPY_NO_EXPORT PyArray_Descr* _descriptor_from_pep3118_format(char const *s); diff --git a/numpy/core/src/multiarray/scalartypes.c.src b/numpy/core/src/multiarray/scalartypes.c.src index b8976d08f815..03d9aa57de5f 100644 --- a/numpy/core/src/multiarray/scalartypes.c.src +++ b/numpy/core/src/multiarray/scalartypes.c.src @@ -67,8 +67,11 @@ gentype_alloc(PyTypeObject *type, Py_ssize_t nitems) const size_t size = _PyObject_VAR_SIZE(type, nitems + 1); obj = (PyObject *)PyObject_Malloc(size); + if (obj == NULL) { + PyErr_NoMemory(); + return NULL; + } /* - * Fixme. Need to check for no memory. * If we don't need to zero memory, we could use * PyObject_{New, NewVar} for this whole function. */ @@ -2596,16 +2599,30 @@ NPY_NO_EXPORT PyTypeObject PyGenericArrType_Type = { .tp_basicsize = sizeof(PyObject), }; + +static PyObject * +void_alloc(PyTypeObject *type, Py_ssize_t items) +{ + PyVoidScalarObject *self = (PyVoidScalarObject *)gentype_alloc(type, items); + if (self == NULL) { + return NULL; + } + self->_buffer_info = buffer_info_tag(NULL); + return (PyObject *)self; +} + + static void void_dealloc(PyVoidScalarObject *v) { - _dealloc_cached_buffer_info((PyObject *)v); - if (v->flags & NPY_ARRAY_OWNDATA) { npy_free_cache(v->obval, Py_SIZE(v)); } Py_XDECREF(v->descr); Py_XDECREF(v->base); + if (_buffer_info_free(v->_buffer_info, (PyObject *)v) < 0) { + PyErr_WriteUnraisable(NULL); + } Py_TYPE(v)->tp_free(v); } @@ -3014,6 +3031,7 @@ void_arrtype_new(PyTypeObject *type, PyObject *args, PyObject *kwds) npy_free_cache(destptr, memu); return PyErr_NoMemory(); } + ((PyVoidScalarObject *)ret)->_buffer_info = buffer_info_tag(NULL); ((PyVoidScalarObject *)ret)->obval = destptr; Py_SET_SIZE((PyVoidScalarObject *)ret, (int) memu); ((PyVoidScalarObject *)ret)->descr = @@ -4010,6 +4028,7 @@ initialize_numeric_types(void) /**end repeat**/ PyStringArrType_Type.tp_itemsize = sizeof(char); + PyVoidArrType_Type.tp_alloc = void_alloc; PyVoidArrType_Type.tp_dealloc = (destructor) void_dealloc; PyArrayIter_Type.tp_iter = PyObject_SelfIter; From 2030884ad0cbb2307b060912956a008d78f0dcb4 Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Fri, 6 Nov 2020 19:01:39 -0600 Subject: [PATCH 02/11] MAINT: Do not tag the NULL (no buffers exported) The allocation is not the right place to initialize to anything but NULL, so take the easy path and do not tag the NULL default. --- numpy/core/src/multiarray/arrayobject.c | 3 +-- numpy/core/src/multiarray/buffer.c | 27 ++++++++++++++++++--- numpy/core/src/multiarray/ctors.c | 4 +-- numpy/core/src/multiarray/npy_buffer.h | 12 --------- numpy/core/src/multiarray/scalartypes.c.src | 14 ----------- 5 files changed, 26 insertions(+), 34 deletions(-) diff --git a/numpy/core/src/multiarray/arrayobject.c b/numpy/core/src/multiarray/arrayobject.c index 2638d17e2812..a2474d79fcf5 100644 --- a/numpy/core/src/multiarray/arrayobject.c +++ b/numpy/core/src/multiarray/arrayobject.c @@ -432,7 +432,7 @@ WARN_IN_DEALLOC(PyObject* warning, const char * msg) { static void array_dealloc(PyArrayObject *self) { - PyArrayObject_fields *fa = (PyArrayObject_fields *) self; + PyArrayObject_fields *fa = (PyArrayObject_fields *)self; if (_buffer_info_free(fa->_buffer_info, (PyObject *)self) < 0) { PyErr_WriteUnraisable(NULL); @@ -1733,7 +1733,6 @@ array_alloc(PyTypeObject *type, Py_ssize_t NPY_UNUSED(nitems)) /* nitems will always be 0 */ PyObject *obj = PyObject_Malloc(type->tp_basicsize); PyObject_Init(obj, type); - return obj; } diff --git a/numpy/core/src/multiarray/buffer.c b/numpy/core/src/multiarray/buffer.c index d3c6bfe0f6d3..cd0f96778902 100644 --- a/numpy/core/src/multiarray/buffer.c +++ b/numpy/core/src/multiarray/buffer.c @@ -591,14 +591,33 @@ _buffer_info_cmp(_buffer_info_t *a, _buffer_info_t *b) /* - * We tag the pointer by adding 2, to ensure that we are most likely to - * notice if a C defined subclass extends our structure and does not take - * into account that it may grow. + * Tag the buffer info pointer by adding 2 (unless it is NULL to simplify + * object initialization). + * The linked list of buffer-infos was appended to the array struct in + * NumPy 1.20. Tagging the pointer gives us a chance to raise/print + * a useful error message instead of crashing hard if a C-subclass uses + * the same field. */ -static int +static NPY_INLINE void * +buffer_info_tag(void *buffer_info) +{ + if (buffer_info == NULL) { + return buffer_info; + } + else { + return (void *)((uintptr_t)buffer_info + 2); + } +} + + +static NPY_INLINE int _buffer_info_untag( void *tagged_buffer_info, _buffer_info_t **buffer_info, PyObject *obj) { + if (tagged_buffer_info == NULL) { + *buffer_info = NULL; + return 0; + } if (NPY_UNLIKELY(((uintptr_t)tagged_buffer_info & 0x7) != 2)) { PyErr_Format(PyExc_RuntimeError, "Object of type %S appears to be C subclassed NumPy array, " diff --git a/numpy/core/src/multiarray/ctors.c b/numpy/core/src/multiarray/ctors.c index e0830dddd4e3..f6031e370fe7 100644 --- a/numpy/core/src/multiarray/ctors.c +++ b/numpy/core/src/multiarray/ctors.c @@ -752,15 +752,15 @@ PyArray_NewFromDescr_int( } fa = (PyArrayObject_fields *) subtype->tp_alloc(subtype, 0); - ((PyArrayObject_fields *)fa)->_buffer_info = buffer_info_tag(NULL); - if (fa == NULL) { Py_DECREF(descr); return NULL; } + fa->_buffer_info = NULL; fa->nd = nd; fa->dimensions = NULL; fa->data = NULL; + if (data == NULL) { fa->flags = NPY_ARRAY_DEFAULT; if (flags) { diff --git a/numpy/core/src/multiarray/npy_buffer.h b/numpy/core/src/multiarray/npy_buffer.h index b4c655ce3674..d10f1a020446 100644 --- a/numpy/core/src/multiarray/npy_buffer.h +++ b/numpy/core/src/multiarray/npy_buffer.h @@ -6,18 +6,6 @@ extern NPY_NO_EXPORT PyBufferProcs array_as_buffer; NPY_NO_EXPORT int _buffer_info_free(void *buffer_info, PyObject *obj); -/* - * Tag the buffer info pointer. This was appended to the array struct - * in NumPy 1.20, tagging the pointer gives us a chance to raise/print - * a useful error message when a user modifies fields that should belong to - * us. - */ -static NPY_INLINE void * -buffer_info_tag(void *buffer_info) -{ - return (void *)((uintptr_t)buffer_info + 2); -} - NPY_NO_EXPORT PyArray_Descr* _descriptor_from_pep3118_format(char const *s); diff --git a/numpy/core/src/multiarray/scalartypes.c.src b/numpy/core/src/multiarray/scalartypes.c.src index 03d9aa57de5f..d018fccbbd22 100644 --- a/numpy/core/src/multiarray/scalartypes.c.src +++ b/numpy/core/src/multiarray/scalartypes.c.src @@ -2600,18 +2600,6 @@ NPY_NO_EXPORT PyTypeObject PyGenericArrType_Type = { }; -static PyObject * -void_alloc(PyTypeObject *type, Py_ssize_t items) -{ - PyVoidScalarObject *self = (PyVoidScalarObject *)gentype_alloc(type, items); - if (self == NULL) { - return NULL; - } - self->_buffer_info = buffer_info_tag(NULL); - return (PyObject *)self; -} - - static void void_dealloc(PyVoidScalarObject *v) { @@ -3031,7 +3019,6 @@ void_arrtype_new(PyTypeObject *type, PyObject *args, PyObject *kwds) npy_free_cache(destptr, memu); return PyErr_NoMemory(); } - ((PyVoidScalarObject *)ret)->_buffer_info = buffer_info_tag(NULL); ((PyVoidScalarObject *)ret)->obval = destptr; Py_SET_SIZE((PyVoidScalarObject *)ret, (int) memu); ((PyVoidScalarObject *)ret)->descr = @@ -4028,7 +4015,6 @@ initialize_numeric_types(void) /**end repeat**/ PyStringArrType_Type.tp_itemsize = sizeof(char); - PyVoidArrType_Type.tp_alloc = void_alloc; PyVoidArrType_Type.tp_dealloc = (destructor) void_dealloc; PyArrayIter_Type.tp_iter = PyObject_SelfIter; From ee682f6e2fb37b4b8785a87b00adbf1d09d4f87c Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Thu, 12 Nov 2020 13:20:01 -0600 Subject: [PATCH 03/11] TST: Add test for best try RuntimeError on corrupt buffer-info --- .../src/multiarray/_multiarray_tests.c.src | 36 +++++++++++++++++++ numpy/core/tests/test_multiarray.py | 19 ++++++++++ 2 files changed, 55 insertions(+) diff --git a/numpy/core/src/multiarray/_multiarray_tests.c.src b/numpy/core/src/multiarray/_multiarray_tests.c.src index 5b6b6dc78fe0..e6a090dc4691 100644 --- a/numpy/core/src/multiarray/_multiarray_tests.c.src +++ b/numpy/core/src/multiarray/_multiarray_tests.c.src @@ -36,6 +36,7 @@ IsPythonScalar(PyObject * dummy, PyObject *args) #include "npy_pycompat.h" + /** Function to test calling via ctypes */ EXPORT(void*) forward_pointer(void *x) { @@ -684,6 +685,38 @@ create_custom_field_dtype(PyObject *NPY_UNUSED(mod), PyObject *args) } +PyObject * +corrupt_or_fix_bufferinfo(PyObject *dummy, PyObject *obj) +{ + void **buffer_info_ptr; + if (PyArray_Check(obj)) { + buffer_info_ptr = &((PyArrayObject_fields *)obj)->_buffer_info; + } + else if (PyArray_IsScalar(obj, Void)) { + buffer_info_ptr = &((PyVoidScalarObject *)obj)->_buffer_info; + } + else { + PyErr_SetString(PyExc_TypeError, + "argument must be an array or void scalar"); + return NULL; + } + if (*buffer_info_ptr == NULL) { + /* set to an invalid value (as a subclass might accidentally) */ + *buffer_info_ptr = &corrupt_or_fix_bufferinfo; + } + else if (*buffer_info_ptr == &corrupt_or_fix_bufferinfo) { + /* Reset to a NULL (good value) */ + *buffer_info_ptr = NULL; + } + else { + PyErr_SetString(PyExc_TypeError, + "buffer was already exported, this test doesn't support that"); + return NULL; + } + Py_RETURN_NONE; +} + + /* check no elison for avoided increfs */ static PyObject * incref_elide(PyObject *dummy, PyObject *args) @@ -2158,6 +2191,9 @@ static PyMethodDef Multiarray_TestsMethods[] = { {"create_custom_field_dtype", create_custom_field_dtype, METH_VARARGS, NULL}, + {"corrupt_or_fix_bufferinfo", + corrupt_or_fix_bufferinfo, + METH_O, NULL}, {"incref_elide", incref_elide, METH_VARARGS, NULL}, diff --git a/numpy/core/tests/test_multiarray.py b/numpy/core/tests/test_multiarray.py index 61806f99f455..12306cbb883d 100644 --- a/numpy/core/tests/test_multiarray.py +++ b/numpy/core/tests/test_multiarray.py @@ -7526,6 +7526,25 @@ class foo(ctypes.Structure): f.a = 3 assert_equal(arr['a'], 3) + @pytest.mark.parametrize("obj", [np.ones(3), np.ones(1, dtype="i,i")[()]]) + def test_error_if_stored_buffer_info_is_corrupted(self, obj): + """ + If a user extends a NumPy array before 1.20 and then runs it + on NumPy 1.20+. A C-subclassed array might in theory modify + the new buffer-info field. This checks that an error is raised + if this happens (for buffer export), an error is written on delete. + This is a sanity check to help users transition to safe code, it + may be deleted at any point. + """ + # corrupt buffer info: + _multiarray_tests.corrupt_or_fix_bufferinfo(obj) + name = type(obj) + with pytest.raises(RuntimeError, + match=f".*{name} appears to be C subclassed"): + memoryview(obj) + # Fix buffer info again before we delete (or we lose the memory) + _multiarray_tests.corrupt_or_fix_bufferinfo(obj) + class TestArrayAttributeDeletion: From 6c2e8cd716ee69c786f9461931e2ff372a3452a7 Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Sun, 22 Nov 2020 13:40:24 -0600 Subject: [PATCH 04/11] Remove NPY_SIZEOF_PYARRAYOBJECT and add some documentation --- doc/release/upcoming_changes/16938.c_api.rst | 21 ++++++++-------- .../reference/c-api/types-and-structures.rst | 24 +++++++++++++++++++ numpy/core/include/numpy/ndarraytypes.h | 1 - numpy/core/src/multiarray/methods.c | 2 +- 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/doc/release/upcoming_changes/16938.c_api.rst b/doc/release/upcoming_changes/16938.c_api.rst index 260046991fe0..202d7df9598f 100644 --- a/doc/release/upcoming_changes/16938.c_api.rst +++ b/doc/release/upcoming_changes/16938.c_api.rst @@ -1,17 +1,18 @@ Size of ``np.ndarray`` and ``np.void_`` changed ----------------------------------------------- The size of the ``PyArrayObject`` and ``PyVoidScalarObject`` -structures have changed. This also modifies the header -definition:: +structures have changed. The following header definition has been +removed:: - #define NPY_SIZEOF_PYARRAYOBJECT (PyArray_Type.tp_basicsize) + #define NPY_SIZEOF_PYARRAYOBJECT (sizeof(PyArrayObject_fields)) -since the size should not be considered a compile time constant. +since the size must not be considered a compile time constant. -In the case that your code subclasses any of these on the C-level, -it will have to be recompiled and should add guards against such -size changes for the future. NumPy will attempt to give a graceful -warning, but in many cases crashes are likely if this is done. +The most likely relevant use are potential subclasses written in C which +will have to be recompiled and should be updated. Please see the +documentation for :c:type:`PyArrayObject` for more details and contact +the NumPy developers if you are affected by this change. + +NumPy will attempt to give a graceful error but a program expecting a +fixed structure size may have undefined behaviour and likely crash. -We are not aware of any user impacted by this change. If your -project is, please get in touch with the NumPy developers. diff --git a/doc/source/reference/c-api/types-and-structures.rst b/doc/source/reference/c-api/types-and-structures.rst index 6a9c4a9cf766..763f985a6577 100644 --- a/doc/source/reference/c-api/types-and-structures.rst +++ b/doc/source/reference/c-api/types-and-structures.rst @@ -79,6 +79,8 @@ PyArray_Type and PyArrayObject of :c:type:`NPY_AO` (deprecated) which is defined to be equivalent to :c:type:`PyArrayObject`. Direct access to the struct fields are deprecated. Use the ``PyArray_*(arr)`` form instead. + As of NumPy 1.20, the size of this struct is not considered part of + the NumPy ABI (see note at the end of the member list). .. code-block:: c @@ -92,6 +94,7 @@ PyArray_Type and PyArrayObject PyArray_Descr *descr; int flags; PyObject *weakreflist; + /* version dependend private members */ } PyArrayObject; .. c:macro:: PyObject_HEAD @@ -173,6 +176,27 @@ PyArray_Type and PyArrayObject This member allows array objects to have weak references (using the weakref module). + .. note:: + + Further members are considered private and version dependend. If the size + of the struct is important for your code, special care must be taken. + A possible use-case when this is relevant is subclassing in C. + If your code relies on ``sizeof(PyArrayObject)`` to be constant, + you must add the following check at import time: + + .. code-block:: c + + if (sizeof(PyArrayObject) < PyArray_Type.tp_basicsize) { + PyErr_SetString(PyExc_ImportError, + "Binary incompatibility with NumPy, must recompile/update X."); + return NULL; + } + + To ensure that your code does not have to be compiled for a specific + NumPy version, you may add a constant, leaving room for changes in NumPy. + A solution guaranteed to be compatible with any future NumPy version + requires the use of a runtime calculate offset and allocation size. + PyArrayDescr_Type and PyArray_Descr ----------------------------------- diff --git a/numpy/core/include/numpy/ndarraytypes.h b/numpy/core/include/numpy/ndarraytypes.h index ef219d6e1079..cb2a13435077 100644 --- a/numpy/core/include/numpy/ndarraytypes.h +++ b/numpy/core/include/numpy/ndarraytypes.h @@ -721,7 +721,6 @@ typedef struct tagPyArrayObject { } PyArrayObject; #endif -#define NPY_SIZEOF_PYARRAYOBJECT (PyArray_Type.tp_basicsize) /* Array Flags Object */ typedef struct PyArrayFlagsObject { diff --git a/numpy/core/src/multiarray/methods.c b/numpy/core/src/multiarray/methods.c index 76df2337b9d7..9c8bb4135005 100644 --- a/numpy/core/src/multiarray/methods.c +++ b/numpy/core/src/multiarray/methods.c @@ -2180,7 +2180,7 @@ static PyObject * array_sizeof(PyArrayObject *self) { /* object + dimension and strides */ - Py_ssize_t nbytes = NPY_SIZEOF_PYARRAYOBJECT + + Py_ssize_t nbytes = Py_TYPE(self)->tp_basicsize + PyArray_NDIM(self) * sizeof(npy_intp) * 2; if (PyArray_CHKFLAGS(self, NPY_ARRAY_OWNDATA)) { nbytes += PyArray_NBYTES(self); From 70565ea8756a0b8607fbb7bcb032240f81db5f7e Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Mon, 23 Nov 2020 12:31:41 -0600 Subject: [PATCH 05/11] Use 3 to tag the pointer and object for a "bad" one in the test --- numpy/core/src/multiarray/_multiarray_tests.c.src | 5 +++-- numpy/core/src/multiarray/buffer.c | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/numpy/core/src/multiarray/_multiarray_tests.c.src b/numpy/core/src/multiarray/_multiarray_tests.c.src index e6a090dc4691..7780a2de4c31 100644 --- a/numpy/core/src/multiarray/_multiarray_tests.c.src +++ b/numpy/core/src/multiarray/_multiarray_tests.c.src @@ -702,9 +702,10 @@ corrupt_or_fix_bufferinfo(PyObject *dummy, PyObject *obj) } if (*buffer_info_ptr == NULL) { /* set to an invalid value (as a subclass might accidentally) */ - *buffer_info_ptr = &corrupt_or_fix_bufferinfo; + *buffer_info_ptr = obj; + assert(((uintptr_t)obj & 7) == 0); } - else if (*buffer_info_ptr == &corrupt_or_fix_bufferinfo) { + else if (*buffer_info_ptr == obj) { /* Reset to a NULL (good value) */ *buffer_info_ptr = NULL; } diff --git a/numpy/core/src/multiarray/buffer.c b/numpy/core/src/multiarray/buffer.c index cd0f96778902..813850224714 100644 --- a/numpy/core/src/multiarray/buffer.c +++ b/numpy/core/src/multiarray/buffer.c @@ -605,7 +605,7 @@ buffer_info_tag(void *buffer_info) return buffer_info; } else { - return (void *)((uintptr_t)buffer_info + 2); + return (void *)((uintptr_t)buffer_info + 3); } } @@ -618,7 +618,7 @@ _buffer_info_untag( *buffer_info = NULL; return 0; } - if (NPY_UNLIKELY(((uintptr_t)tagged_buffer_info & 0x7) != 2)) { + if (NPY_UNLIKELY(((uintptr_t)tagged_buffer_info & 0x7) != 3)) { PyErr_Format(PyExc_RuntimeError, "Object of type %S appears to be C subclassed NumPy array, " "void scalar, or allocated in a non-standard way." @@ -629,7 +629,7 @@ _buffer_info_untag( Py_TYPE(obj)); return -1; } - *buffer_info = (void *)((uintptr_t)tagged_buffer_info - 2); + *buffer_info = (void *)((uintptr_t)tagged_buffer_info - 3); return 0; } From 3149ad451cc84483401a277d1a899d8ffc06ef10 Mon Sep 17 00:00:00 2001 From: mattip Date: Wed, 25 Nov 2020 12:10:13 +0200 Subject: [PATCH 06/11] DEP: deprecate the NPY_SIZEOF_PYARRAYOBJECT macro --- doc/release/upcoming_changes/16938.c_api.rst | 5 +++-- numpy/core/include/numpy/ndarraytypes.h | 6 ++++++ numpy/core/src/multiarray/_multiarray_tests.c.src | 14 ++++++++++++++ numpy/core/tests/test_deprecations.py | 8 ++++++++ 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/doc/release/upcoming_changes/16938.c_api.rst b/doc/release/upcoming_changes/16938.c_api.rst index 202d7df9598f..6e9c495dfdf0 100644 --- a/doc/release/upcoming_changes/16938.c_api.rst +++ b/doc/release/upcoming_changes/16938.c_api.rst @@ -2,11 +2,12 @@ Size of ``np.ndarray`` and ``np.void_`` changed ----------------------------------------------- The size of the ``PyArrayObject`` and ``PyVoidScalarObject`` structures have changed. The following header definition has been -removed:: +deprecated:: #define NPY_SIZEOF_PYARRAYOBJECT (sizeof(PyArrayObject_fields)) -since the size must not be considered a compile time constant. +since the size must not be considered a compile time constant: it will +change for different runtime versions of NumPy. The most likely relevant use are potential subclasses written in C which will have to be recompiled and should be updated. Please see the diff --git a/numpy/core/include/numpy/ndarraytypes.h b/numpy/core/include/numpy/ndarraytypes.h index cb2a13435077..a0a945234396 100644 --- a/numpy/core/include/numpy/ndarraytypes.h +++ b/numpy/core/include/numpy/ndarraytypes.h @@ -721,6 +721,12 @@ typedef struct tagPyArrayObject { } PyArrayObject; #endif +/* 2020-Nov-25 1.20 */ +#define NPY_SIZEOF_PYARRAYOBJECT \ + (DEPRECATE("NPY_SIZEOF_PYARRAYOBJECT is deprecated since it cannot be used at compile time " \ + "and is not constant across different runtime versions of NumPy\n")<0? -1: PyArray_Type.tp_basicsize) + + /* Array Flags Object */ typedef struct PyArrayFlagsObject { diff --git a/numpy/core/src/multiarray/_multiarray_tests.c.src b/numpy/core/src/multiarray/_multiarray_tests.c.src index 7780a2de4c31..099bdf04943d 100644 --- a/numpy/core/src/multiarray/_multiarray_tests.c.src +++ b/numpy/core/src/multiarray/_multiarray_tests.c.src @@ -2167,6 +2167,17 @@ run_intp_converter(PyObject* NPY_UNUSED(self), PyObject *args) return tup; } +static PyObject * +test_macro(PyObject* NPY_UNUSED(self), PyObject *args) +{ + int ret = NPY_SIZEOF_PYARRAYOBJECT; + if (ret < 0) { + return NULL; + } + return PyLong_FromLong(ret); +} + + static PyMethodDef Multiarray_TestsMethods[] = { {"IsPythonScalar", IsPythonScalar, @@ -2348,6 +2359,9 @@ static PyMethodDef Multiarray_TestsMethods[] = { {"run_intp_converter", run_intp_converter, METH_VARARGS, NULL}, + {"test_macro", + test_macro, + METH_VARARGS, NULL}, {NULL, NULL, 0, NULL} /* Sentinel */ }; diff --git a/numpy/core/tests/test_deprecations.py b/numpy/core/tests/test_deprecations.py index a67fe62c3d9b..2b778f76b56c 100644 --- a/numpy/core/tests/test_deprecations.py +++ b/numpy/core/tests/test_deprecations.py @@ -785,3 +785,11 @@ class TestDeprecatedUnpickleObjectScalar(_DeprecationTestCase): def test_deprecated(self): ctor = np.core.multiarray.scalar self.assert_deprecated(lambda: ctor(np.dtype("O"), 1)) + + +class TestMacros(_DeprecationTestCase): + # 2020-11-25 + def test_macro(self): + from numpy.core._multiarray_tests import test_macro + assert_(test_macro() > 32) + self.assert_deprecated(test_macro) From 89037ab25ce9bab64068b863e7961203e278c945 Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Wed, 25 Nov 2020 10:46:25 -0600 Subject: [PATCH 07/11] Tune down matti's deprecation to write the error instead. --- numpy/core/include/numpy/ndarraytypes.h | 3 ++- numpy/core/tests/test_deprecations.py | 25 ++++++++++++++++++++++--- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/numpy/core/include/numpy/ndarraytypes.h b/numpy/core/include/numpy/ndarraytypes.h index a0a945234396..106ad700aaf1 100644 --- a/numpy/core/include/numpy/ndarraytypes.h +++ b/numpy/core/include/numpy/ndarraytypes.h @@ -724,7 +724,8 @@ typedef struct tagPyArrayObject { /* 2020-Nov-25 1.20 */ #define NPY_SIZEOF_PYARRAYOBJECT \ (DEPRECATE("NPY_SIZEOF_PYARRAYOBJECT is deprecated since it cannot be used at compile time " \ - "and is not constant across different runtime versions of NumPy\n")<0? -1: PyArray_Type.tp_basicsize) + "and is not constant across different runtime versions of NumPy\n") < 0 ? \ + PyErr_WriteUnraisable(NULL) : NULL, PyArray_Type.tp_basicsize) diff --git a/numpy/core/tests/test_deprecations.py b/numpy/core/tests/test_deprecations.py index 2b778f76b56c..84c8455a6554 100644 --- a/numpy/core/tests/test_deprecations.py +++ b/numpy/core/tests/test_deprecations.py @@ -789,7 +789,26 @@ def test_deprecated(self): class TestMacros(_DeprecationTestCase): # 2020-11-25 - def test_macro(self): + def test_macro_warning(self): from numpy.core._multiarray_tests import test_macro - assert_(test_macro() > 32) - self.assert_deprecated(test_macro) + + with pytest.warns(DeprecationWarning): + res = test_macro() + + assert res > 32 + + def test_macro_error_print(self, capsys): + from numpy.core._multiarray_tests import test_macro + + with warnings.catch_warnings(): + warnings.simplefilter("error", DeprecationWarning) + res = test_macro() + + assert res > 32 + + msg = ("NPY_SIZEOF_PYARRAYOBJECT is deprecated since it cannot be " + "used at compile time and is not constant across different " + "runtime versions of NumPy\n") + captured = capsys.readouterr() + # There may also be a traceback in there, so lets match pedantic: + assert captured.err.count(msg) == 1 From 8cb3efb76c4df5e34389533d2126825f37f0f13e Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Wed, 25 Nov 2020 12:08:17 -0600 Subject: [PATCH 08/11] Tweak macro, so that clang hopefully doesn't complain. --- numpy/core/include/numpy/ndarraytypes.h | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/numpy/core/include/numpy/ndarraytypes.h b/numpy/core/include/numpy/ndarraytypes.h index 106ad700aaf1..106b637da6ab 100644 --- a/numpy/core/include/numpy/ndarraytypes.h +++ b/numpy/core/include/numpy/ndarraytypes.h @@ -721,11 +721,18 @@ typedef struct tagPyArrayObject { } PyArrayObject; #endif -/* 2020-Nov-25 1.20 */ +/* 2020-Nov-25, NumPy 1.20 */ +/* + * The following macro makes some best efforts to give a deprecation warning + * and return the correct runtime size of the structure. The macro should + * be replaced with `PyArray_Type.tp_basicsize` or similar, the size + * may change in the future. + */ #define NPY_SIZEOF_PYARRAYOBJECT \ (DEPRECATE("NPY_SIZEOF_PYARRAYOBJECT is deprecated since it cannot be used at compile time " \ "and is not constant across different runtime versions of NumPy\n") < 0 ? \ - PyErr_WriteUnraisable(NULL) : NULL, PyArray_Type.tp_basicsize) + (PyErr_WriteUnraisable(NULL), PyArray_Type.tp_basicsize) : \ + PyArray_Type.tp_basicsize) From 803354eaf9e19b1aa68c3bca07966bf2b918eadc Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Wed, 25 Nov 2020 12:27:49 -0600 Subject: [PATCH 09/11] Use None instead of NULL in PyErr_WriteUnraisable, pypy seems to have a bug with it --- numpy/core/include/numpy/ndarraytypes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/numpy/core/include/numpy/ndarraytypes.h b/numpy/core/include/numpy/ndarraytypes.h index 106b637da6ab..c1b26b63a9e1 100644 --- a/numpy/core/include/numpy/ndarraytypes.h +++ b/numpy/core/include/numpy/ndarraytypes.h @@ -731,7 +731,7 @@ typedef struct tagPyArrayObject { #define NPY_SIZEOF_PYARRAYOBJECT \ (DEPRECATE("NPY_SIZEOF_PYARRAYOBJECT is deprecated since it cannot be used at compile time " \ "and is not constant across different runtime versions of NumPy\n") < 0 ? \ - (PyErr_WriteUnraisable(NULL), PyArray_Type.tp_basicsize) : \ + (PyErr_WriteUnraisable(Py_None), PyArray_Type.tp_basicsize) : \ PyArray_Type.tp_basicsize) From 802977b11bdc2ffa8e2b922f82996227114ab429 Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Wed, 25 Nov 2020 18:47:01 -0600 Subject: [PATCH 10/11] Just comment it out... --- doc/release/upcoming_changes/16938.c_api.rst | 2 +- numpy/core/include/numpy/ndarraytypes.h | 20 +++++++-------- numpy/core/tests/test_deprecations.py | 27 -------------------- 3 files changed, 10 insertions(+), 39 deletions(-) diff --git a/doc/release/upcoming_changes/16938.c_api.rst b/doc/release/upcoming_changes/16938.c_api.rst index 6e9c495dfdf0..aff72c8e5375 100644 --- a/doc/release/upcoming_changes/16938.c_api.rst +++ b/doc/release/upcoming_changes/16938.c_api.rst @@ -2,7 +2,7 @@ Size of ``np.ndarray`` and ``np.void_`` changed ----------------------------------------------- The size of the ``PyArrayObject`` and ``PyVoidScalarObject`` structures have changed. The following header definition has been -deprecated:: +removed:: #define NPY_SIZEOF_PYARRAYOBJECT (sizeof(PyArrayObject_fields)) diff --git a/numpy/core/include/numpy/ndarraytypes.h b/numpy/core/include/numpy/ndarraytypes.h index c1b26b63a9e1..473983b5022e 100644 --- a/numpy/core/include/numpy/ndarraytypes.h +++ b/numpy/core/include/numpy/ndarraytypes.h @@ -721,19 +721,17 @@ typedef struct tagPyArrayObject { } PyArrayObject; #endif -/* 2020-Nov-25, NumPy 1.20 */ /* - * The following macro makes some best efforts to give a deprecation warning - * and return the correct runtime size of the structure. The macro should - * be replaced with `PyArray_Type.tp_basicsize` or similar, the size - * may change in the future. + * Removed 2020-Nov-25, NumPy 1.20 + * #define NPY_SIZEOF_PYARRAYOBJECT (sizeof(PyArrayObject_fields)) + * + * The above macro was removed as it gave a false sense of a stable ABI + * with respect to the structures size. If you require a runtime constant, + * you can use `PyArray_Type.tp_basicsize` instead. Otherwise, please + * see the PyArrayObject documentation or ask the NumPy developers for + * information on how to correctly replace the macro in a way that is + * compatible with multiple NumPy versions. */ -#define NPY_SIZEOF_PYARRAYOBJECT \ - (DEPRECATE("NPY_SIZEOF_PYARRAYOBJECT is deprecated since it cannot be used at compile time " \ - "and is not constant across different runtime versions of NumPy\n") < 0 ? \ - (PyErr_WriteUnraisable(Py_None), PyArray_Type.tp_basicsize) : \ - PyArray_Type.tp_basicsize) - /* Array Flags Object */ diff --git a/numpy/core/tests/test_deprecations.py b/numpy/core/tests/test_deprecations.py index 84c8455a6554..a67fe62c3d9b 100644 --- a/numpy/core/tests/test_deprecations.py +++ b/numpy/core/tests/test_deprecations.py @@ -785,30 +785,3 @@ class TestDeprecatedUnpickleObjectScalar(_DeprecationTestCase): def test_deprecated(self): ctor = np.core.multiarray.scalar self.assert_deprecated(lambda: ctor(np.dtype("O"), 1)) - - -class TestMacros(_DeprecationTestCase): - # 2020-11-25 - def test_macro_warning(self): - from numpy.core._multiarray_tests import test_macro - - with pytest.warns(DeprecationWarning): - res = test_macro() - - assert res > 32 - - def test_macro_error_print(self, capsys): - from numpy.core._multiarray_tests import test_macro - - with warnings.catch_warnings(): - warnings.simplefilter("error", DeprecationWarning) - res = test_macro() - - assert res > 32 - - msg = ("NPY_SIZEOF_PYARRAYOBJECT is deprecated since it cannot be " - "used at compile time and is not constant across different " - "runtime versions of NumPy\n") - captured = capsys.readouterr() - # There may also be a traceback in there, so lets match pedantic: - assert captured.err.count(msg) == 1 From a216c5b3508a2c9c62a4e37c133350832ce26f86 Mon Sep 17 00:00:00 2001 From: Sebastian Berg Date: Thu, 26 Nov 2020 00:06:52 -0600 Subject: [PATCH 11/11] Apply suggestions from code review Co-authored-by: Matti Picus --- numpy/core/src/multiarray/_multiarray_tests.c.src | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/numpy/core/src/multiarray/_multiarray_tests.c.src b/numpy/core/src/multiarray/_multiarray_tests.c.src index 099bdf04943d..7780a2de4c31 100644 --- a/numpy/core/src/multiarray/_multiarray_tests.c.src +++ b/numpy/core/src/multiarray/_multiarray_tests.c.src @@ -2167,17 +2167,6 @@ run_intp_converter(PyObject* NPY_UNUSED(self), PyObject *args) return tup; } -static PyObject * -test_macro(PyObject* NPY_UNUSED(self), PyObject *args) -{ - int ret = NPY_SIZEOF_PYARRAYOBJECT; - if (ret < 0) { - return NULL; - } - return PyLong_FromLong(ret); -} - - static PyMethodDef Multiarray_TestsMethods[] = { {"IsPythonScalar", IsPythonScalar, @@ -2359,9 +2348,6 @@ static PyMethodDef Multiarray_TestsMethods[] = { {"run_intp_converter", run_intp_converter, METH_VARARGS, NULL}, - {"test_macro", - test_macro, - METH_VARARGS, NULL}, {NULL, NULL, 0, NULL} /* Sentinel */ };