Allow dicts to contain non-comparable objects as keys
RELNOTES: Skylark dicts internally don't rely on keys order anymore and accept
any hashable values (i.e. structs with immutable values) as keys. Iteration order of dictionaries is no longer specified.
--
PiperOrigin-RevId: 141055080
MOS_MIGRATED_REVID=141055080
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 17f5484..066c544 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
@@ -657,6 +657,11 @@
}
@Test
+ public void testProtoFieldsOrder() throws Exception {
+ checkTextMessage("struct(d=4, b=2, c=3, a=1).to_proto()", "a: 1", "b: 2", "c: 3", "d: 4");
+ }
+
+ @Test
public void testTextMessageEscapes() throws Exception {
checkTextMessage("struct(name='a\"b').to_proto()", "name: \"a\\\"b\"");
checkTextMessage("struct(name='a\\'b').to_proto()", "name: \"a'b\"");
@@ -810,6 +815,14 @@
}
@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);
@@ -934,6 +947,18 @@
}
@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");
+ assertThat(eval("str([d[k] for k in d])")).isEqualTo("[\"aa\", \"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/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
index b60dace..a36fc74 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java
@@ -547,27 +547,27 @@
assertEquals(
SkylarkList.createImmutable(
ImmutableList.<String>of(
- "cmd",
- "compatible_with",
- "executable",
- "features",
+ "name",
+ "visibility",
+ "tags",
+ "generator_name",
"generator_function",
"generator_location",
- "generator_name",
- "heuristic_label_expansion",
- "kind",
- "local",
- "message",
- "name",
- "output_to_bindir",
- "outs",
+ "features",
+ "compatible_with",
"restricted_to",
"srcs",
- "stamp",
- "tags",
- "toolchains",
"tools",
- "visibility")),
+ "toolchains",
+ "outs",
+ "cmd",
+ "output_to_bindir",
+ "local",
+ "message",
+ "executable",
+ "stamp",
+ "heuristic_label_expansion",
+ "kind")),
result);
}
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 fa72014..9bdaa15 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
@@ -146,7 +146,11 @@
.testStatement("'hello' == 'bye'", false)
.testStatement("None == None", true)
.testStatement("[1, 2] == [1, 2]", true)
- .testStatement("[1, 2] == [2, 1]", false);
+ .testStatement("[1, 2] == [2, 1]", false)
+ .testStatement("{'a': 1, 'b': 2} == {'b': 2, 'a': 1}", true)
+ .testStatement("{'a': 1, 'b': 2} == {'a': 1}", false)
+ .testStatement("{'a': 1, 'b': 2} == {'a': 1, 'b': 2, 'c': 3}", false)
+ .testStatement("{'a': 1, 'b': 2} == {'a': 1, 'b': 3}", false);
}
@Test
@@ -157,7 +161,11 @@
.testStatement("'hello' != 'hel' + 'lo'", false)
.testStatement("'hello' != 'bye'", true)
.testStatement("[1, 2] != [1, 2]", false)
- .testStatement("[1, 2] != [2, 1]", true);
+ .testStatement("[1, 2] != [2, 1]", true)
+ .testStatement("{'a': 1, 'b': 2} != {'b': 2, 'a': 1}", false)
+ .testStatement("{'a': 1, 'b': 2} != {'a': 1}", true)
+ .testStatement("{'a': 1, 'b': 2} != {'a': 1, 'b': 2, 'c': 3}", true)
+ .testStatement("{'a': 1, 'b': 2} != {'a': 1, 'b': 3}", true);
}
@Test
@@ -272,7 +280,10 @@
.update(kwargs.getName(), kwargs)
.testEval(
"kwargs(foo=1, bar='bar', wiz=[1,2,3]).items()",
- "[('bar', 'bar'), ('foo', 1), ('wiz', [1, 2, 3])]");
+ "[('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)]");
}
@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 ea81adb..a001f3f 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()", "[5, 3]")
- .testEval("{'a': 5, 'c': 2, 'b': 4, 'd': 3}.values()", "[5, 4, 2, 3]");
+ .testEval("{True: 3, False: 5}.values()", "[3, 5]")
+ .testEval("{'a': 5, 'c': 2, 'b': 4, 'd': 3}.values()", "[5, 2, 4, 3]");
// sorted by keys
}
@@ -1321,9 +1321,9 @@
new BothModesTest()
.testEval("{1: 'foo'}.keys()", "[1]")
.testEval("{}.keys()", "[]")
- .testEval("{True: 3, False: 5}.keys()", "[False, True]")
+ .testEval("{True: 3, False: 5}.keys()", "[True, False]")
.testEval(
- "{1:'a', 2:'b', 6:'c', 0:'d', 5:'e', 4:'f', 3:'g'}.keys()", "[0, 1, 2, 3, 4, 5, 6]");
+ "{1:'a', 2:'b', 6:'c', 0:'d', 5:'e', 4:'f', 3:'g'}.keys()", "[1, 2, 6, 0, 5, 4, 3]");
}
@Test
@@ -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), ('b', 4), ('c', 2)]");
+ .testEval("{'a': 5, 'c': 2, 'b': 4}.items()", "[('a', 5), ('c', 2), ('b', 4)]");
}
@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()");
}
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
index dcb927f..e949290 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkEvaluationTest.java
@@ -919,7 +919,7 @@
" for a in d:",
" s += a",
" return s",
- "s = foo()").testLookup("s", "abc");
+ "s = foo()").testLookup("s", "cab");
}
@Test
