Repository: cython/cython
Pull request: Allow fused function pointer return type
Fixes https://github.com/cython/cython/issues/4644

For fused functions it makes sense that the return type is ignored
(a function can't be specialized based on return type alone) but
for function pointers the return type should be included (since
such a pointer might be an argument to a fused function)

Problem statement:
[BUG] Fused function with function pointer parameter that has a struct argument causes a compiler crash
**describe the bug**
I'm developing some bindings where there are a lot of C functions that basically have the signature (error_code)(*getter)(some_c_struct*, primitive_value* value_ptr)
That take a pointer, retrieve a value and either store that value in the provided pointer or return an error.
I have a fused function which can convert a C value into any of about fifty different Python enums depending on its type, and having to write the same code over (E.G:
    cdef primitive_value x
    handle_errors(get_x(&self.c_struct, &x))
    return convert_to_py(x)
 about a hundred times wouldn't be too enjoyable, so I tried writing a fused helper function that takes a Python value which is a Cython extension class wrapping a C struct and a function pointer to a getter function. It then gets a C struct pointer from the Python-wrapped struct using a fused function, calls the getter with that pointer, converts errors to exceptions and returns the value.
However it crashes the compiler.
**to reproduce**
```cython
# types we support converting to Python. Some ints, some C enum types and maybe cython.p_char to represent strings
cdef fused python_convertable:
	int
	size_t

# some structs that the C functions we're wrapping want access to, in order to get or set properties
cdef struct pair_of_ints:
	int x, y

cdef struct coords:
	float x, y, z


# some struct types we have Python classes for and want to pass temporary pointers to C functions
# the Python classes are not shown here for simplicity
cdef fused c_struct:
	pair_of_ints
	coords


# a helper function that takes a Python object that must be a Cython class containing a c_struct and a C function pointer of type python_value(c_struct*)
# it gets a pointer to the contained struct or raises an exception if the Python object is of the wrong type. It then calls the passed in function pointer with the c_struct pointer and converts the return value to a Python object using a fused conversion function that isn't defined here for simplicity
cdef call_getter(object owner, python_convertable(*f)(c_struct*) ):
	cdef c_struct* c_struct_ptr = NULL
# pretend we got the c_struct pointer from owner and put it in c_struct_ptr
	cdef python_convertable result = f(c_struct_ptr)
	return result # we don't have any complex types that need converting or we would handle it in a separate fused function

```
**Expected behavior**
It compiles
**environment**
OS: Windows 10 (64-bit)
Python: 3.9.5
Cython: 0.29.26

**additional context**
I think I've encountered compiler crashes in the past with fused types, but I don't remember it.

Hints from issue discussion:
Oh, here is how to reproduce and the error:
cythonize -i -3 fused_function_pointer.pyx
...
```
Error compiling Cython file:
------------------------------------------------------------
...
	coords


# a helper function that takes a Python object that must be a Cython class containing a c_struct and a C function pointer of type python_value(c_struct*)
# it gets a pointer to the contained struct or raises an exception if the Python object is of the wrong type. It then calls the passed in function pointer with the c_struct pointer and converts the return value to a Python object using a fused conversion function that isn't defined here for simplicity
cdef call_getter(object owner, python_convertable(*f)(c_struct*) ):
    ^
------------------------------------------------------------

fused_function_pointer.pyx:23:5: Compiler crash in AnalyseDeclarationsTransform

ModuleNode.body = StatListNode(fused_function_pointer.pyx:2:0)
StatListNode.stats[4] = CFuncDefNode(fused_function_pointer.pyx:23:5,
    args = [...]/2,
    has_fused_arguments = True,
    modifiers = [...]/0,
    visibility = 'private')
File 'FusedNode.py', line 61, in __init__: FusedCFuncDefNode(fused_function_pointer.pyx:23:5,
    fused_compound_types = [...]/1,
    match = "dest_sig[{{dest_sig_idx}}] = '{{specialized_type_name}}'",
    no_match = 'dest_sig[{{dest_sig_idx}}] = None',
    nodes = [...]/0)
File 'FusedNode.py', line 144, in copy_cdef: FusedCFuncDefNode(fused_function_pointer.pyx:23:5,
    fused_compound_types = [...]/1,
    match = "dest_sig[{{dest_sig_idx}}] = '{{specialized_type_name}}'",
    no_match = 'dest_sig[{{dest_sig_idx}}] = None',
    nodes = [...]/0)

Compiler crash traceback from this point on:
  File "C:\python\lib\site-packages\Cython\Compiler\FusedNode.py", line 144, in copy_cdef
    type = copied_node.type.specialize(fused_to_specific)
  File "C:\python\lib\site-packages\Cython\Compiler\PyrexTypes.py", line 2941, in specialize
    [arg.specialize(values) for arg in self.args],
  File "C:\python\lib\site-packages\Cython\Compiler\PyrexTypes.py", line 2941, in <listcomp>
    [arg.specialize(values) for arg in self.args],
  File "C:\python\lib\site-packages\Cython\Compiler\PyrexTypes.py", line 3254, in specialize
    return CFuncTypeArg(self.name, self.type.specialize(values), self.pos, self.cname)
  File "C:\python\lib\site-packages\Cython\Compiler\PyrexTypes.py", line 2547, in specialize
    base_type = self.base_type.specialize(values)
  File "C:\python\lib\site-packages\Cython\Compiler\PyrexTypes.py", line 2940, in specialize
    result = CFuncType(self.return_type.specialize(values),
  File "C:\python\lib\site-packages\Cython\Compiler\PyrexTypes.py", line 1655, in specialize
    return values[self]
KeyError: FusedType(name='python_convertable')
Traceback (most recent call last):
  File "C:\python\Scripts\cythonize-script.py", line 33, in <module>
    sys.exit(load_entry_point('Cython==0.29.26', 'console_scripts', 'cythonize')())
  File "C:\python\lib\site-packages\Cython\Build\Cythonize.py", line 223, in main
    cython_compile(path, options)
  File "C:\python\lib\site-packages\Cython\Build\Cythonize.py", line 97, in cython_compile
    ext_modules = cythonize(
  File "C:\python\lib\site-packages\Cython\Build\Dependencies.py", line 1103, in cythonize
    cythonize_one(*args)
  File "C:\python\lib\site-packages\Cython\Build\Dependencies.py", line 1226, in cythonize_one
    raise CompileError(None, pyx_file)
Cython.Compiler.Errors.CompileError: C:\py\fused_function_pointer.pyx
```
Please, edit your messages so if there's any code you make it look like that:
\`\`\`
code goes here
\`\`\`

It's hard to understand your messages in the current state with the wrong formatting
Ahh, sorry about that. I edited it. Is it fine now?
@Keithcat1 That's better thanks. Could you edit the comment https://github.com/cython/cython/issues/4644#issuecomment-1054691130 too?
Okay. I can't mark it as Cython or Python code because it contains both, but it looks better now. Not sure why I was doing it that way. I was going to fix this in my past issues, but I see that someone already did it. Thanks.
Thanks - that's a lot easier to work with now. Marking the type of code isn't so important - it mainly makes it a lot easier to copy and test the example code.

Commit message: Add a multi-level function pointer test

Patch to review:
diff --git a/Cython/Compiler/PyrexTypes.py b/Cython/Compiler/PyrexTypes.py
index 32ee0ad7e84..05ca43e87b6 100644
--- a/Cython/Compiler/PyrexTypes.py
+++ b/Cython/Compiler/PyrexTypes.py
@@ -78,7 +78,7 @@ def __deepcopy__(self, memo):
         """
         return self
 
-    def get_fused_types(self, result=None, seen=None, subtypes=None):
+    def get_fused_types(self, result=None, seen=None, subtypes=None, include_function_return_type=False):
         subtypes = subtypes or self.subtypes
         if not subtypes:
             return None
@@ -91,10 +91,10 @@ def get_fused_types(self, result=None, seen=None, subtypes=None):
             list_or_subtype = getattr(self, attr)
             if list_or_subtype:
                 if isinstance(list_or_subtype, BaseType):
-                    list_or_subtype.get_fused_types(result, seen)
+                    list_or_subtype.get_fused_types(result, seen, include_function_return_type=include_function_return_type)
                 else:
                     for subtype in list_or_subtype:
-                        subtype.get_fused_types(result, seen)
+                        subtype.get_fused_types(result, seen, include_function_return_type=include_function_return_type)
 
         return result
 
@@ -1845,7 +1845,7 @@ def specialize(self, values):
         else:
             raise CannotSpecialize()
 
-    def get_fused_types(self, result=None, seen=None):
+    def get_fused_types(self, result=None, seen=None, include_function_return_type=False):
         if result is None:
             return [self]
 
@@ -2757,6 +2757,12 @@ def find_cpp_operation_type(self, operator, operand_type=None):
             return self.base_type.find_cpp_operation_type(operator, operand_type)
         return None
 
+    def get_fused_types(self, result=None, seen=None, include_function_return_type=False):
+        # The include_function_return_type argument means that the return type in function pointers
+        # gets included (unlike for fused functions themselves, where the return type cannot
+        # be an independent fused type)
+        return super(CPointerBaseType, self).get_fused_types(result, seen, include_function_return_type=True)
+
 
 class CNullPtrType(CPtrType):
 
@@ -3232,10 +3238,13 @@ def get_all_specialized_function_types(self):
 
         return result
 
-    def get_fused_types(self, result=None, seen=None, subtypes=None):
+    def get_fused_types(self, result=None, seen=None, subtypes=None, include_function_return_type=False):
         """Return fused types in the order they appear as parameter types"""
-        return super(CFuncType, self).get_fused_types(result, seen,
-                                                      subtypes=['args'])
+        return super(CFuncType, self).get_fused_types(
+            result, seen,
+            # for function pointer types, we consider the result type; for plain function
+            # types we don't (because it must be derivable from the arguments)
+            subtypes=self.subtypes if include_function_return_type else ['args'])
 
     def specialize_entry(self, entry, cname):
         assert not self.is_fused
@@ -3865,7 +3874,7 @@ def create_to_py_utility_code(self, env):
     def is_template_type(self):
         return self.templates is not None and self.template_type is None
 
-    def get_fused_types(self, result=None, seen=None):
+    def get_fused_types(self, result=None, seen=None, include_function_return_type=False):
         if result is None:
             result = []
             seen = set()
diff --git a/tests/run/fused_types.pyx b/tests/run/fused_types.pyx
index 0c7f9adcd2a..9a8ab58038a 100644
--- a/tests/run/fused_types.pyx
+++ b/tests/run/fused_types.pyx
@@ -510,3 +510,48 @@ def convert_to_ptr(cython.floating x):
         return handle_float(&x)
     elif cython.floating is double:
         return handle_double(&x)
+
+cdef double get_double():
+    return 1.0
+cdef float get_float():
+    return 0.0
+
+cdef call_func_pointer(cython.floating (*f)()):
+    return f()
+
+def test_fused_func_pointer():
+    """
+    >>> test_fused_func_pointer()
+    1.0
+    0.0
+    """
+    print(call_func_pointer(get_double))
+    print(call_func_pointer(get_float))
+
+cdef double get_double_from_int(int i):
+    return i
+
+cdef call_func_pointer_with_1(cython.floating (*f)(cython.integral)):
+    return f(1)
+
+def test_fused_func_pointer2():
+    """
+    >>> test_fused_func_pointer2()
+    1.0
+    """
+    print(call_func_pointer_with_1(get_double_from_int))
+
+cdef call_function_that_calls_fused_pointer(object (*f)(cython.floating (*)(cython.integral))):
+    if cython.floating is double and cython.integral is int:
+        return 5*f(get_double_from_int)
+    else:
+        return None  # practically it's hard to make this kind of function useful...
+
+def test_fused_func_pointer_multilevel():
+    """
+    >>> test_fused_func_pointer_multilevel()
+    5.0
+    None
+    """
+    print(call_function_that_calls_fused_pointer(call_func_pointer_with_1[double, int]))
+    print(call_function_that_calls_fused_pointer(call_func_pointer_with_1[float, int]))


Code review comments:
diff_hunk: @@ -2757,10 +2757,11 @@ def find_cpp_operation_type(self, operator, operand_type=None):
             return self.base_type.find_cpp_operation_type(operator, operand_type)
         return None
 
-    def get_fused_types(self, result=None, seen=None, is_pointer=False):
-        # The is_pointer argument means that the return type in function pointers
-        # gets included (unlike for fused functions)
-        return super(CPointerBaseType, self).get_fused_types(result, seen, is_pointer=True)
+    def get_fused_types(self, result=None, seen=None, include_function_return_type=False):
+        # The include_function_return_type argument means that the return type in function pointers
+        # gets included (unlike for fused functions themselves, where the return type cannot
+        # be an independent fused type)
path: Cython/Compiler/PyrexTypes.py
text: @user1:
```suggestion
        # For function pointers, include the return type - unlike for fused functions themselves,
        # where the return type cannot be an independent fused type (i.e. is derived or non-fused).
```