Allow soft-complex->double coercion to run without gil (#5287)

It'll only need the GIL on failure (which it can get)
and "power of" type maths is the soft of thing that people are
likely already doing in nogil blocks

4 files changed, 83 insertions, 11 deletions

diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py
index 9f456f739..249a40f46 100644
--- a/Cython/Compiler/ExprNodes.py
+++ b/Cython/Compiler/ExprNodes.py
@@ -6312,8 +6312,6 @@ class SimpleCallNode(CallNode):
         if self.is_temp and self.type.is_reference:
             self.type = PyrexTypes.CFakeReferenceType(self.type.ref_base_type)
 
-        # Called in 'nogil' context?
-        self.nogil = env.nogil
         # C++ exception handler
         if func_type.exception_check == '+':
             if needs_cpp_exception_conversion(func_type):
@@ -6431,6 +6429,7 @@ class SimpleCallNode(CallNode):
                     code.error_goto_if_null(self.result(), self.pos)))
             self.generate_gotref(code)
         elif func_type.is_cfunction:
+            nogil = not code.funcstate.gil_owned
             if self.has_optional_args:
                 actual_nargs = len(self.args)
                 expected_nargs = len(func_type.args) - func_type.optional_arg_count
@@ -6458,7 +6457,7 @@ class SimpleCallNode(CallNode):
                 if exc_val is not None:
                     exc_checks.append("%s == %s" % (self.result(), func_type.return_type.cast_code(exc_val)))
                 if exc_check:
-                    if self.nogil:
+                    if nogil:
                         code.globalstate.use_utility_code(
                             UtilityCode.load_cached("ErrOccurredWithGIL", "Exceptions.c"))
                         exc_checks.append("__Pyx_ErrOccurredWithGIL()")
@@ -6478,7 +6477,7 @@ class SimpleCallNode(CallNode):
                 if func_type.exception_check == '+':
                     translate_cpp_exception(code, self.pos, '%s%s;' % (lhs, rhs),
                                             self.result() if self.type.is_pyobject else None,
-                                            func_type.exception_value, self.nogil)
+                                            func_type.exception_value, nogil)
                 else:
                     if exc_checks:
                         goto_error = code.error_goto_if(" && ".join(exc_checks), self.pos)
@@ -14138,18 +14137,21 @@ class CoerceToComplexNode(CoercionNode):
 
 
 def coerce_from_soft_complex(arg, dst_type, env):
+    from .UtilNodes import HasGilNode
     cfunc_type = PyrexTypes.CFuncType(
         PyrexTypes.c_double_type,
-        [ PyrexTypes.CFuncTypeArg("value", PyrexTypes.soft_complex_type, None) ],
-        exception_value = "-1",
-        exception_check = True
+        [ PyrexTypes.CFuncTypeArg("value", PyrexTypes.soft_complex_type, None),
+          PyrexTypes.CFuncTypeArg("have_gil", PyrexTypes.c_bint_type, None) ],
+        exception_value="-1",
+        exception_check=True,
+        nogil=True  # We can acquire the GIL internally on failure
     )
     call = PythonCapiCallNode(
         arg.pos,
         "__Pyx_SoftComplexToDouble",
         cfunc_type,
         utility_code = UtilityCode.load_cached("SoftComplexToDouble", "Complex.c"),
-        args = [arg]
+        args = [arg, HasGilNode(arg.pos)],
     )
     call = call.analyse_types(env)
     if call.type != dst_type:
diff --git a/Cython/Compiler/UtilNodes.py b/Cython/Compiler/UtilNodes.py
index 3c4b9d633..81d3038ea 100644
--- a/Cython/Compiler/UtilNodes.py
+++ b/Cython/Compiler/UtilNodes.py
@@ -10,7 +10,7 @@ from . import Nodes
 from . import ExprNodes
 from .Nodes import Node
 from .ExprNodes import AtomicExprNode
-from .PyrexTypes import c_ptr_type
+from .PyrexTypes import c_ptr_type, c_bint_type
 
 
 class TempHandle(object):
@@ -369,3 +369,20 @@ class TempResultFromStatNode(ExprNodes.ExprNode):
 
     def generate_function_definitions(self, env, code):
         self.body.generate_function_definitions(env, code)
