2 files changed, 62 insertions, 34 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
index 2afcbc3017..cb3372430c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
@@ -32,8 +32,8 @@ import com.google.devtools.build.lib.syntax.Type.ConversionException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.Iterator;
+import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
@@ -1186,37 +1186,52 @@ public class MethodLibrary {
       + "will overwrite values from the positional argument if a key appears multiple times. "
       + "Dictionaries are always sorted by their keys",
       optionalPositionals = {
-          @Param(name = "args", type = Iterable.class, defaultValue = "[]",
+          @Param(name = "args", type = Object.class, defaultValue = "[]",
               doc =
-              "List of entries. Entries must be tuples or lists with exactly "
-              + "two elements: key, value"),
+              "Either a dictionary or a list of entries. Entries must be tuples or lists with "
+              + "exactly two elements: key, value"),
       },
       extraKeywords = {@Param(name = "kwargs", doc = "Dictionary of additional entries.")},
       useLocation = true)
   private static final BuiltinFunction dict = new BuiltinFunction("dict") {
     @SuppressWarnings("unused")
-    public Map<Object, Object> invoke(Iterable<Object> args, Map<Object, Object> kwargs,
-        Location loc) throws EvalException, ConversionException {
+    public Map<Object, Object> invoke(Object args, Map<Object, Object> kwargs, Location loc)
+        throws EvalException {
+      Map<Object, Object> result =
+          (args instanceof Map<?, ?>)
+              ? new LinkedHashMap<>((Map<?, ?>) args) : getMapFromArgs(args, loc);
+      result.putAll(kwargs);
+      return result;
+    }
+
+    private Map<Object, Object> getMapFromArgs(Object args, Location loc) throws EvalException {
+      Map<Object, Object> result = new LinkedHashMap<>();
+      int pos = 0;
+      for (Object element : Type.OBJECT_LIST.convert(args, "parameter args in dict()")) {
+        List<Object> pair = convertToPair(element, pos, loc);
+        result.put(pair.get(0), pair.get(1));
+        ++pos;
+      }
+      return result;
+    }
+
+    private List<Object> convertToPair(Object element, int pos, Location loc)
+        throws EvalException {
       try {
-        Map<Object, Object> result = new HashMap<>();
-        List<Object> list = Type.OBJECT_LIST.convert(args, "dict(args)");
-
-        for (Object tuple : list) {
-          List<Object> mapping = Type.OBJECT_LIST.convert(tuple, "dict(args)");
-          int numElements = mapping.size();
-
-          if (numElements != 2) {
-            throw new EvalException(
-                location,
-                String.format(
-                    "Tuple has length %d, but exactly two elements are required", numElements));
-          }
-          result.put(mapping.get(0), mapping.get(1));
+        List<Object> tuple = Type.OBJECT_LIST.convert(element, "");
+        int numElements = tuple.size();
+        if (numElements != 2) {
+          throw new EvalException(
+              location,
+              String.format("Sequence #%d has length %d, but exactly two elements are required",
+                  pos, numElements));
         }
-        result.putAll(kwargs);
-        return result;
-      } catch (IllegalArgumentException | ClassCastException | NullPointerException ex) {
-        throw new EvalException(loc, ex);
+        return tuple;
+      } catch (ConversionException e) {
+        throw new EvalException(
+            loc,
+            String.format(
+                "Cannot convert dictionary update sequence element #%d to a sequence", pos));
       }
     }
   };
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
index 72fd205ecf..de926812a7 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/MethodLibraryTest.java
@@ -123,8 +123,8 @@ public class MethodLibraryTest extends EvaluationTestCase {
             + "but should be string",
             "'test'.startswith(1)")
         .testIfErrorContains(
-            "Method dict(args: Iterable, **kwargs) is not applicable for arguments "
-            + "(string, dict): 'args' is string, but should be Iterable",
+            "expected value of type 'list(object)' for parameter args in dict(), "
+            + "but got \"a\" (string)",
             "dict('a')");
   }
 
@@ -647,6 +647,20 @@ public class MethodLibraryTest extends EvaluationTestCase {
   }
 
   @Test
+  public void testDictionaryCopy() throws Exception {
+    new SkylarkTest()
+        .setUp("x = {1 : 2}", "y = dict(x)")
+        .testEval("x[1] == 2 and y[1] == 2", "True");
+  }
+
+  @Test
+  public void testDictionaryCopyKeyCollision() throws Exception {
+    new SkylarkTest()
+        .setUp("x = {'test' : 2}", "y = dict(x, test = 3)")
+        .testEval("y['test']", "3");
+  }
+
+  @Test
   public void testDictionaryWithMultipleKeys() throws Exception {
     new BothModesTest().testStatement("{0: 'a', 1: 'b', 0: 'c'}[0]", "c");
   }
@@ -741,19 +755,18 @@ public class MethodLibraryTest extends EvaluationTestCase {
 
   @Test
   public void testDictionaryCreationInvalidPositional() throws Exception {
-    String unexpectedString =
-        "expected value of type 'list(object)' for dict(args), but got \"a\" (string)";
-
     new BothModesTest()
         .testIfErrorContains(
-            "Method dict(args: Iterable, **kwargs) is not applicable for arguments "
-            + "(string, dict): 'args' is string, but should be Iterable",
+            "expected value of type 'list(object)' for parameter args in dict(), "
+            + "but got \"a\" (string)",
             "dict('a')")
-        .testIfErrorContains(unexpectedString, "dict(['a'])")
-        .testIfErrorContains(unexpectedString, "dict([('a')])")
+        .testIfErrorContains(
+            "Cannot convert dictionary update sequence element #0 to a sequence", "dict(['a'])")
+        .testIfErrorContains(
+            "Cannot convert dictionary update sequence element #0 to a sequence", "dict([('a')])")
         .testIfErrorContains("too many (3) positional arguments", "dict((3,4), (3,2), (1,2))")
         .testIfErrorContains(
-            "Tuple has length 3, but exactly two elements are required",
+            "Sequence #0 has length 3, but exactly two elements are required",
             "dict([('a', 'b', 'c')])");
   }