Repository: cython/cython
Pull request: Handle typedefs in ufuncs
~Extern typedefs~ Mapping of type to Numpy constant gets left to the C compiler to sort out since we can't rely on Cython having been told correctly.

Also fixes issues with using types written as `cimported_module.type` - these weren't visible in the utility code scope. Types are now declared in the utility code scope only using a temporary name instead.

Closes #6161

Problem statement:
[BUG] numpy types cannot be used for ufunc arguments
### Describe the bug

Using numpy's C types for ufunc declarations raises a very long error.

```
Compiling /home/mnoethe/test/cython/test_numpy_ufunc.pyx because it changed.
[1/1] Cythonizing /home/mnoethe/test/cython/test_numpy_ufunc.pyx

Error compiling Cython file:
------------------------------------------------------------
...
from stdlib.stdint import int64_t

np.import_array()

@cython.ufunc
cdef np.int64_t add_one(np.int64_t x):
     ^
------------------------------------------------------------

test_numpy_ufunc.pyx:9:5: Type 'int64_t' cannot be used as a ufunc argument


...
```

### Code to reproduce the behaviour:

```cython
#cython: language_level=3
cimport cython
cimport numpy as np

np.import_array()

@cython.ufunc
cdef np.int64_t add_one(np.int64_t x):
    return x + 1

```


### Expected behaviour

Numpy ufuncs work with numpy data types.

### OS

Linux, Ubuntu 22.04

### Python version

3.11

### Cython version

cython                    3.0.10          py311hb755f60_0    conda-forge

### Additional context

_No response_

Hints from issue discussion:
Thanks for the report. It's good to know what people are trying this and finding the obvious deficiencies :-)

It's slightly harder than it looks. Cython doesn't actually know a lot about integer typedefs - mostly just that it's "some integer" and whether it's signed or not. However, it needs to tell Numpy exactly what type is actually being used. So it either needs some special-casing for these Numpy typedefs, or it needs a clever runtime mechanism to work out what to tell Numpy.

Commit message: Handle typedefs in ufuncs

Extern typedefs get left to the C compiler to sort out since
we can't rely on Cython having been told correctly.

Patch to review:
diff --git a/Cython/Compiler/UFuncs.py b/Cython/Compiler/UFuncs.py
index b7525679031..d446f8acf64 100644
--- a/Cython/Compiler/UFuncs.py
+++ b/Cython/Compiler/UFuncs.py
@@ -2,17 +2,13 @@
     Nodes,
     ExprNodes,
     FusedNode,
-    TreeFragment,
-    Pipeline,
-    ParseTreeTransforms,
     Naming,
-    UtilNodes,
 )
 from .Errors import error
 from . import PyrexTypes
 from .UtilityCode import CythonUtilityCode
 from .Code import TempitaUtilityCode, UtilityCode
