Rollback of commit 984d6d48d0e07ac3be2bbfec667158165390eb4f.
--
MOS_MIGRATED_REVID=140121290
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
index 2785b64..c83ed98 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
@@ -72,9 +72,6 @@
o1 = SkylarkType.convertToSkylark(o1, /*env=*/ null);
o2 = SkylarkType.convertToSkylark(o2, /*env=*/ null);
- if (o1 instanceof ClassObject && o2 instanceof ClassObject) {
- throw new ComparisonException("Cannot compare structs");
- }
if (o1 instanceof SkylarkNestedSet && o2 instanceof SkylarkNestedSet) {
throw new ComparisonException("Cannot compare sets");
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 7ca5793..5748f63 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -801,7 +801,7 @@
ImmutableList.Builder<Object> posargs = new ImmutableList.Builder<>();
// We copy this into an ImmutableMap in the end, but we can't use an ImmutableMap.Builder, or
// we'd still have to have a HashMap on the side for the sake of properly handling duplicates.
- Map<String, Object> kwargs = new LinkedHashMap<>();
+ Map<String, Object> kwargs = new HashMap<>();
BaseFunction function = checkCallable(funcValue, getLocation());
evalArguments(posargs, kwargs, env);
return function.call(posargs.build(), ImmutableMap.copyOf(kwargs), this, env);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java
index cf371af..6ae2ae5 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkDict.java
@@ -18,8 +18,8 @@
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
import com.google.devtools.build.lib.syntax.SkylarkMutable.MutableMap;
-import java.util.LinkedHashMap;
import java.util.Map;
+import java.util.TreeMap;
import javax.annotation.Nullable;
/**
@@ -46,7 +46,7 @@
public final class SkylarkDict<K, V>
extends MutableMap<K, V> implements Map<K, V>, SkylarkIndexable {
- private final LinkedHashMap<K, V> contents = new LinkedHashMap<>();
+ private final TreeMap<K, V> contents = new TreeMap<>(EvalUtils.SKYLARK_COMPARATOR);
private final Mutability mutability;
@@ -139,7 +139,7 @@
/** @return the first key in the dict */
K firstKey() {
- return contents.entrySet().iterator().next().getKey();
+ return contents.firstKey();
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index b5cb1f6..17f5484 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -810,14 +810,6 @@
}
@Test
- public void testStructIncomparability() throws Exception {
- checkErrorContains("Cannot compare structs", "struct(a = 1) < struct(a = 2)");
- checkErrorContains("Cannot compare structs", "struct(a = 1) > struct(a = 2)");
- checkErrorContains("Cannot compare structs", "struct(a = 1) <= struct(a = 2)");
- checkErrorContains("Cannot compare structs", "struct(a = 1) >= struct(a = 2)");
- }
-
- @Test
public void testStructAccessingFieldsFromSkylark() throws Exception {
eval("x = struct(a = 1, b = 2)", "x1 = x.a", "x2 = x.b");
assertThat(lookup("x1")).isEqualTo(1);
@@ -942,17 +934,6 @@
}
@Test
- public void testStructsInDicts() throws Exception {
- eval("d = {struct(a = 1): 'aa', struct(b = 2): 'bb'}");
- assertThat(eval("d[struct(a = 1)]")).isEqualTo("aa");
- assertThat(eval("d[struct(b = 2)]")).isEqualTo("bb");
-
- checkErrorContains(
- "unhashable type: 'struct'",
- "{struct(a = []): 'foo'}");
- }
-
- @Test
public void testStructMembersAreImmutable() throws Exception {
checkErrorContains(
"cannot assign to 's.x'",
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
index 75e42eb..fa72014 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/EvaluationTest.java
@@ -272,10 +272,7 @@
.update(kwargs.getName(), kwargs)
.testEval(
"kwargs(foo=1, bar='bar', wiz=[1,2,3]).items()",
- "[('foo', 1), ('bar', 'bar'), ('wiz', [1, 2, 3])]")
- .testEval(
- "kwargs(wiz=[1,2,3], bar='bar', foo=1).items()",
- "[('wiz', [1, 2, 3]), ('bar', 'bar'), ('foo', 1)]");
+ "[('bar', 'bar'), ('foo', 1), ('wiz', [1, 2, 3])]");
}
@Test
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 d416c30..ea81adb 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
@@ -1311,8 +1311,8 @@
new BothModesTest()
.testEval("{1: 'foo'}.values()", "['foo']")
.testEval("{}.values()", "[]")
- .testEval("{True: 3, False: 5}.values()", "[3, 5]")
- .testEval("{'a': 5, 'c': 2, 'b': 4, 'd': 3}.values()", "[5, 2, 4, 3]");
+ .testEval("{True: 3, False: 5}.values()", "[5, 3]")
+ .testEval("{'a': 5, 'c': 2, 'b': 4, 'd': 3}.values()", "[5, 4, 2, 3]");
// sorted by keys
}
@@ -1342,7 +1342,7 @@
.testEval("{'a': 'foo'}.items()", "[('a', 'foo')]")
.testEval("{}.items()", "[]")
.testEval("{1: 3, 2: 5}.items()", "[(1, 3), (2, 5)]")
- .testEval("{'a': 5, 'c': 2, 'b': 4}.items()", "[('a', 5), ('c', 2), ('b', 4)]");
+ .testEval("{'a': 5, 'c': 2, 'b': 4}.items()", "[('a', 5), ('b', 4), ('c', 2)]");
}
@Test
@@ -1378,9 +1378,9 @@
"popitem(): dictionary is empty",
"d = {2: 'bar', 3: 'baz', 1: 'foo'}\n"
+ "if len(d) != 3: fail('popitem 0')\n"
+ + "if d.popitem() != (1, 'foo'): fail('popitem 1')\n"
+ "if d.popitem() != (2, 'bar'): fail('popitem 2')\n"
+ "if d.popitem() != (3, 'baz'): fail('popitem 3')\n"
- + "if d.popitem() != (1, 'foo'): fail('popitem 1')\n"
+ "if d != {}: fail('popitem 4')\n"
+ "d.popitem()");
}
