Merge pull request #1466 from sanxiyn/unpack-for-call

Raise TypeError for duplicate keyword arguments

bytecode/src/bytecode.rs

@@ -243,6 +243,7 @@ pub enum Instruction {
     BuildMap {
         size: usize,
         unpack: bool,
+        for_call: bool,
     },
     BuildSlice {
         size: usize,
@@ -519,7 +520,11 @@ impl Instruction {
             BuildTuple { size, unpack } => w!(BuildTuple, size, unpack),
             BuildList { size, unpack } => w!(BuildList, size, unpack),
             BuildSet { size, unpack } => w!(BuildSet, size, unpack),
-            BuildMap { size, unpack } => w!(BuildMap, size, unpack),
+            BuildMap {
+                size,
+                unpack,
+                for_call,
+            } => w!(BuildMap, size, unpack, for_call),
             BuildSlice { size } => w!(BuildSlice, size),
             ListAppend { i } => w!(ListAppend, i),
             SetAdd { i } => w!(SetAdd, i),

compiler/src/compile.rs

@@ -658,6 +658,7 @@ impl<O: OutputStream> Compiler<O> {
             self.emit(Instruction::BuildMap {
                 size: num_kw_only_defaults,
                 unpack: false,
+                for_call: false,
             });
         }
 
@@ -896,6 +897,7 @@ impl<O: OutputStream> Compiler<O> {
             self.emit(Instruction::BuildMap {
                 size: num_annotations,
                 unpack: false,
+                for_call: false,
             });
         }
 
@@ -1464,6 +1466,7 @@ impl<O: OutputStream> Compiler<O> {
                 self.emit(Instruction::BuildMap {
                     size: subsize,
                     unpack: false,
+                    for_call: false,
                 });
                 size += 1;
             }
@@ -1472,10 +1475,15 @@ impl<O: OutputStream> Compiler<O> {
             self.emit(Instruction::BuildMap {
                 size,
                 unpack: false,
+                for_call: false,
             });
         }
         if size > 1 || has_unpacking {
-            self.emit(Instruction::BuildMap { size, unpack: true });
+            self.emit(Instruction::BuildMap {
+                size,
+                unpack: true,
+                for_call: false,
+            });
         }
         Ok(())
     }
@@ -1704,12 +1712,17 @@ impl<O: OutputStream> Compiler<O> {
                 self.emit(Instruction::BuildMap {
                     size: subsize,
                     unpack: false,
+                    for_call: false,
                 });
                 size += 1;
             }
         }
         if size > 1 {
-            self.emit(Instruction::BuildMap { size, unpack: true });
+            self.emit(Instruction::BuildMap {
+                size,
+                unpack: true,
+                for_call: true,
+            });
         }
         Ok(())
     }
@@ -1855,6 +1868,7 @@ impl<O: OutputStream> Compiler<O> {
                 self.emit(Instruction::BuildMap {
                     size: 0,
                     unpack: false,
+                    for_call: false,
                 });
             }
         }

tests/snippets/function_args.py

@@ -105,3 +105,12 @@ with assert_raises(SyntaxError):
 
 with assert_raises(SyntaxError):
     exec("def f(a=1, b): pass")
+
+
+def f(a):
+    pass
+
+x = {'a': 1}
+y = {'a': 2}
+with assert_raises(TypeError):
+    f(**x, **y)

vm/src/frame.rs

@@ -284,9 +284,11 @@ impl Frame {
                 self.push_value(list_obj);
                 Ok(None)
             }
-            bytecode::Instruction::BuildMap { size, unpack } => {
-                self.execute_build_map(vm, *size, *unpack)
-            }
+            bytecode::Instruction::BuildMap {
+                size,
+                unpack,
+                for_call,
+            } => self.execute_build_map(vm, *size, *unpack, *for_call),
             bytecode::Instruction::BuildSlice { size } => self.execute_build_slice(vm, *size),
             bytecode::Instruction::ListAppend { i } => {
                 let list_obj = self.nth_value(*i);
@@ -809,13 +811,30 @@ impl Frame {
         Ok(None)
     }
 
-    fn execute_build_map(&self, vm: &VirtualMachine, size: usize, unpack: bool) -> FrameResult {
+    #[allow(clippy::collapsible_if)]
+    fn execute_build_map(
+        &self,
+        vm: &VirtualMachine,
+        size: usize,
+        unpack: bool,
+        for_call: bool,
+    ) -> FrameResult {
         let map_obj = vm.ctx.new_dict();
         if unpack {
             for obj in self.pop_multiple(size) {
                 // Take all key-value pairs from the dict:
                 let dict: PyDictRef = obj.downcast().expect("Need a dictionary to build a map.");
                 for (key, value) in dict {
+                    if for_call {
+                        if map_obj.contains_key(&key, vm) {
+                            let key_repr = vm.to_repr(&key)?;
+                            let msg = format!(
+                                "got multiple values for keyword argument {}",
+                                key_repr.as_str()
+                            );
+                            return Err(vm.new_type_error(msg));
+                        }
+                    }
                     map_obj.set_item(&key, value, vm).unwrap();
                 }
             }