Skip to content

Commit

Permalink
Avoid useless runtime type checks when coercing a None default argume…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
scoder committed Aug 26, 2023
1 parent f34e0bb commit 541299b
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 4 deletions.
13 changes: 9 additions & 4 deletions Cython/Compiler/ExprNodes.py
Expand Up @@ -1034,14 +1034,18 @@ def coerce_to(self, dst_type, env):
error(self.pos, msg % tup)

elif dst_type.is_pyobject:
if not src.type.is_pyobject:
# We never need a type check when assigning None to a Python object type.
if src.is_none:
pass
elif src.constant_result is None:
src = NoneNode(src.pos).coerce_to(dst_type, env)
elif not src.type.is_pyobject:
if dst_type is bytes_type and src.type.is_int:
src = CoerceIntToBytesNode(src, env)
else:
src = CoerceToPyTypeNode(src, env, type=dst_type)
if not src.type.subtype_of(dst_type):
if src.constant_result is not None:
src = PyTypeTestNode(src, dst_type, env)
elif not src.type.subtype_of(dst_type):
src = PyTypeTestNode(src, dst_type, env)
elif is_pythran_expr(dst_type) and is_pythran_supported_type(src.type):
# We let the compiler decide whether this is valid
return src
Expand Down Expand Up @@ -10068,6 +10072,7 @@ class DefaultLiteralArgNode(ExprNode):
def __init__(self, pos, arg):
super(DefaultLiteralArgNode, self).__init__(pos)
self.arg = arg
self.constant_result = arg.constant_result
self.type = self.arg.type
self.evaluated = False

Expand Down
86 changes: 86 additions & 0 deletions tests/run/default_optional_gh5643.py
@@ -0,0 +1,86 @@
# mode: run
# tag: pep484, warnings, pure3.6
# ticket: 5643
# cython: language_level=3

try:
from typing import Optional
except ImportError:
pass


# no crash
def gh5643_optional(a: Optional[int] = None):
"""
>>> gh5643_optional()
True
>>> gh5643_optional(1)
False
"""
return a is None


# no crash
def gh5643_int_untyped(a: int = 1, b = None):
"""
>>> gh5643_int_untyped(2)
(False, True)
>>> gh5643_int_untyped(2, None)
(False, True)
>>> gh5643_int_untyped(1, 3)
(True, False)
"""
return a == 1, b is None


# used to crash
def gh5643_int_int_none(a: int = 1, b: int = None): # should warn about missing "Optional[]"
"""
>>> gh5643_int_int_none()
(True, True)
>>> gh5643_int_int_none(2, 3)
(False, False)
"""
return a == 1, b is None


def gh5643_int_int_integer(a: int = 1, b: int = 3):
"""
>>> gh5643_int_int_integer()
(True, True)
>>> gh5643_int_int_integer(2, 3)
(False, True)
"""
return a == 1, b == 3


# used to crash
def gh5643_int_optional_none(a: int = 1, b: Optional[int] = None):
"""
>>> gh5643_int_optional_none()
(True, True)
>>> gh5643_int_optional_none(2)
(False, True)
>>> gh5643_int_optional_none(2, 3)
(False, False)
"""
return a == 1, b is None


def gh5643_int_optional_integer(a: int = 1, b: Optional[int] = 2):
"""
>>> gh5643_int_optional_integer()
(True, True)
>>> gh5643_int_optional_integer(2)
(False, True)
>>> gh5643_int_optional_integer(2, 3)
(False, False)
>>> gh5643_int_optional_integer(2, 2)
(False, True)
"""
return a == 1, b == 2


_WARNINGS = """
37:36: PEP-484 recommends 'typing.Optional[...]' for arguments that can be None.
"""

0 comments on commit 541299b

Please sign in to comment.