-from .Visitor import PrintTree, TreeVisitor, VisitorTransform
+from .Visitor import TreeVisitor
 
 numpy_int_types = [
     "NPY_BYTE",
@@ -41,6 +37,21 @@
 
 
 def _get_type_constant(pos, type_):
+    while type_.is_typedef and not type_.typedef_is_external:
+        type_ = type_.typedef_base_type
+    if type_.is_typedef:
+        assert type_.typedef_is_external
+        if type_.is_complex:
+            return f"__PYX_GET_NPY_COMPLEX_TYPE({type_.empty_declaration_code()})"
+        elif type_.is_int:
+            signed = ""
+            if type_.signed == PyrexTypes.SIGNED:
+                signed = "S"
+            elif type_.signed == PyrexTypes.UNSIGNED:
+                signed = "U"
+            return f"__PYX_GET_NPY_{signed}INT_TYPE({type_.empty_declaration_code()})"
+        elif type_.is_float:
+            return f"__PYX_GET_NPY_FLOAT_TYPE({type_.empty_declaration_code()})"
     if type_.is_complex:
         # 'is' checks don't seem to work for complex types
         if type_ == PyrexTypes.c_float_complex_type:
@@ -49,6 +60,8 @@ def _get_type_constant(pos, type_):
             return "NPY_CDOUBLE"
         elif type_ == PyrexTypes.c_longdouble_complex_type:
             return "NPY_CLONGDOUBLE"
+        else:
+            return f"__PYX_NPY_GET_COMPLEX_TYPE({type_.empty_declaration_code()})"
     elif type_.is_numeric:
         postfix = type_.empty_declaration_code().upper().replace(" ", "")
         typename = "NPY_%s" % postfix
@@ -166,6 +179,9 @@ def use_generic_utility_code(self):
         self.global_scope.use_utility_code(
             UtilityCode.load_cached("UFuncsInit", "UFuncs_C.c")
         )
+        self.global_scope.use_utility_code(
+            UtilityCode.load_cached("UFuncTypeHandling", "UFuncs_C.c")
+        )
         self.global_scope.use_utility_code(
             UtilityCode.load_cached("NumpyImportUFunc", "NumpyImportArray.c")
         )
@@ -237,6 +253,34 @@ def generate_ufunc_initialization(converters, cfunc_nodes, original_node):
     pos = original_node.pos
     func_name = original_node.entry.name
     docstr = original_node.doc
+
+    validation_call_node = ExprNodes.PythonCapiCallNode(
+        pos,
+        function_name="__Pyx_validate_ufunc_types",
+        func_type=PyrexTypes.CFuncType(
+            PyrexTypes.c_int_type,
+            [PyrexTypes.CFuncTypeArg("types", PyrexTypes.c_char_ptr_type, None),
+             PyrexTypes.CFuncTypeArg("arg_count", PyrexTypes.c_py_ssize_t_type, None),
+             PyrexTypes.CFuncTypeArg("input_arg_count", PyrexTypes.c_py_ssize_t_type, None)],
+            exception_value="-1"
+        ),
+        args = [
+            ExprNodes.ConstNode(
+                pos, type=PyrexTypes.c_char_ptr_type, value=f"{ufunc_types_name}()"
+            ),
+            ExprNodes.ConstNode(
+                pos, type=PyrexTypes.c_py_ssize_t_type, value=str(narg_in+narg_out)
+            ),
+            ExprNodes.ConstNode(
+                pos, type=PyrexTypes.c_py_ssize_t_type, value=str(narg_in)
+            ),
+        ]
+    )
+    validation_call_node = Nodes.ExprStatNode(
+        pos,
+        expr=validation_call_node
+    )
+
     args_to_func = '%s(), %s, %s(), %s, %s, %s, PyUFunc_None, "%s", %s, 0' % (
         ufunc_funcs_name,
         ufunc_data_name,
@@ -270,7 +314,7 @@ def generate_ufunc_initialization(converters, cfunc_nodes, original_node):
         ),
         rhs=call_node,
     )
-    return assgn_node
+    return [validation_call_node, assgn_node]
 
 
 def _generate_stats_from_converters(converters, node):
@@ -282,5 +326,5 @@ def _generate_stats_from_converters(converters, node):
         converter.global_scope.utility_code_list.extend(tree.scope.utility_code_list)
         stats.append(ufunc_node)
 
-    stats.append(generate_ufunc_initialization(converters, stats, node))
+    stats.extend(generate_ufunc_initialization(converters, stats, node))
     return stats
diff --git a/Cython/Utility/UFuncs_C.c b/Cython/Utility/UFuncs_C.c
index 2115e4b2b52..b577883c2bc 100644
--- a/Cython/Utility/UFuncs_C.c
+++ b/Cython/Utility/UFuncs_C.c
@@ -14,6 +14,58 @@
 #define __PYX_PYUFUNCGENERICFUNCTION_CAST(x) (PyUFuncGenericFunction)x
 #endif
 
