From 541299b72cee9c852c0f10b6d1dd42e8c54f86a7 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Sat, 26 Aug 2023 17:39:41 +0200 Subject: [PATCH] Avoid useless runtime type checks when coercing a None default argument 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 https://github.com/cython/cython/issues/5643 --- Cython/Compiler/ExprNodes.py | 13 +++-- tests/run/default_optional_gh5643.py | 86 ++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 tests/run/default_optional_gh5643.py diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py index e5e050ec408..e1adc9e88b6 100644 --- a/Cython/Compiler/ExprNodes.py +++ b/Cython/Compiler/ExprNodes.py @@ -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 @@ -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 diff --git a/tests/run/default_optional_gh5643.py b/tests/run/default_optional_gh5643.py new file mode 100644 index 00000000000..3038960a964 --- /dev/null +++ b/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. +"""