From c25c3ccc4b862592b06e66fd0fc508e4d388437b Mon Sep 17 00:00:00 2001 From: peng weikang Date: Wed, 14 Apr 2021 02:38:17 +0800 Subject: [PATCH] Remove incorrect dict unpacking optimisation that leaked external dict changes into the result (GH-4091) Closes https://github.com/cython/cython/issues/3227 --- Cython/Compiler/ExprNodes.py | 11 +---------- tests/run/dict.pyx | 19 +++++++++++++++++++ tests/run/kwargs_passthrough.pyx | 32 ++++++++++++++++++++++---------- 3 files changed, 42 insertions(+), 20 deletions(-) diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py index 970e343d587..2714d3e4d7a 100644 --- a/Cython/Compiler/ExprNodes.py +++ b/Cython/Compiler/ExprNodes.py @@ -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): diff --git a/tests/run/dict.pyx b/tests/run/dict.pyx index d51cef7da93..691b7863599 100644 --- a/tests/run/dict.pyx +++ b/tests/run/dict.pyx @@ -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())) diff --git a/tests/run/kwargs_passthrough.pyx b/tests/run/kwargs_passthrough.pyx index 8f6ef2f45f6..c09b6cba43f 100644 --- a/tests/run/kwargs_passthrough.pyx +++ b/tests/run/kwargs_passthrough.pyx @@ -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 @@ -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 @@ -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): @@ -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): @@ -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): @@ -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 @@ -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()