+/////////////////////// UFuncTypeHandling.proto //////////////
+
+#define __PYX_GET_NPY_COMPLEX_TYPE(tp) \
+    sizeof(tp) == sizeof(npy_cfloat) ? NPY_CFLOAT : \
+    sizeof(tp) == sizeof(npy_cdouble) ? NPY_CDOUBLE : \
+    sizeof(tp) == sizeof(npy_clongdouble) ? NPY_CLONGDOUBLE : NPY_NOTYPE
+
+#define __PYX_GET_NPY_FLOAT_TYPE(tp) \
+    sizeof(tp) == sizeof(npy_float) ? NPY_FLOAT : \
+    sizeof(tp) == sizeof(npy_double) ? NPY_DOUBLE : \
+    sizeof(tp) == sizeof(npy_longdouble) ? NPY_LONGDOUBLE : NPY_NOTYPE
+
+#define __PYX_GET_NPY_UINT_TYPE(tp) \
+    sizeof(tp) == 1 ? NPY_UINT8 : \
+    sizeof(tp) == 2 ? NPY_UINT16 : \
+    sizeof(tp) == 4 ? NPY_UINT32 : \
+    sizeof(tp) == 8 ? NPY_UINT64 : NPY_NOTYPE
+
+#define __PYX_GET_NPY_SINT_TYPE(tp) \
+    sizeof(tp) == 1 ? NPY_INT8 : \
+    sizeof(tp) == 2 ? NPY_INT16 : \
+    sizeof(tp) == 4 ? NPY_INT32 : \
+    sizeof(tp) == 8 ? NPY_INT64 : NPY_NOTYPE
+
+#define __PYX_GET_NPY_INT_TYPE(tp) \
+    (((tp)-1) < (tp)0) ? \
+        (__PYX_GET_NPY_SINT_TYPE(tp)) : \
+        (__PYX_GET_NPY_UINT_TYPE(tp))
+
+static int __Pyx_validate_ufunc_types(char *types, Py_ssize_t count, Py_ssize_t input_count);
+
+/////////////////////// UFuncTypeHandling ///////////////
+
+// DW - it's a bit of a shame that this has to be a runtime check since the C compiler does
+// know it. We could make this a compile-time error in C++, but not easily in C.
+static int __Pyx_validate_ufunc_types(char *types, Py_ssize_t count, Py_ssize_t input_count) {
+    Py_ssize_t i;
+    for (i=0; i<count; ++i) {
+        if (types[i] == NPY_NOTYPE) {
+            PyErr_Format(
+                PyExc_TypeError,
+                "Invalid type for %s argument %d to ufunc. "
+                "This is from an external typedef that Cython could not resolve.",
+                (i < input_count ? "input" : "output"),
+                (i < input_count ? i : i - input_count)
+            );
+            return -1;
+        }
+    }
+    return 0;
+}
+
 /////////////////////// UFuncConsts.proto ////////////////////
 
 // getter functions because we can't forward-declare arrays
diff --git a/tests/run/ufunc.pyx b/tests/run/ufunc.pyx
index 71acdce5715..270b0bb9cd6 100644
--- a/tests/run/ufunc.pyx
+++ b/tests/run/ufunc.pyx
@@ -16,6 +16,26 @@ large_int_arr_2d = np.arange(1500*600, dtype=int).reshape((1500, -1))
 large_double_arr_1d = large_int_arr_1d.astype(np.double)
 large_double_arr_2d = large_int_arr_2d.astype(np.double)
 
+ctypedef double mydouble
+ctypedef int myint
+ctypedef double complex mycdouble
+
+cdef extern from *:
+    """
+    typedef float someone_elses_float;
+    typedef short someone_elses_int;
+    typedef unsigned char someone_elses_unsigned_int;
+    typedef signed long long someone_elses_signed_int;
+    typedef signed int extern_actually_signed;
+    typedef unsigned int extern_actually_unsigned;
+    """
+    ctypedef double someone_elses_float
+    ctypedef int someone_elses_int
+    ctypedef unsigned int someone_elses_unsigned_int
+    ctypedef signed int someone_elses_signed_int
+    ctypedef int extern_actually_signed
+    ctypedef int extern_actually_unsigned
+
 # it's fairly hard to test that nogil results in the GIL actually
 # being released unfortunately
 @cython.ufunc
