Commit f6eeeda5 authored by da-woods's avatar da-woods Committed by GitHub

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.
parent a0571a69
......@@ -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
......
......@@ -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]
......
......@@ -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
......
# 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]
"""
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment