Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Cython 3.0.1 compile time crash: None Default Value #5643

Closed
RaubCamaioni opened this issue Aug 25, 2023 · 3 comments · Fixed by #5652
Closed

[BUG] Cython 3.0.1 compile time crash: None Default Value #5643

RaubCamaioni opened this issue Aug 25, 2023 · 3 comments · Fixed by #5652

Comments

@RaubCamaioni
Copy link

RaubCamaioni commented Aug 25, 2023

Cython 3.0.1 compile time crash: None Default Value

Cython 3.0.1 crashes during compilation when None default present AND other default values in argument.
Cython 3.0.0 does not crash when None default values are present.

Crashes in both pure python and cython code. py/pyx

Snippet of error:

        File "/tmp/pip-build-env-ywx0l3dj/overlay/lib/python3.8/site-packages/Cython/Compiler/ExprNodes.py", line 846, in generate_subexpr_evaluation_code
          node.generate_evaluation_code(code)
        File "/tmp/pip-build-env-ywx0l3dj/overlay/lib/python3.8/site-packages/Cython/Compiler/ExprNodes.py", line 837, in generate_evaluation_code
          self.generate_result_code(code)
        File "/tmp/pip-build-env-ywx0l3dj/overlay/lib/python3.8/site-packages/Cython/Compiler/ExprNodes.py", line 8017, in generate_result_code
          self.generate_operation_code(code)
        File "/tmp/pip-build-env-ywx0l3dj/overlay/lib/python3.8/site-packages/Cython/Compiler/ExprNodes.py", line 8576, in generate_operation_code
          self.generate_sequence_packing_code(code)
        File "/tmp/pip-build-env-ywx0l3dj/overlay/lib/python3.8/site-packages/Cython/Compiler/ExprNodes.py", line 8084, in generate_sequence_packing_code
          code.put_incref(arg.result(), arg.ctype())
        File "/tmp/pip-build-env-ywx0l3dj/overlay/lib/python3.8/site-packages/Cython/Compiler/ExprNodes.py", line 10113, in result
          self.defaults_struct.lookup(self.arg.name).cname)
      AttributeError: 'NoneType' object has no attribute 'cname'

Code to reproduce the behaviour:

Compiling the following code causes cython 3.0.1 to crash.

crash line

File "/tmp/pip-build-env-ywx0l3dj/overlay/lib/python3.8/site-packages/Cython/Compiler/ExprNodes.py", line 10113, in result
          self.defaults_struct.lookup(self.arg.name).cname)
      AttributeError: 'NoneType' object has no attribute 'cname'

reproduce crash code

from typing import Optional

# no crash
def foo1(a: Optional[int] = None):
    return 1

# no crash
def foo(a: int = 1, b = None):
    return 1

# crash
def foo(a: int = 1, b: int = None):
    return 1

# crash
def foo(a: int = 1, b: Optional[int] = None):
    return 1

Expected behaviour

The compiler should not crash if None default value is present with other default values in function argument definition.

If default None types are not allowed, interpret as python object.
Send compile warning to user None default value assignment not allowed.

OS

Debian GNU/Linux 12 (bookworm) in Docker in WLS2 in windows

Python version

3.8.17

Cython version

3.0.1

Additional context

Cython is amazing :).

@RaubCamaioni RaubCamaioni changed the title [BUG] [BUG] Cython 3.0.1 compile time crash Aug 25, 2023
@RaubCamaioni RaubCamaioni changed the title [BUG] Cython 3.0.1 compile time crash [BUG] Cython 3.0.1 compile time crash: None Default Value Aug 25, 2023
@scoder
Copy link
Contributor

scoder commented Aug 26, 2023

Thanks for the short reproducer. I can add that this requires language_level=3(str) and doesn't occur with language_level=2, which we still use for a large part of our test suite for legacy reasons.

@scoder
Copy link
Contributor

scoder commented Aug 26, 2023

I investigated this a little and found that we generate a type check for the default argument (None) that then gets in the way so that we don't detect the default value as a literal any more (it's now a type checked literal) and instead generate a more complex setup for the default arguments. One unfortunate coincidence here is that the type of int is ambiguous between Py2 and Py3, so we coerce the None default value as arbitrary Python object (forgetting that it is compile time compatible with int) and then add the runtime type check against int. If int was a simple type like any other and not "int or long", this would be easily resolved by assigning the type int to the default argument.

I can add a special case for None when coercing it as default argument to avoid the runtime check, thus simplifying the generated C code and avoiding the problem all together.

scoder added a commit to scoder/cython that referenced this issue Aug 26, 2023
…nt to a Python type argument.

For Python int (which we turn into a plain Python object type internally to avoid Py2 int/long issues), we previously generated a type check which made the None default argument much more complex than it was.

Closes cython#5643
@scoder scoder added this to the 3.0.2 milestone Aug 26, 2023
scoder added a commit to scoder/cython that referenced this issue Aug 26, 2023
…integer) default value to a fused type generated complex argument (which ends up as a struct).

Closes cython#5643
scoder added a commit to scoder/cython that referenced this issue Aug 26, 2023
…integer) default value to a fused type generated complex argument (which ends up as a struct).

Closes cython#5643
scoder added a commit that referenced this issue Aug 26, 2023
…nt to a Python type argument. (GH-5652)

For Python int (which we turn into a plain Python object type internally to avoid Py2 int/long issues), we previously generated a type check which made the None default argument much more complex than it was.

Closes #5643
@hughmiles-atamate
Copy link

I got the same error from this line of Python source:

def set_single_coil(self, slave_id: int = 0, coil_address: int = 0, coil_status: int = True) -> None:

The code was cythoned correctly in version 3.0.0.

I changed the line to

def set_single_coil(self, slave_id: int = 0, coil_address: int = 0, coil_status: bool = True) -> None:

and it cythoned correctly in 3.0.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants