bazel syntax: remove Location parameters
Remove Location parameter from SkylarkIndexable.getIndex
(and thread parameter from SkylarkQueryable).
Also, make SkylarkIndexable and SkylarkQueryable
consistently take a StarlarkSemantics.
Eval: don't pass Locations down into evaluation;
obtain them only after an error, on the way up.
(This is a prerequisite for compilation, and for
finer-grained syntax locations. This CL addresses only
the easy cases.)
Also:
- Dict.get2: avoid duplicate hash lookup in success case.
- EvalUtils.index: pass mutability and semantics, not thread
- remove unnecessary parameters to binary operators.
- improvements to error messages
PiperOrigin-RevId: 289523719
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunctionTest.java
index 933956a..ee39c29 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/SingleToolchainResolutionFunctionTest.java
@@ -30,7 +30,6 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
-import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.Info;
import com.google.devtools.build.lib.packages.NativeProvider;
import com.google.devtools.build.lib.packages.Provider;
@@ -38,6 +37,7 @@
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
import com.google.devtools.build.lib.syntax.Printer;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.skyframe.EvaluationResult;
import com.google.devtools.build.skyframe.SkyKey;
import javax.annotation.Nullable;
@@ -263,12 +263,12 @@
public void repr(Printer printer) {}
@Override
- public Object getIndex(Object key, Location loc) {
+ public Object getIndex(StarlarkSemantics semantics, Object key) {
return null;
}
@Override
- public boolean containsKey(Object key, Location loc) {
+ public boolean containsKey(StarlarkSemantics semantics, Object key) {
return false;
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
index 34d8df9..3033f7e 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleImplementationFunctionsTest.java
@@ -969,7 +969,7 @@
public void testStructPlusArtifactErrorMessage() throws Exception {
setRuleContext(createRuleContext("//foo:foo"));
checkEvalErrorContains(
- "unsupported operand type(s) for +: 'File' and 'struct'",
+ "unsupported binary operation: File + struct",
"ruleContext.files.tools[0] + struct(a = 1)");
}
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 a5233db..236aea0 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
@@ -183,7 +183,7 @@
.testExpression("123 + 456", 579)
.testExpression("456 - 123", 333)
.testExpression("8 % 3", 2)
- .testIfErrorContains("unsupported operand type(s) for %: 'int' and 'string'", "3 % 'foo'")
+ .testIfErrorContains("unsupported binary operation: int % string", "3 % 'foo'")
.testExpression("-5", -5)
.testIfErrorContains("unsupported unary operation: -string", "-'foo'");
}
@@ -341,7 +341,7 @@
.testExpression("-7 // 2", -4)
.testExpression("-7 // -2", 3)
.testExpression("2147483647 // 2", 1073741823)
- .testIfErrorContains("unsupported operand type(s) for //: 'string' and 'int'", "'str' // 2")
+ .testIfErrorContains("unsupported binary operation: string // int", "'str' // 2")
.testIfExactError("integer division by zero", "5 // 0");
}
@@ -385,8 +385,7 @@
assertThat(x).isEqualTo(Tuple.of(1, 2, 3, 4));
assertThat(EvalUtils.isImmutable(x)).isTrue();
- checkEvalError("unsupported operand type(s) for +: 'tuple' and 'list'",
- "(1,2) + [3,4]"); // list + tuple
+ checkEvalError("unsupported binary operation: tuple + list", "(1,2) + [3,4]");
}
@Test
@@ -572,10 +571,8 @@
newTest()
.testExpression("[1, 2] + [3, 4]", StarlarkList.of(thread.mutability(), 1, 2, 3, 4))
.testExpression("(1, 2) + (3, 4)", Tuple.of(1, 2, 3, 4))
- .testIfExactError(
- "unsupported operand type(s) for +: 'list' and 'tuple'", "[1, 2] + (3, 4)")
- .testIfExactError(
- "unsupported operand type(s) for +: 'tuple' and 'list'", "(1, 2) + [3, 4]");
+ .testIfExactError("unsupported binary operation: list + tuple", "[1, 2] + (3, 4)")
+ .testIfExactError("unsupported binary operation: tuple + list", "(1, 2) + [3, 4]");
}
@Test
@@ -744,7 +741,8 @@
newTest()
.testIfErrorContains(
"'in <string>' requires string as left operand, not 'int'", "1 in '123'")
- .testIfErrorContains("'int' is not iterable. in operator only works on ", "'a' in 1");
+ .testIfErrorContains(
+ "unsupported binary operation: string in int (right operand must be", "'a' in 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 176f82e..39b226e 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
@@ -97,7 +97,7 @@
+ LINE_SEPARATOR
+ "\t\ts += \"2\""
+ LINE_SEPARATOR
- + "unsupported operand type(s) for +: 'int' and 'string'",
+ + "unsupported binary operation: int + string",
"def foo():",
" s = 1",
" s += '2'",
@@ -229,7 +229,7 @@
@Test
public void testBooleanUnsupportedOperationFails() throws Exception {
new BothModesTest()
- .testIfErrorContains("unsupported operand type(s) for +: 'bool' and 'bool'", "True + True");
+ .testIfErrorContains("unsupported binary operation: bool + bool", "True + True");
}
@Test
@@ -474,7 +474,7 @@
.testExpression("repr(a)", "range(0, 3)")
.testExpression("repr(range(1,2,3))", "range(1, 2, 3)")
.testExpression("type(a)", "range")
- .testIfErrorContains("unsupported operand type(s) for +: 'range' and 'range'", "a + a")
+ .testIfErrorContains("unsupported binary operation: range + range", "a + a")
.testIfErrorContains("'range' value has no field or method 'append'", "a.append(3)")
.testExpression("str(list(range(5)))", "[0, 1, 2, 3, 4]")
.testExpression("str(list(range(0)))", "[]")
@@ -504,7 +504,7 @@
.testExpression("str(range(0, 10, 2)[::-2])", "range(8, -2, -4)")
.testExpression("str(range(5)[1::-1])", "range(1, -1, -1)")
.testIfErrorContains("step cannot be 0", "range(2, 3, 0)")
- .testIfErrorContains("unsupported operand type(s) for *: 'range' and 'int'", "range(3) * 3")
+ .testIfErrorContains("unsupported binary operation: range * int", "range(3) * 3")
.testIfErrorContains("Cannot compare range objects", "range(3) < range(5)")
.testIfErrorContains("Cannot compare range objects", "range(4) > [1]")
.testExpression("4 in range(1, 10)", true)
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 10f8408..6beb2fd 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
@@ -1468,7 +1468,7 @@
new SkylarkTest("--incompatible_depset_union=false")
.testExpression("str(depset([1, 3]) | depset([1, 2]))", "depset([1, 2, 3])")
.testExpression("str(depset([1, 2]) | [1, 3])", "depset([1, 2, 3])")
- .testIfExactError("unsupported operand type(s) for |: 'int' and 'bool'", "2 | False");
+ .testIfExactError("unsupported binary operation: int | bool", "2 | False");
}
@Test
@@ -1476,7 +1476,8 @@
new SkylarkTest()
.testIfErrorContains("not iterable", "list(depset(['a', 'b']))")
.testIfErrorContains("not iterable", "max(depset([1, 2, 3]))")
- .testIfErrorContains("not iterable", "1 in depset([1, 2, 3])")
+ .testIfErrorContains(
+ "unsupported binary operation: int in depset", "1 in depset([1, 2, 3])")
.testIfErrorContains("not iterable", "sorted(depset(['a', 'b']))")
.testIfErrorContains("not iterable", "tuple(depset(['a', 'b']))")
.testIfErrorContains("not iterable", "[x for x in depset()]")
@@ -1648,11 +1649,10 @@
@Test
public void testPlusOnDictDeprecated() throws Exception {
new SkylarkTest()
- .testIfErrorContains(
- "unsupported operand type(s) for +: 'dict' and 'dict'", "{1: 2} + {3: 4}");
+ .testIfErrorContains("unsupported binary operation: dict + dict", "{1: 2} + {3: 4}");
new SkylarkTest()
.testIfErrorContains(
- "unsupported operand type(s) for +: 'dict' and 'dict'",
+ "unsupported binary operation: dict + dict",
"def func():",
" d = {1: 2}",
" d += {3: 4}",
@@ -1857,8 +1857,8 @@
@Test
public void testListAnTupleConcatenationDoesNotWorkInSkylark() throws Exception {
- new SkylarkTest().testIfExactError("unsupported operand type(s) for +: 'list' and 'tuple'",
- "[1, 2] + (3, 4)");
+ new SkylarkTest()
+ .testIfExactError("unsupported binary operation: list + tuple", "[1, 2] + (3, 4)");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/StarlarkFileTest.java b/src/test/java/com/google/devtools/build/lib/syntax/StarlarkFileTest.java
index db95ec2..4c2a991 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/StarlarkFileTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/StarlarkFileTest.java
@@ -69,7 +69,7 @@
EvalUtils.exec(file, thread);
throw new AssertionError("execution succeeded unexpectedly");
} catch (EvalException ex) {
- assertThat(ex.getMessage()).contains("unsupported operand type(s) for +: 'int' and 'list'");
+ assertThat(ex.getMessage()).contains("unsupported binary operation: int + list");
assertThat(ex.getLocation().line()).isEqualTo(4);
}
}