@@ -197,3 +217,39 @@ def test_can_throw():
     ...
     RuntimeError
     """
+
+@cython.ufunc
+cdef mydouble known_typedefs(myint x, mycdouble y):
+    return x*y.real*y.imag
+
+def test_known_typedefs():
+    """
+    >>> known_typedefs(np.int32(3), 4+5j)
+    60.0
+    """
+
+@cython.ufunc
+cdef someone_elses_float unknown_typedefs(someone_elses_int x, someone_elses_signed_int y, someone_elses_unsigned_int z):
+    return x+y+z
+
+def test_unknown_typedefs():
+    """
+    >>> unknown_typedefs(np.short(1), np.longlong(3), np.uint8(2))
+    6.0
+    """
+
+ctypedef fused two_int_types:
+    extern_actually_signed
+    extern_actually_unsigned
+
+@cython.ufunc
+cdef object disambiguate_signedness(two_int_types x):
+    return cython.typeof(x)
+
+def test_disambiguate_asignedness():
+    """
+    >>> disambiguate_signedness(np.int32(-5))
+    'extern_actually_signed'
+    >>> disambiguate_signedness(np.uint32(10))
+    'extern_actually_unsigned'
+    """
diff --git a/tests/run/ufunc_import_failure.srctree b/tests/run/ufunc_import_failure.srctree
new file mode 100644
index 00000000000..d8a79dcee91
--- /dev/null
+++ b/tests/run/ufunc_import_failure.srctree
@@ -0,0 +1,52 @@
+PYTHON setup.py build_ext --inplace
+PYTHON test_bad_ufunc_type.py
+
+######## setup.py ###########
+from setuptools import Extension, setup
+from Cython.Build import cythonize
+
+try:
+    import numpy as np
+except ImportError:
+    # We can't use tags in srctree tests, so just make it "work" without numpy
+    exit()
+
+
+setup(ext_modules=cythonize([
+    Extension(
+        "bad_ufunc_type", ["bad_ufunc_type.pyx"],
+        include_dirs=[np.get_include()]
+    )]))
+
+######## bad_ufunc_type.pyx ###########
+
+cdef extern from *:
+    """
+    typedef char someone_elses_double;
+    """
+    # Note that this is wrong... char is not the same size as a usable
+    # floating point type.
+    ctypedef double someone_elses_double
+    
+cimport cython
+
+@cython.ufunc
+cdef double use_our_float_type(someone_elses_double x):
+    return x
+
+
+######## test_bad_ufunc_type.py ###########
+
+try:
+    import numpy
+except ImportError:
+    print("Skipping 'ufunc_import_failure' due to missing numpy")
+    exit()
+
+try:
+    import bad_ufunc_type
+except TypeError:
+    pass  # good
+else:
+    print("TypeError not caught")
+    exit(1)


Code review comments:
diff_hunk: @@ -14,6 +14,58 @@
 #define __PYX_PYUFUNCGENERICFUNCTION_CAST(x) (PyUFuncGenericFunction)x
 #endif
 
+/////////////////////// UFuncTypeHandling.proto //////////////
+
+#define __PYX_GET_NPY_COMPLEX_TYPE(tp) \
+    sizeof(tp) == sizeof(npy_cfloat) ? NPY_CFLOAT : \
+    sizeof(tp) == sizeof(npy_cdouble) ? NPY_CDOUBLE : \
+    sizeof(tp) == sizeof(npy_clongdouble) ? NPY_CLONGDOUBLE : NPY_NOTYPE
+
+#define __PYX_GET_NPY_FLOAT_TYPE(tp) \
+    sizeof(tp) == sizeof(npy_float) ? NPY_FLOAT : \
+    sizeof(tp) == sizeof(npy_double) ? NPY_DOUBLE : \
+    sizeof(tp) == sizeof(npy_longdouble) ? NPY_LONGDOUBLE : NPY_NOTYPE
+
+#define __PYX_GET_NPY_UINT_TYPE(tp) \
+    sizeof(tp) == 1 ? NPY_UINT8 : \
+    sizeof(tp) == 2 ? NPY_UINT16 : \
+    sizeof(tp) == 4 ? NPY_UINT32 : \
+    sizeof(tp) == 8 ? NPY_UINT64 : NPY_NOTYPE
+
+#define __PYX_GET_NPY_SINT_TYPE(tp) \
+    sizeof(tp) == 1 ? NPY_INT8 : \
+    sizeof(tp) == 2 ? NPY_INT16 : \
+    sizeof(tp) == 4 ? NPY_INT32 : \
+    sizeof(tp) == 8 ? NPY_INT64 : NPY_NOTYPE
+
+#define __PYX_GET_NPY_INT_TYPE(tp) \
+    (((tp)-1) < (tp)0) ? \
+        (__PYX_GET_NPY_SINT_TYPE(tp)) : \
+        (__PYX_GET_NPY_UINT_TYPE(tp))
+
+static int __Pyx_validate_ufunc_types(char *types, Py_ssize_t count, Py_ssize_t input_count);
+
+/////////////////////// UFuncTypeHandling ///////////////
+
+// DW - it's a bit of a shame that this has to be a runtime check since the C compiler does
+// know it. We could make this a compile-time error in C++, but not easily in C.
path: Cython/Utility/UFuncs_C.c
text: @user1:
It could potentially figure it out if it inlines the validation function. But how many types do you expect here? One, two, three? Probably rarely more, right? Why not call the validation function multiple times, instead of using a loop? You could also inline the `NPY_NOTYPE` comparison and keep the helper function only for setting the exception. That should make it easy for the C compiler to discard the calls in the normal cases.

@author:
Ah I think you're slightly misunderstanding the comment.

What I really want is a failure from the C compiler because it already knows that it's used `NPY_NOTYPE`. I'm not worried about the cost of the validation function. I wish I didn't have to try to import the module to work out that I've got it wrong.

@user1:
Something like this?
https://github.com/cython/cython/blob/bc590426ced4c7d5ba0c8476e18204812e0f354a/Cython/Utility/ModuleSetupCode.c#L404-L407

@author:
Yeah possibly. I'll have another look and see if I can do something like that.

diff_hunk: @@ -14,6 +14,58 @@
 #define __PYX_PYUFUNCGENERICFUNCTION_CAST(x) (PyUFuncGenericFunction)x
 #endif
 
+/////////////////////// UFuncTypeHandling.proto //////////////
+
+#define __PYX_GET_NPY_COMPLEX_TYPE(tp) \
+    sizeof(tp) == sizeof(npy_cfloat) ? NPY_CFLOAT : \
+    sizeof(tp) == sizeof(npy_cdouble) ? NPY_CDOUBLE : \
+    sizeof(tp) == sizeof(npy_clongdouble) ? NPY_CLONGDOUBLE : NPY_NOTYPE
+
+#define __PYX_GET_NPY_FLOAT_TYPE(tp) \
+    sizeof(tp) == sizeof(npy_float) ? NPY_FLOAT : \
+    sizeof(tp) == sizeof(npy_double) ? NPY_DOUBLE : \
+    sizeof(tp) == sizeof(npy_longdouble) ? NPY_LONGDOUBLE : NPY_NOTYPE
+
+#define __PYX_GET_NPY_UINT_TYPE(tp) \
+    sizeof(tp) == 1 ? NPY_UINT8 : \
+    sizeof(tp) == 2 ? NPY_UINT16 : \
+    sizeof(tp) == 4 ? NPY_UINT32 : \
+    sizeof(tp) == 8 ? NPY_UINT64 : NPY_NOTYPE
+
+#define __PYX_GET_NPY_SINT_TYPE(tp) \
+    sizeof(tp) == 1 ? NPY_INT8 : \
+    sizeof(tp) == 2 ? NPY_INT16 : \
+    sizeof(tp) == 4 ? NPY_INT32 : \
+    sizeof(tp) == 8 ? NPY_INT64 : NPY_NOTYPE
+
+#define __PYX_GET_NPY_INT_TYPE(tp) \
+    (((tp)-1) < (tp)0) ? \
+        (__PYX_GET_NPY_SINT_TYPE(tp)) : \
+        (__PYX_GET_NPY_UINT_TYPE(tp))
path: Cython/Utility/UFuncs_C.c
text: @user1:
This tends to trigger "unsigned value cannot be < 0" warnings. I keep forgetting what works best, but this might work better:
```suggestion
#define __PYX_GET_NPY_INT_TYPE(tp) ( \
    (((tp)-1) > (tp)0) ? \
        (__PYX_GET_NPY_UINT_TYPE(tp)) : \
        (__PYX_GET_NPY_SINT_TYPE(tp)) )
```
Also, it's always best to put macro expressions into parentheses. You never know how they'll end up being used, now or later.

diff_hunk: @@ -237,6 +253,34 @@ def generate_ufunc_initialization(converters, cfunc_nodes, original_node):
     pos = original_node.pos
     func_name = original_node.entry.name
     docstr = original_node.doc
+
+    validation_call_node = ExprNodes.PythonCapiCallNode(
+        pos,
+        function_name="__Pyx_validate_ufunc_types",
+        func_type=PyrexTypes.CFuncType(
+            PyrexTypes.c_int_type,
+            [PyrexTypes.CFuncTypeArg("types", PyrexTypes.c_char_ptr_type, None),
+             PyrexTypes.CFuncTypeArg("arg_count", PyrexTypes.c_py_ssize_t_type, None),
+             PyrexTypes.CFuncTypeArg("input_arg_count", PyrexTypes.c_py_ssize_t_type, None)],
+            exception_value="-1"
+        ),
path: Cython/Compiler/UFuncs.py
text: @user1:
Did you try extracting the function type creation into a constant?