+
+
+class HasGilNode(AtomicExprNode):
+    """
+    Simple node that evaluates to 0 or 1 depending on whether we're
+    in a nogil context
+    """
+    type = c_bint_type
+
+    def analyse_types(self, env):
+        return self
+
+    def generate_result_code(self, code):
+        self.has_gil = code.funcstate.gil_owned
+
+    def calculate_result_code(self):
+        return "1" if self.has_gil else "0"
diff --git a/Cython/Utility/Complex.c b/Cython/Utility/Complex.c
index 642184486..fd2cd081a 100644
--- a/Cython/Utility/Complex.c
+++ b/Cython/Utility/Complex.c
@@ -323,22 +323,27 @@ static {{type}} __Pyx_PyComplex_As_{{type_name}}(PyObject* o) {
 
 /////////////// SoftComplexToDouble.proto //////////////////
 
-static double __Pyx_SoftComplexToDouble(__pyx_t_double_complex value); /* proto */
+static double __Pyx_SoftComplexToDouble(__pyx_t_double_complex value, int have_gil); /* proto */
 
 /////////////// SoftComplexToDouble //////////////////
 //@requires: RealImag
 
-static double __Pyx_SoftComplexToDouble(__pyx_t_double_complex value) {
+static double __Pyx_SoftComplexToDouble(__pyx_t_double_complex value, int have_gil) {
     // This isn't an absolutely perfect match for the Python behaviour:
     // In Python the type would be determined right after the number is
     // created (usually '**'), while here it's determined when coerced
     // to a PyObject, which may be a few operations later.
     if (unlikely(__Pyx_CIMAG(value))) {
+        PyGILState_STATE gilstate;
+        if (!have_gil)
+            gilstate = PyGILState_Ensure();
         PyErr_SetString(PyExc_TypeError,
             "Cannot convert 'complex' with non-zero imaginary component to 'double' "
             "(this most likely comes from the '**' operator; "
             "use 'cython.cpow(True)' to return 'nan' instead of a "
             "complex number).");
+        if (!have_gil)
+            PyGILState_Release(gilstate);
         return -1.;
     }
     return __Pyx_CREAL(value);
diff --git a/tests/run/cpow.pyx b/tests/run/cpow.pyx
index d2691ba6a..6f050337c 100644
--- a/tests/run/cpow.pyx
+++ b/tests/run/cpow.pyx
@@ -208,6 +208,54 @@ def pythagoras_with_typedef(double a, double b):
     return result
 
 
+@cython.cpow(False)
+def power_coercion_in_nogil_1(double a, double b):
+    """
+    >>> power_coercion_in_nogil_1(2., 2.)
+    4.0
+    >>> power_coercion_in_nogil_1(-1., 0.5)
+    Traceback (most recent call last):
+    ...
+    TypeError: Cannot convert 'complex' with non-zero imaginary component to 'double' (this most likely comes from the '**' operator; use 'cython.cpow(True)' to return 'nan' instead of a complex number).
+    """
+    cdef double c
+    with nogil:
+        c = a**b
+    return c
+
+
+cdef double nogil_fun(double x) nogil:
+    return x
+
+def power_coercion_in_nogil_2(double a, double b):
+    """
+    >>> power_coercion_in_nogil_2(2., 2.)
+    4.0
+    >>> power_coercion_in_nogil_2(-1., 0.5)
+    Traceback (most recent call last):
+    ...
+    TypeError: Cannot convert 'complex' with non-zero imaginary component to 'double' (this most likely comes from the '**' operator; use 'cython.cpow(True)' to return 'nan' instead of a complex number).
+    """
+    c = a**b
+    with nogil:
+        d = nogil_fun(c)
+    return d
+
+
+def power_coercion_in_nogil_3(double a, double b, double c):
+    """
+    >>> power_coercion_in_nogil_3(2., 2., 1.0)
+    0.25
+    >>> power_coercion_in_nogil_3(-1., 0.5, 1.0)
+    Traceback (most recent call last):
+    ...
+    TypeError: Cannot convert 'complex' with non-zero imaginary component to 'double' (this most likely comes from the '**' operator; use 'cython.cpow(True)' to return 'nan' instead of a complex number).
+    """
+    with nogil:
+        c /= a**b
+    return c
+
+
 _WARNINGS = """
 63:21: Treating '**' as if 'cython.cpow(True)' since it is directly assigned to a a non-complex C numeric type. This is likely to be fragile and we recommend setting 'cython.cpow' explicitly.
 64:32: Treating '**' as if 'cython.cpow(True)' since it is directly assigned to a a non-complex C numeric type. This is likely to be fragile and we recommend setting 'cython.cpow' explicitly.