[BUG]: Avoiding promoter overrides leading to global state corruption by SwayamInSync · Pull Request #78 · numpy/numpy-quaddtype

SwayamInSync · 2026-03-27T23:59:35Z

closes #76

seberg

Thanks, LGTM, but the ld promoter doesn't really look right. I.e. it doesn't do anything interesting?

To be fair, that seems like an existing bug...

seberg · 2026-03-28T12:10:38Z

src/include/umath/promoters.hpp

+
+    // Preserve the integer type for the exponent (slot 1)
+    Py_INCREF(op_dtypes[1]);
+    new_op_dtypes[1] = op_dtypes[1];


you want to promote to PyArray_PyLongDType here. (assuming signature[1] == NULL).
(I guess you could add more logic in case of non-ints, but maybe doesn't matter much in practice... cast safety may even reject it normally anyway.)

Although long seems weird compared to just using intp that always works, but OK. Otherwise this looks like it doesn't do anything really.

Right, I updated the loop registration + promoter to use intp

ngoldbaum

Looks good! I asked Claude Code to review your code and it spotted some pre-existing reference leaks.

Maybe you could reduce boilerplate and the chance for bugs by using the following helper function:

diff --git a/src/include/umath/promoters.hpp b/src/include/umath/promoters.hpp
index 3b3c1ef..2a3d26c 100644
--- a/src/include/umath/promoters.hpp
+++ b/src/include/umath/promoters.hpp
@@ -12,9 +12,10 @@
 #include "../dtype.h"

 inline int
-quad_ufunc_promoter(PyUFuncObject *ufunc, PyArray_DTypeMeta *op_dtypes[],
-                    PyArray_DTypeMeta *signature[], PyArray_DTypeMeta *new_op_dtypes[])
+quad_ufunc_promoter(PyObject *ob_ufunc, PyArray_DTypeMeta *const op_dtypes[],
+                    PyArray_DTypeMeta *const signature[], PyArray_DTypeMeta *new_op_dtypes[])
 {
+    PyUFuncObject *ufunc = (PyUFuncObject *)ufunc;
     int nin = ufunc->nin;
     int nargs = ufunc->nargs;
     PyArray_DTypeMeta *common = NULL;
@@ -56,7 +57,8 @@ quad_ufunc_promoter(PyUFuncObject *ufunc, PyArray_DTypeMeta *op_dtypes[],
     }
     // If no common output dtype, use standard promotion for inputs
     if (common == NULL) {
-        common = PyArray_PromoteDTypeSequence(nin, op_dtypes);
+        common = PyArray_PromoteDTypeSequence(
+                nin, (PyArray_DTypeMeta **)op_dtypes);
         if (common == NULL) {
             if (PyErr_ExceptionMatches(PyExc_TypeError)) {
                 PyErr_Clear();  // Do not propagate normal promotion errors
@@ -86,5 +88,42 @@ quad_ufunc_promoter(PyUFuncObject *ufunc, PyArray_DTypeMeta *op_dtypes[],
     return 0;
 }

+static inline int
+add_promoter(PyObject *ufunc, PyObject *dtypes[], size_t n_dtypes,
+             PyArrayMethod_PromoterFunction *promoter_impl)
+{
+    PyObject *DType_tuple = NULL;
+    PyObject *promoter_capsule = NULL;
+    int ret = -1;
+
+    DType_tuple = PyTuple_New(n_dtypes);
+
+    if (DType_tuple == NULL) {
+        goto cleanup;
+    }
+
+    for (size_t i=0; i<n_dtypes; i++) {
+        Py_INCREF((PyObject *)dtypes[i]);
+        PyTuple_SET_ITEM(DType_tuple, i, (PyObject *)dtypes[i]);
+    }
+
+    promoter_capsule = PyCapsule_New(
+            (void *)&promoter_impl, "numpy._ufunc_promoter", NULL);
+
+    if (promoter_capsule == NULL) {
+        goto cleanup;
+    }
+
+    if (PyUFunc_AddPromoter(ufunc, DType_tuple, promoter_capsule) < 0) {
+        goto cleanup;
+    }
+
+    ret = 0;
+  cleanup:
+    Py_XDECREF(promoter_capsule);
+    Py_XDECREF(DType_tuple);
+
+    return ret;
+}

-#endif
\ No newline at end of file
+#endif

And then you call it like so:

        PyObject *left_DTypes[] = {
            (PyObject *)&QuadPrecDType,
            (PyObject *)&PyArrayDescr_Type,
            (PyObject *)&QuadPrecDType,
        };

        if (add_promoter(ufunc, left_DTypes, 4, quad_ufunc_promoter) != 0) {
            Py_DECREF(ufunc);
            return -1;
        }

ngoldbaum · 2026-03-31T15:44:33Z

src/include/umath/promoters.hpp



+inline int
+quad_ldexp_promoter(PyUFuncObject *ufunc, PyArray_DTypeMeta *op_dtypes[],


The type of the first argument should be PyObject *. There's also some const missing from the second and third parameters:

https://numpy.org/devdocs/reference/c-api/array.html#c.PyArrayMethod_PromoterFunction

The same error happens in quad_ufunc_promoter.

ngoldbaum · 2026-03-31T15:46:19Z

src/csrc/umath/binary_ops.cpp

 int
 create_quad_ldexp_ufunc(PyObject *numpy, const char *ufunc_name)
 {
    PyObject *ufunc = PyObject_GetAttrString(numpy, ufunc_name);


It's a pre-existing issue but since you're touching this code: ufunc object is never DECREF'd below, including the error paths.

ngoldbaum · 2026-03-31T15:50:28Z

src/csrc/umath/binary_ops.cpp

 int
 create_quad_binary_2out_ufunc(PyObject *numpy, const char *ufunc_name)
 {
    PyObject *ufunc = PyObject_GetAttrString(numpy, ufunc_name);


similarly here, ufunc is never cleaned up below. Since you're touching the error paths you might as well fix this too.

ngoldbaum · 2026-03-31T15:51:58Z

src/csrc/umath/binary_ops.cpp

 int
 create_quad_binary_ufunc(PyObject *numpy, const char *ufunc_name)
 {
    PyObject *ufunc = PyObject_GetAttrString(numpy, ufunc_name);


Same here, ufunc is never DECREF'd.

SwayamInSync · 2026-03-31T16:54:26Z

Thanks @ngoldbaum will apply the current scope fixes and I like this add_promoter helper func, will patch this in a different PR

- Fix quad_ufunc_promoter, quad_ldexp_promoter, and comparison_ufunc_promoter signatures to match PyArrayMethod_PromoterFunction typedef: (PyObject *, PyArray_DTypeMeta *const[], PyArray_DTypeMeta *const[], PyArray_DTypeMeta *[]) - Add missing Py_DECREF(ufunc) in all create_quad_*_ufunc error/success return paths (binary_ops, comparison_ops, unary_ops, unary_props) - Add missing Py_INCREF before Py_XSETREF in comparison_ufunc_promoter

SwayamInSync

Fixed the de-ref of ufunc object and promoter signature.
Noticed some more simplifcations can be done as discussed below, will push them in a new PR to avoid unnecessary bloat

SwayamInSync · 2026-04-01T14:43:14Z

src/include/umath/promoters.hpp

 inline int
-quad_ufunc_promoter(PyUFuncObject *ufunc, PyArray_DTypeMeta *op_dtypes[],
-                    PyArray_DTypeMeta *signature[], PyArray_DTypeMeta *new_op_dtypes[])
+quad_ufunc_promoter(PyObject *ufunc_obj, PyArray_DTypeMeta *const op_dtypes[],


Since we change the promoter registration to ensure one type remains quaddtype, we can very much simplify this current code ( no need to call PyArray_PromoteDTypeSequence, which requires casting away the const-ness) so if that sounds uncomfortable I made a new PR which removes this (have to add here to ensure the proper build)

SwayamInSync · 2026-04-01T14:43:40Z

src/csrc/umath/comparison_ops.cpp

 NPY_NO_EXPORT int
-comparison_ufunc_promoter(PyUFuncObject *ufunc, PyArray_DTypeMeta *op_dtypes[],
-                          PyArray_DTypeMeta *signature[], PyArray_DTypeMeta *new_op_dtypes[])
+comparison_ufunc_promoter(PyObject *ufunc_obj, PyArray_DTypeMeta *const op_dtypes[],


Same simplification thing here as well + noticed a bug of dtype registration not matching with registered loop

ngoldbaum · 2026-04-01T16:12:02Z

src/csrc/umath/comparison_ops.cpp

    if (res < 0) {
        return -1;
    }
+    Py_INCREF(&PyArray_BoolDType);


good catch!

ngoldbaum · 2026-04-01T16:33:48Z

LGTM, thanks @SwayamInSync!

As a followup, maybe consider adding a NumPy test that uses quaddtype directly. I think it would be really neat if we could write NumPy tests using quaddtype. It's often annoying that we don't have a realistic new-style user dtype available for testing. It would also be nice to know when we break user dtype builds.

SwayamInSync · 2026-04-01T17:02:52Z

As a followup, maybe consider adding a NumPy test that uses quaddtype directly. I think it would be really neat if we could write NumPy tests using quaddtype. It's often annoying that we don't have a realistic new-style user dtype available for testing. It would also be nice to know when we break user dtype builds.

inside NumPy repo?

ngoldbaum · 2026-04-01T17:36:45Z

yeah, inside the numpy repo as a test in NumPy itself, assuming that's possible right now

SwayamInSync · 2026-04-01T17:49:19Z

yeah, inside the numpy repo as a test in NumPy itself, assuming that's possible right now

Got it, yeah I think its fesible.
To push these changes (including the other PR) should we backport these changes into a 1.1 version and release that?

SwayamInSync added 2 commits March 27, 2026 03:18

avoiding general promotion register in matmul

1ee4998

adding tests

3412657

SwayamInSync mentioned this pull request Mar 28, 2026

Promoters are matching too broadly #76

Closed

SwayamInSync requested a review from ngoldbaum March 28, 2026 00:02

seberg reviewed Mar 28, 2026

View reviewed changes

SwayamInSync added 2 commits March 28, 2026 14:35

promote + register loop with intp

34e2117

fix comment

107bec5

ngoldbaum reviewed Mar 31, 2026

View reviewed changes

SwayamInSync commented Apr 1, 2026

View reviewed changes

SwayamInSync mentioned this pull request Apr 1, 2026

[BUG, REFAC]: Simplifies quad_ufunc_promoter + fixes reduction bug in comparison_ufunc_promoter #79

Open

ngoldbaum reviewed Apr 1, 2026

View reviewed changes

src/csrc/umath/comparison_ops.cpp

if (res < 0) {

return -1;

}

Py_INCREF(&PyArray_BoolDType);

Copy link
Copy Markdown

Member

ngoldbaum Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch!

ngoldbaum merged commit bab707d into numpy:main Apr 1, 2026
12 checks passed



		inline int
		quad_ldexp_promoter(PyUFuncObject ufunc, PyArray_DTypeMeta op_dtypes[],

Uh oh!

Conversation

SwayamInSync commented Mar 27, 2026

Uh oh!

seberg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ngoldbaum left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SwayamInSync commented Mar 31, 2026

Uh oh!

SwayamInSync left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SwayamInSync Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ngoldbaum commented Apr 1, 2026

Uh oh!

Uh oh!

SwayamInSync commented Apr 1, 2026

Uh oh!

ngoldbaum commented Apr 1, 2026

Uh oh!

SwayamInSync commented Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SwayamInSync left a comment •

edited

Loading

SwayamInSync Apr 1, 2026 •

edited

Loading