8000 MNT: replace _PyDict_GetItemStringWithError with PyDict_GetItemString… · numpy/numpy@50a705c · GitHub
[go: up one dir, main page]
Skip to content

Commit 50a705c

Browse files
ngoldbaumngoldbaum
authored
MNT: replace _PyDict_GetItemStringWithError with PyDict_GetItemStringRef (#26282)
Replaces all usages of the private _PyDict_GetItemStringWithError Python C API function with PyDict_GetItemStringRef. The latter is available in the python 3.13 C API and via python-capicompat in older python versions. The new function has the same semantics as PyDict_GetItemRef but takes a UTF-8 C string instead of a python object. Like _PyDict_GetItemStringWithError, it does not suppress errors. It also has the nice improvement that the return type is now int and it signals success or failure. This lets me re-structure control flow a bit and avoid PyErr_Occurred() calls. I used the new PyDict_ContainsString function if all we care about is whether a key is in the dict. I also used Py_SETREF in a few places that were using the unsafe decref then set pattern. See #26159 for the relevant tracking issue. --- * MNT: replace _PyDict_GetItemStringWithError with PyDict_GetItemStringRef * MNT: fix issue spotted in code review * MNT: fix two more issues spotted in code review * MNT: more missing decrefs spotted in code review * MNT: delete unnecessary decrefs (covered by goto fail logic)
1 parent 91255fe commit 50a705c

File tree

6 files changed

+82
-62
lines changed

6 files changed

+82
-62
lines changed

‎numpy/_core/src/common/ufunc_override.c

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#define _MULTIARRAYMODULE
33

44
#include "numpy/ndarraytypes.h"
5+
#include "npy_pycompat.h"
56
#include "get_attr_string.h"
67
#include "npy_import.h"
78
#include "ufunc_override.h"
@@ -99,12 +100,11 @@ PyUFuncOverride_GetOutObjects(PyObject *kwds, PyObject **out_kwd_obj, PyObject *
99100
*out_kwd_obj = NULL;
100101
return -1;
101102
}
102-
/* borrowed reference */
103-
*out_kwd_obj = _PyDict_GetItemStringWithError(kwds, "out");
104-
if (*out_kwd_obj == NULL) {
105-
if (PyErr_Occurred()) {
106-
return -1;
107-
}
103+
int result = PyDict_GetItemStringRef(kwds, "out", out_kwd_obj);
104+
if (result == -1) {
105+
return -1;
106+
}
107+
else if (result == 0) {
108108
Py_INCREF(Py_None);
109109
*out_kwd_obj = Py_None;
110110
return 0;
@@ -118,15 +118,14 @@ PyUFuncOverride_GetOutObjects(PyObject *kwds, PyObject **out_kwd_obj, PyObject *
118118
seq = PySequence_Fast(*out_kwd_obj,
119119
"Could not convert object to sequence");
120120
if (seq == NULL) {
121-
*out_kwd_obj = NULL;
121+
Py_CLEAR(*out_kwd_obj);
122122
return -1;
123123
}
124124
*out_objs = PySequence_Fast_ITEMS(seq);
125-
*out_kwd_obj = seq;
125+
Py_SETREF(*out_kwd_obj, seq);
126126
return PySequence_Fast_GET_SIZE(seq);
127127
}
128128
else {
129-
Py_INCREF(*out_kwd_obj);
130129
*out_objs = out_kwd_obj;
131130
return 1;
132131
}

‎numpy/_core/src/multiarray/ctors.c

Lines changed: 39 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
#include "numpy/npy_math.h"
1313

1414
#include "npy_config.h"
15-
15+
#include "npy_pycompat.h"
1616
#include "npy_ctypes.h"
1717

1818
#include "multiarraymodule.h"
@@ -2180,10 +2180,10 @@ PyArray_FromInterface(PyObject *origin)
21802180
}
21812181

21822182
/* Get type string from interface specification */
2183-
attr = _PyDict_GetItemStringWithError(iface, "typestr");
2184-
if (attr == NULL) {
2183+
int result = PyDict_GetItemStringRef(iface, "typestr", &attr);
2184+
if (result <= 0) {
21852185
Py_DECREF(iface);
2186-
if (!PyErr_Occurred()) {
2186+
if (result == 0) {
21872187
PyErr_SetString(PyExc_ValueError,
21882188
"Missing __array_interface__ typestr");
21892189
}
@@ -2207,43 +2207,47 @@ PyArray_FromInterface(PyObject *origin)
22072207
* the 'descr' attribute.
22082208
*/
22092209
if (dtype->type_num == NPY_VOID) {
2210-
PyObject *descr = _PyDict_GetItemStringWithError(iface, "descr");
2211-
if (descr == NULL && PyErr_Occurred()) {
2210+
PyObject *descr = NULL;
2211+
result = PyDict_GetItemStringRef(iface, "descr", &descr);
2212+
if (result == -1) {
22122213
goto fail;
22132214
}
22142215
PyArray_Descr *new_dtype = NULL;
2215-
if (descr != NULL) {
2216+
if (result == 1) {
22162217
int is_default = _is_default_descr(descr, attr);
22172218
if (is_default < 0) {
2219+
Py_DECREF(descr);
22182220
goto fail;
22192221
}
22202222
if (!is_default) {
22212223
if (PyArray_DescrConverter2(descr, &new_dtype) != NPY_SUCCEED) {
2224+
Py_DECREF(descr);
22222225
goto fail;
22232226
}
22242227
if (new_dtype != NULL) {
2225-
Py_DECREF(dtype);
2226-
dtype = new_dtype;
2228+
Py_SETREF(dtype, new_dtype);
22272229
}
22282230
}
2229-
22302231
}
2231-
2232+
Py_DECREF(descr);
22322233
}
2234+
Py_CLEAR(attr);
22332235

22342236
/* Get shape tuple from interface specification */
2235-
attr = _PyDict_GetItemStringWithError(iface, "shape");
2236-
if (attr == NULL) {
2237-
if (PyErr_Occurred()) {
2238-
return NULL;
2239-
}
2237+
result = PyDict_GetItemStringRef(iface, "shape", &attr);
2238+
if (result < 0) {
2239+
return NULL;
2240+
}
2241+
if (result == 0) {
22402242
/* Shape must be specified when 'data' is specified */
2241-
PyObject *data = _PyDict_GetItemStringWithError(iface, "data");
2242-
if (data == NULL && PyErr_Occurred()) {
2243+
int result = PyDict_ContainsString(iface, "data");
2244+
if (result < 0) {
2245+
Py_DECREF(attr);
22432246
return NULL;
22442247
}
2245-
else if (data != NULL) {
2248+
else if (result == 1) {
22462249
Py_DECREF(iface);
2250+
Py_DECREF(attr);
22472251
PyErr_SetString(PyExc_ValueError,
22482252
"Missing __array_interface__ shape");
22492253
return NULL;
@@ -2271,10 +2275,11 @@ PyArray_FromInterface(PyObject *origin)
22712275
}
22722276
}
22732277
}
2278+
Py_CLEAR(attr);
22742279

22752280
/* Get data buffer from interface specification */
2276-
attr = _PyDict_GetItemStringWithError(iface, "data");
2277-
if (attr == NULL && PyErr_Occurred()){
2281+
result = PyDict_GetItemStringRef(iface, "data", &attr);
2282+
if (result == -1){
22782283
return NULL;
22792284
}
22802285

@@ -2337,20 +2342,24 @@ PyArray_FromInterface(PyObject *origin)
23372342
PyBuffer_Release(&view);
23382343

23392344
/* Get offset number from interface specification */
2340-
attr = _PyDict_GetItemStringWithError(iface, "offset");
2341-
if (attr == NULL && PyErr_Occurred()) {
2345+
PyObject *offset = NULL;
2346+
result = PyDict_GetItemStringRef(iface, "offset", &offset);
2347+
if (result == -1) {
23422348
goto fail;
23432349
}
2344-
else if (attr) {
2345-
npy_longlong num = PyLong_AsLongLong(attr);
2350+
else if (result == 1) {
2351+
npy_longlong num = PyLong_AsLongLong(offset);
23462352
if (error_converting(num)) {
23472353
PyErr_SetString(PyExc_TypeError,
23482354
"__array_interface__ offset must be an integer");
2355+
Py_DECREF(offset);
23492356
goto fail;
23502357
}
23512358
data += num;
2359+
Py_DECREF(offset);
23522360
}
23532361
}
2362+
Py_CLEAR(attr);
23542363

23552364
ret = (PyArrayObject *)PyArray_NewFromDescrAndBase(
23562365
&PyArray_Type, dtype,
@@ -2376,11 +2385,11 @@ PyArray_FromInterface(PyObject *origin)
23762385
goto fail;
23772386
}
23782387
}
2379-
attr = _PyDict_GetItemStringWithError(iface, "strides");
2380-
if (attr == NULL && PyErr_Occurred()){
2388+
result = PyDict_GetItemStringRef(iface, "strides", &attr);
2389+
if (result == -1){
23812390
return NULL;
23822391
}
2383-
if (attr != NULL && attr != Py_None) {
2392+
if (result == 1 && attr != Py_None) {
23842393
if (!PyTuple_Check(attr)) {
23852394
PyErr_SetString(PyExc_TypeError,
23862395
"strides must be a tuple");
@@ -2404,12 +2413,14 @@ PyArray_FromInterface(PyObject *origin)
24042413
if (n) {
24052414
memcpy(PyArray_STRIDES(ret), strides, n*sizeof(npy_intp));
24062415
}
2416+
Py_DECREF(attr);
24072417
}
24082418
PyArray_UpdateFlags(ret, NPY_ARRAY_UPDATE_ALL);
24092419
Py_DECREF(iface);
24102420
return (PyObject *)ret;
24112421

24122422
fail:
2423+
Py_XDECREF(attr);
24132424
Py_XDECREF(dtype);
24142425
Py_XDECREF(iface);
24152426
return NULL;

‎numpy/_core/src/multiarray/number.c

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
#include "numpy/arrayobject.h"
99

1010
#include "npy_config.h"
11-
11+
#include "npy_pycompat.h"
1212
#include "npy_import.h"
1313
#include "common.h"
1414
#include "number.h"
@@ -60,24 +60,25 @@ array_inplace_matrix_multiply(PyArrayObject *m1, PyObject *m2);
6060
* Those not present will not be changed
6161
*/
6262

63-
/* FIXME - macro contains a return */
64-
#define SET(op) temp = _PyDict_GetItemStringWithError(dict, #op); \
65-
if (temp == NULL && PyErr_Occurred()) { \
63+
/* FIXME - macro contains returns */
64+
#define SET(op) \
65+
res = PyDict_GetItemStringRef(dict, #op, &temp); \
66+
if (res == -1) { \
6667
return -1; \
6768
} \
68-
else if (temp != NULL) { \
69+
else if (res == 1) { \
6970
if (!(PyCallable_Check(temp))) { \
71+
Py_DECREF(temp); \
7072
return -1; \
7173
} \
72-
Py_INCREF(temp); \
73-
Py_XDECREF(n_ops.op); \
74-
n_ops.op = temp; \
74+
Py_XSETREF(n_ops.op, temp); \
7575
}
7676

7777
NPY_NO_EXPORT int
7878
_PyArray_SetNumericOps(PyObject *dict)
7979
{
8080
PyObject *temp = NULL;
81+
int res;
8182
SET(add);
8283
SET(subtract);
8384
SET(multiply);

‎numpy/_core/src/umath/override.c

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
#include "numpy/ndarraytypes.h"
55
#include "numpy/ufuncobject.h"
66
#include "npy_import.h"
7-
7+
#include "npy_pycompat.h"
88
#include "override.h"
99
#include "ufunc_override.h"
1010

@@ -148,18 +148,17 @@ static int
148148
normalize_signature_keyword(PyObject *normal_kwds)
149149
{
150150
/* If the keywords include `sig` rename to `signature`. */
151-
PyObject* obj = _PyDict_GetItemStringWithError(normal_kwds, "sig");
152-
if (obj == NULL && PyErr_Occurred()) {
151+
PyObject* obj = NULL;
152+
int result = PyDict_GetItemStringRef(normal_kwds, "sig", &obj);
153+
if (result == -1) {
153154
return -1;
154155
}
155-
if (obj != NULL) {
156-
/*
157-
* No INCREF or DECREF needed: got a borrowed reference above,
158-
* and, unlike e.g. PyList_SetItem, PyDict_SetItem INCREF's it.
159-
*/
156+
if (result == 1) {
160157
if (PyDict_SetItemString(normal_kwds, "signature", obj) < 0) {
158+
Py_DECREF(obj);
161159
return -1;
162160
}
161+
Py_DECREF(obj);
163162
if (PyDict_DelItemString(normal_kwds, "sig") < 0) {
164163
return -1;
165164
}

‎numpy/_core/src/umath/ufunc_object.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
#include <stddef.h>
3434

3535
#include "npy_config.h"
36-
36+
#include "npy_pycompat.h"
3737
#include "npy_argparse.h"
3838

3939
#include "numpy/arrayobject.h"
@@ -142,8 +142,10 @@ PyUFunc_clearfperr()
142142
NPY_NO_EXPORT int
143143
set_matmul_flags(PyObject *d)
144144
{
145-
PyObject *matmul = _PyDict_GetItemStringWithError(d, "matmul");
146-
if (matmul == NULL) {
145+
PyObject *matmul = NULL;
146+
int result = PyDict_GetItemStringRef(d, "matmul", &matmul);
147+
if (result <= 0) {
148+
// caller sets an error if one isn't already set
147149
return -1;
148150
}
149151
/*
@@ -162,6 +164,7 @@ set_matmul_flags(PyObject *d)
162164
NPY_ITER_UPDATEIFCOPY |
163165
NPY_UFUNC_DEFAULT_OUTPUT_FLAGS) &
164166
~NPY_ITER_OVERLAP_ASSUME_ELEMENTWISE;
167+
Py_DECREF(matmul);
165168
return 0;
166169
}
167170

‎numpy/_core/src/umath/umathmodule.c

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "numpy/arrayobject.h"
2222
#include "numpy/ufuncobject.h"
2323
#include "numpy/npy_3kcompat.h"
24+
#include "npy_pycompat.h"
2425
#include "abstract.h"
2526

2627
#include "numpy/npy_math.h"
@@ -303,29 +304,35 @@ int initumath(PyObject *m)
303304
* TODO: This should probably be done at a better place, or even in the
304305
* code generator directly.
305306
*/
306-
s = _PyDict_GetItemStringWithError(d, "logical_and");
307-
if (s == NULL) {
307+
int res = PyDict_GetItemStringRef(d, "logical_and", &s);
308+
if (res <= 0) {
308309
return -1;
309310
}
310311
if (install_logical_ufunc_promoter(s) < 0) {
312+
Py_DECREF(s);
311313
return -1;
312314
}
315+
Py_DECREF(s);
313316

314-
s = _PyDict_GetItemStringWithError(d, "logical_or");
315-
if (s == NULL) {
317+
res = PyDict_GetItemStringRef(d, "logical_or", &s);
318+
if (res <= 0) {
316319
return -1;
317320
}
318321
if (install_logical_ufunc_promoter(s) < 0) {
322+
Py_DECREF(s);
319323
return -1;
320324
}
325+
Py_DECREF(s);
321326

322-
s = _PyDict_GetItemStringWithError(d, "logical_xor");
323-
if (s == NULL) {
327+
res = PyDict_GetItemStringRef(d, "logical_xor", &s);
328+
if (res <= 0) {
324329
return -1;
325330
}
326331
if (install_logical_ufunc_promoter(s) < 0) {
332+
Py_DECREF(s);
327333
return -1;
328334
}
335+
Py_DECREF(s);
329336

330337
if (init_string_ufuncs(d) < 0) {
331338
return -1;

0 commit comments

Comments
 (0)
0