Skip to content

Commit

Permalink
Remove incorrect dict unpacking optimisation that leaked external dic…
Browse files Browse the repository at this point in the history
…t changes into the result (cythonGH-4091)

Closes cython#3227
  • Loading branch information
pengwk authored Apr 13, 2021
1 parent afe3abb commit c25c3cc
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 20 deletions.
11 changes: 1 addition & 10 deletions Cython/Compiler/ExprNodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -6756,22 +6756,13 @@ def infer_type(self, env):
return dict_type

def analyse_types(self, env):
args = [
self.keyword_args = [
arg.analyse_types(env).coerce_to_pyobject(env).as_none_safe_node(
# FIXME: CPython's error message starts with the runtime function name
'argument after ** must be a mapping, not NoneType')
for arg in self.keyword_args
]

if len(args) == 1 and args[0].type is dict_type:
# strip this intermediate node and use the bare dict
arg = args[0]
if arg.is_name and arg.entry.is_arg and len(arg.entry.cf_assignments) == 1:
# passing **kwargs through to function call => allow NULL
arg.allow_null = True
return arg

self.keyword_args = args
return self

def may_be_none(self):
Expand Down
19 changes: 19 additions & 0 deletions tests/run/dict.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,22 @@ def item_creation_sideeffect(L, sideeffect, unhashable):
[2, 4, 5]
"""
return {1:2, sideeffect(2): 3, 3: 4, unhashable(4): 5, sideeffect(5): 6}


def dict_unpacking_not_for_arg_create_a_copy():
"""
>>> dict_unpacking_not_for_arg_create_a_copy()
[('a', 'modified'), ('b', 'original')]
[('a', 'original'), ('b', 'original')]
"""
data = {'a': 'original', 'b': 'original'}

func = lambda: {**data}

call_once = func()
call_once['a'] = 'modified'

call_twice = func()

print(sorted(call_once.items()))
print(sorted(call_twice.items()))
32 changes: 22 additions & 10 deletions tests/run/kwargs_passthrough.pyx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
cimport cython
import cython


@cython.test_fail_if_path_exists('//MergedDictNode')
#@cython.test_fail_if_path_exists('//MergedDictNode')
def wrap_passthrough(f):
"""
>>> def f(a=1): return a
Expand Down Expand Up @@ -80,7 +79,7 @@ def wrap_passthrough_more(f):
return wrapper


@cython.test_fail_if_path_exists('//MergedDictNode')
#@cython.test_fail_if_path_exists('//MergedDictNode')
def wrap_passthrough2(f):
"""
>>> def f(a=1): return a
Expand All @@ -99,7 +98,7 @@ def wrap_passthrough2(f):
return wrapper


@cython.test_fail_if_path_exists('//MergedDictNode')
#@cython.test_fail_if_path_exists('//MergedDictNode')
def wrap_modify(f):
"""
>>> def f(a=1, test=2):
Expand All @@ -123,7 +122,7 @@ def wrap_modify(f):
return wrapper


@cython.test_fail_if_path_exists('//MergedDictNode')
#@cython.test_fail_if_path_exists('//MergedDictNode')
def wrap_modify_mix(f):
"""
>>> def f(a=1, test=2):
Expand Down Expand Up @@ -187,7 +186,21 @@ def wrap_modify_func(f):
return wrapper


@cython.test_assert_path_exists('//MergedDictNode')
def modify_in_function():
"""
>>> modify_in_function()
{'foo': 'bar'}
{'foo': 'bar'}
"""
def inner(**kwds):
kwds['foo'] = 'modified'
d = {'foo': 'bar'}
print(d)
inner(**d)
print(d)


#@cython.test_assert_path_exists('//MergedDictNode')
def wrap_modify_func_mix(f):
"""
>>> def f(a=1, test=2):
Expand Down Expand Up @@ -215,12 +228,11 @@ def wrap_modify_func_mix(f):
return wrapper


@cython.test_fail_if_path_exists('//MergedDictNode')
#@cython.test_fail_if_path_exists('//MergedDictNode')
def wrap_reassign(f):
"""
>>> def f(a=1, test=2):
... return a, test
>>> wrapped = wrap_reassign(f)
>>> wrapped(1)
CALLED
Expand All @@ -239,7 +251,7 @@ def wrap_reassign(f):
return wrapper


@cython.test_fail_if_path_exists('//MergedDictNode')
#@cython.test_fail_if_path_exists('//MergedDictNode')
def kwargs_metaclass(**kwargs):
"""
>>> K = kwargs_metaclass()
Expand Down

0 comments on commit c25c3cc

Please sign in to comment.