summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorda-woods <dw-git@d-woods.co.uk>2021-10-17 19:01:52 +0100
committerGitHub <noreply@github.com>2021-10-17 20:01:52 +0200
commitf6eeeda534dd1139a7ea35bfa627db1789082b06 (patch)
treecb5e742e4ef6d5a9366d06922d9fcb37525c2345
parenta0571a69f503b1dfadc331c84b1a8522194053f5 (diff)
downloadcython-f6eeeda534dd1139a7ea35bfa627db1789082b06.tar.gz
Fix fused cpdef default arguments (GH-4413)
A couple of things were going wrong: * they're creating CloneNodes (but not requiring the contents of the clone of the clone node to be temp) * assignment from a clone node generates cleanup code (which is against the general rules of a clone node), and also loses a reference via giveref * cpdef functions cause a small memory leak (#4412) by assigning to their constants twice. This is unfortunately difficult to test for. With this patch we no longer leak, but still duplicate a little bit of work.
-rw-r--r--Cython/Compiler/ExprNodes.py6
-rw-r--r--Cython/Compiler/FusedNode.py5
-rw-r--r--Cython/Compiler/Nodes.py8
-rw-r--r--tests/run/fused_cpdef.pyx25
4 files changed, 38 insertions, 6 deletions
diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py
index 193eece7e..303492423 100644
--- a/Cython/Compiler/ExprNodes.py
+++ b/Cython/Compiler/ExprNodes.py
@@ -13814,6 +13814,12 @@ class CloneNode(CoercionNode):
def generate_disposal_code(self, code):
pass
+ def generate_post_assignment_code(self, code):
+ # if we're assigning from a CloneNode then it's "giveref"ed away, so it does
+ # need a matching incref (ideally this should happen before the assignment though)
+ if self.is_temp: # should usually be true
+ code.put_incref(self.result(), self.ctype())
+
def free_temps(self, code):
pass
diff --git a/Cython/Compiler/FusedNode.py b/Cython/Compiler/FusedNode.py
index 918a05990..fef148fa1 100644
--- a/Cython/Compiler/FusedNode.py
+++ b/Cython/Compiler/FusedNode.py
@@ -841,7 +841,8 @@ class FusedCFuncDefNode(StatListNode):
for arg in self.node.args:
if arg.default:
arg.default = arg.default.analyse_expressions(env)
- defaults.append(ProxyNode(arg.default))
+ # coerce the argument to temp since CloneNode really requires a temp
+ defaults.append(ProxyNode(arg.default.coerce_to_temp(env)))
else:
defaults.append(None)
@@ -851,7 +852,7 @@ class FusedCFuncDefNode(StatListNode):
# the dispatcher specifically doesn't want its defaults overriding
for arg, default in zip(stat.args, defaults):
if default is not None:
- arg.default = CloneNode(default).coerce_to(arg.type, env)
+ arg.default = CloneNode(default).analyse_expressions(env).coerce_to(arg.type, env)
if self.py_func:
args = [CloneNode(default) for default in defaults if default]
diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py
index 9765b4cb2..aa05a26b8 100644
--- a/Cython/Compiler/Nodes.py
+++ b/Cython/Compiler/Nodes.py
@@ -2350,9 +2350,11 @@ class FuncDefNode(StatNode, BlockNode):
def generate_execution_code(self, code):
code.mark_pos(self.pos)
# Evaluate and store argument default values
- for arg in self.args:
- if not arg.is_dynamic:
- arg.generate_assignment_code(code)
+ # skip this for wrappers since it's done by wrapped function
+ if not self.is_wrapper:
+ for arg in self.args:
+ if not arg.is_dynamic:
+ arg.generate_assignment_code(code)
#
# Special code for the __getbuffer__ function
diff --git a/tests/run/fused_cpdef.pyx b/tests/run/fused_cpdef.pyx
index 4a614e0f4..3378a5083 100644
--- a/tests/run/fused_cpdef.pyx
+++ b/tests/run/fused_cpdef.pyx
@@ -1,4 +1,4 @@
-# cython: language_level=3
+# cython: language_level=3str
# mode: run
cimport cython
@@ -186,3 +186,26 @@ def test_ambiguousmatch():
Traceback (most recent call last):
TypeError: Function call with ambiguous argument types
"""
+
+# https://github.com/cython/cython/issues/4409
+# default arguments + fused cpdef were crashing
+cpdef literal_default(cython.integral x, some_string="value"):
+ return x, some_string
+
+cpdef mutable_default(cython.integral x, some_value=[]):
+ some_value.append(x)
+ return some_value
+
+def test_defaults():
+ """
+ >>> literal_default(1)
+ (1, 'value')
+ >>> literal_default(1, "hello")
+ (1, 'hello')
+ >>> mutable_default(1)
+ [1]
+ >>> mutable_default(2)
+ [1, 2]
+ >>> mutable_default(3,[])
+ [3]
+ """