[pypy-commit] pypy py3.5: Tweak/fix the logic for multiple '**' arguments. Also makes the error messages equal

[arigo]

Author: Armin Rigo <arigo at tunes.org>
Branch: py3.5
Changeset: r87786:a72722e30393
Date: 2016-10-14 15:19 +0200
http://bitbucket.org/pypy/pypy/changeset/a72722e30393/

Log:	Tweak/fix the logic for multiple '**' arguments. Also makes the
	error messages equal to the ones we get from argument.py. Now all
	messages are missing the 'funcname()' prefix that CPython gives
	them, but it's at least consistent---and harder to fix.

diff --git a/pypy/interpreter/argument.py b/pypy/interpreter/argument.py
--- a/pypy/interpreter/argument.py
+++ b/pypy/interpreter/argument.py
@@ -403,7 +403,9 @@
             key = space.identifier_w(w_key)
         except OperationError as e:
             if e.match(space, space.w_TypeError):
-                raise oefmt(space.w_TypeError, "keywords must be strings")
+                raise oefmt(space.w_TypeError,
+                            "keywords must be strings, not '%T'",
+                            w_key)
             if e.match(space, space.w_UnicodeEncodeError):
                 # Allow this to pass through
                 key = None
diff --git a/pypy/interpreter/pyopcode.py b/pypy/interpreter/pyopcode.py
--- a/pypy/interpreter/pyopcode.py
+++ b/pypy/interpreter/pyopcode.py
@@ -46,14 +46,6 @@
 
     return func_with_new_name(opimpl, "opcode_impl_for_%s" % operationname)
 
-def get_func_desc(space, func):
-    if isinstance(func,function.Function):
-        return "()"
-    elif isinstance(func, function.Method):
-        return "()"
-    else:
-        return " object";
-
 
 opcodedesc = bytecode_spec.opcodedesc
 HAVE_ARGUMENT = bytecode_spec.HAVE_ARGUMENT
@@ -1389,54 +1381,50 @@
         w_sum = self.list_unpack_helper(itemcount)
         self.pushvalue(w_sum)
 
+    def BUILD_MAP_UNPACK(self, itemcount, next_instr):
+        self._build_map_unpack(itemcount, with_call=False)
+
+    def BUILD_MAP_UNPACK_WITH_CALL(self, oparg, next_instr):
+        num_maps = oparg & 0xff
+        self._build_map_unpack(num_maps, with_call=True)
+
     @jit.unroll_safe
-    def BUILD_MAP_UNPACK_WITH_CALL(self, itemcount, next_instr):
+    def _build_map_unpack(self, itemcount, with_call):
         space = self.space
-        num_maps = itemcount & 0xff
-        function_location = (itemcount>>8) & 0xff
         w_dict = space.newdict()
-        for i in range(num_maps, 0, -1):
-            w_item = self.peekvalue(i-1)
+        expected_length = 0
+        for i in range(itemcount-1, -1, -1):
+            w_item = self.peekvalue(i)
             if not space.ismapping_w(w_item):
                 raise oefmt(space.w_TypeError,
-                        "'%T' object is not a mapping", w_item)
-            iterator = w_item.iterkeys()
-            while True:
-                w_key = iterator.next_key()
-                if w_key is None:
-                    break
-                if not space.isinstance_w(w_key, space.w_unicode):
-                    err_fun = self.peekvalue(num_maps + function_location-1)
-                    raise oefmt(space.w_TypeError,
-                        "%N%s keywords must be strings", err_fun,
-                                                         get_func_desc(space, err_fun))
-                if space.is_true(space.contains(w_dict,w_key)):
-                    err_fun = self.peekvalue(num_maps + function_location-1)
-                    err_arg = w_key
-                    raise oefmt(space.w_TypeError,
-                        "%N%s got multiple values for keyword argument '%s'",
-                          err_fun, get_func_desc(space, err_fun), space.str_w(err_arg))
+                            "'%T' object is not a mapping", w_item)
+            if with_call:
+                expected_length += space.len_w(w_item)
             space.call_method(w_dict, 'update', w_item)
-        while num_maps != 0:
-            self.popvalue()
-            num_maps -= 1
-        self.pushvalue(w_dict)
-
-    @jit.unroll_safe
-    def BUILD_MAP_UNPACK(self, itemcount, next_instr):
-        space = self.space
-        w_dict = space.newdict()
-        for i in range(itemcount, 0, -1):
-            w_item = self.peekvalue(i-1)
-            if not space.ismapping_w(w_item):
-                raise oefmt(self.space.w_TypeError,
-                        "'%T' object is not a mapping", w_item)
-            space.call_method(w_dict, 'update', w_item)
-        while itemcount != 0:
+        if with_call and space.len_w(w_dict) < expected_length:
+            self._build_map_unpack_error(itemcount)
+        while itemcount > 0:
             self.popvalue()
             itemcount -= 1
         self.pushvalue(w_dict)
 
+    @jit.dont_look_inside
+    def _build_map_unpack_error(self, itemcount):
+        space = self.space
+        w_set = space.newset()
+        for i in range(itemcount-1, -1, -1):
+            w_item = self.peekvalue(i)
+            w_inter = space.call_method(w_set, 'intersection', w_item)
+            if space.is_true(w_inter):
+                w_key = space.next(space.iter(w_inter))
+                if not space.isinstance_w(w_key, space.w_unicode):
+                    raise oefmt(space.w_TypeError,
+                            "keywords must be strings, not '%T'", w_key)
+                raise oefmt(space.w_TypeError,
+                    "got multiple values for keyword argument %R",
+                    w_key)
+            space.call_method(w_set, 'update', w_item)
+
     def GET_YIELD_FROM_ITER(self, oparg, next_instr):
         from pypy.interpreter.astcompiler import consts
         from pypy.interpreter.generator import GeneratorIterator, Coroutine
diff --git a/pypy/interpreter/test/test_argument.py b/pypy/interpreter/test/test_argument.py
--- a/pypy/interpreter/test/test_argument.py
+++ b/pypy/interpreter/test/test_argument.py
@@ -878,3 +878,11 @@
             return (x, y)
         assert f(**{'x': 5}, y=6) == (5, 6)
         """
+
+    def test_error_message_kwargs(self):
+        def f(x, y):
+            pass
+        e = raises(TypeError, "f(y=2, **{3: 5}, x=6)")
+        assert "keywords must be strings" in str(e.value)
+        e = raises(TypeError, "f(y=2, **{'x': 5}, x=6)")
+        assert "got multiple values for keyword argument 'x'" in str(e.value)