Repository: cython/cython
Pull request: Warn when function returning python object is marked as noexcept
Fixes #5661


Problem statement:
[BUG] Warn when noexcept is ignored
### Describe the bug

We should warn when `noexcept` is ignored - just to stop people thinking they're optimizing something that they aren't

### Code to reproduce the behaviour:

```cython
cdef object xx() noexcept:
    pass
```

This also applies to memoryviews

### Expected behaviour

This should generate a warning since exceptions are always checked for functions returning an object

### OS

Linux

### Python version

_No response_

### Cython version

current master

### Additional context

We might have to be a bit careful about fused functions - we probably don't want to generate warnings when *some* of the return types are valid for noexcept

Hints from issue discussion:
Should also obviously only apply to a manual noexcept, not an implicit noexcept
Just a thought. Why not directly generate an error? When an object is returned the `noexcept` is not correctly used.
I just want to add the context of my stackoverflow question that lead to this issue:

https://stackoverflow.com/questions/76972950/cython-returned-memoryview-is-always-considered-uninitialized

I think an error would be acceptable to me for my case as long as it is well described or links to documentation describing the situation. Especially for memoryviews where I'm not sure I completely agree with compiler directives not having an effect on memoryview slicing...but I don't know all the ins and outs of the design so an error as an alternative seems good.
I think if I were starting from scratch I'd do an error. I think now it has to be a warning to avoid breaking existing working code.
> it has to be a warning to avoid breaking existing working code.

Yes. Let's make it a visible warning so that users can adapt, and make it clear in the message that the code is wrong and will be considered an error eventually. We should also issue that warning in a future 0.29.x release. We can still turn it into an error in 3.1, when users had time to adapt.

Commit message: Warn only when noexcept is explicitly declared

Patch to review:
diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py
index de3a9464b0b..2185cd447a4 100644
--- a/Cython/Compiler/Nodes.py
+++ b/Cython/Compiler/Nodes.py
@@ -625,6 +625,7 @@ class CFuncDeclaratorNode(CDeclaratorNode):
     # has_varargs      boolean
     # exception_value  ConstNode or NameNode    NameNode when the name of a c++ exception conversion function
     # exception_check  boolean or "+"    True if PyErr_Occurred check needed, "+" for a c++ check
+    # exc_clause       boolean    True if exception clause is explicitly declared
     # nogil            boolean    Can be called without gil
     # with_gil         boolean    Acquire gil around function body
     # is_const_method  boolean    Whether this is a const method
@@ -718,6 +719,8 @@ def analyse(self, return_type, env, nonempty=0, directive_locals=None, visibilit
                 and (self.exception_value or self.exception_check)
                 and self.exception_check != '+'):
             error(self.pos, "Exception clause not allowed for function returning Python object")
+        elif return_type.is_pyobject and not self.exception_check and visibility != 'extern' and self.exc_clause:
+            warning(self.pos, "noexcept clause is ignored for function returning Python object", 1)
         else:
             if self.exception_value is None and self.exception_check and self.exception_check != '+':
                 # Use an explicit exception return value to speed up exception checks.
@@ -2691,6 +2694,7 @@ def analyse_declarations(self, env):
             defining=self.body is not None, modifiers=self.modifiers,
             overridable=self.overridable, in_pxd=self.inline_in_pxd)
         self.return_type = typ.return_type
+
         if self.return_type.is_array and self.visibility != 'extern':
             error(self.pos, "Function cannot return an array")
         if self.return_type.is_cpp_class:
diff --git a/Cython/Compiler/Parsing.py b/Cython/Compiler/Parsing.py
index 34d23891590..52f8b94512c 100644
--- a/Cython/Compiler/Parsing.py
+++ b/Cython/Compiler/Parsing.py
@@ -2945,7 +2945,7 @@ def p_c_func_declarator(s, pos, ctx, base, cmethod_flag):
     return Nodes.CFuncDeclaratorNode(pos,
         base = base, args = args, has_varargs = ellipsis,
         exception_value = exc_val, exception_check = exc_check,
-        nogil = nogil or ctx.nogil or with_gil, with_gil = with_gil)
+        nogil = nogil or ctx.nogil or with_gil, with_gil = with_gil, exc_clause=exc_clause)
 
 supported_overloaded_operators = cython.declare(frozenset, frozenset((
     '+', '-', '*', '/', '%',
diff --git a/tests/errors/w_noexcept.pyx b/tests/errors/w_noexcept.pyx
new file mode 100644
index 00000000000..051fe897b5b
--- /dev/null
+++ b/tests/errors/w_noexcept.pyx
@@ -0,0 +1,40 @@
+# mode: error
+# tag: werror
+
+cdef object test_return_object_noexcept(x) noexcept:
+    return x
+
+cdef str test_return_str_noexcept() noexcept:
+    return 'a'
+
+cdef test_noexcept() noexcept:
+    pass
+
+cdef test_implicit_noexcept():
+    pass
+
+cdef object test_return_object(x):
+    return x
+
+cdef str test_return_str():
+    return 'a'
+
+cdef extern from *:
+    cdef object extern_return_object():
+        pass
+
+    cdef object extern_noexcept() noexcept:
+        pass
+
+ctypedef fused double_or_object:
+    double
+    object
+
+cdef double_or_object test_fused_noexcept(double_or_object x) noexcept:
+    pass
+
+_ERRORS = """
+4:39: noexcept clause is ignored for function returning Python object
+7:33: noexcept clause is ignored for function returning Python object
+10:18: noexcept clause is ignored for function returning Python object
+"""


Code review comments:
diff_hunk: @@ -625,6 +625,7 @@ class CFuncDeclaratorNode(CDeclaratorNode):
     # has_varargs      boolean
     # exception_value  ConstNode or NameNode    NameNode when the name of a c++ exception conversion function
     # exception_check  boolean or "+"    True if PyErr_Occurred check needed, "+" for a c++ check
+    # exc_clause       boolean    True if exception clause is explicitly declared
path: Cython/Compiler/Nodes.py
text: @user1:
I don't really like the name - maybe `has_explicit_exc_clause`?

I also have a vague memory we already have this information somewhere - I'll try to have a look later. I could be misremembering though.

@author:
Parser seems to not pass further exc_clause:
https://github.com/cython/cython/blob/e9b08d3f071ebfad7f4886e43173cd0f72df6676/Cython/Compiler/Parsing.py#L2960-L2976