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/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
index c226c3a..a8c31f7 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/OutputGroupInfo.java
@@ -35,9 +35,10 @@
import com.google.devtools.build.lib.syntax.Depset;
import com.google.devtools.build.lib.syntax.Dict;
import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.SkylarkIndexable;
+import com.google.devtools.build.lib.syntax.Starlark;
import com.google.devtools.build.lib.syntax.StarlarkIterable;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
@@ -240,25 +241,22 @@
}
@Override
- public Object getIndex(Object key, Location loc) throws EvalException {
+ public Object getIndex(StarlarkSemantics semantics, Object key) throws EvalException {
if (!(key instanceof String)) {
- throw new EvalException(loc, String.format(
- "Output grout names must be strings, got %s instead",
- EvalUtils.getDataTypeName(key)));
+ throw Starlark.errorf(
+ "Output group names must be strings, got %s instead", Starlark.type(key));
}
NestedSet<Artifact> result = outputGroups.get(key);
if (result != null) {
return Depset.of(Artifact.TYPE, result);
} else {
- throw new EvalException(loc, String.format(
- "Output group %s not present", key
- ));
+ throw Starlark.errorf("Output group %s not present", key);
}
}
@Override
- public boolean containsKey(Object key, Location loc) throws EvalException {
+ public boolean containsKey(StarlarkSemantics semantics, Object key) throws EvalException {
return outputGroups.containsKey(key);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ResolvedToolchainContext.java b/src/main/java/com/google/devtools/build/lib/analysis/ResolvedToolchainContext.java
index cf78c07..bf397b5 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ResolvedToolchainContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ResolvedToolchainContext.java
@@ -27,15 +27,15 @@
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
-import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.rules.AliasConfiguredTarget;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
import com.google.devtools.build.lib.skyframe.ToolchainException;
import com.google.devtools.build.lib.skyframe.UnloadedToolchainContext;
import com.google.devtools.build.lib.skylarkbuildapi.platform.ToolchainContextApi;
import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.Printer;
+import com.google.devtools.build.lib.syntax.Starlark;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import javax.annotation.Nullable;
/**
@@ -157,8 +157,7 @@
}
private static Label transformKey(
- Object key, Location loc, ImmutableMap<RepositoryName, RepositoryName> repoMapping)
- throws EvalException {
+ Object key, ImmutableMap<RepositoryName, RepositoryName> repoMapping) throws EvalException {
if (key instanceof Label) {
return (Label) key;
} else if (key instanceof ToolchainTypeInfo) {
@@ -167,43 +166,38 @@
try {
return Label.parseAbsolute((String) key, repoMapping);
} catch (LabelSyntaxException e) {
- throw new EvalException(
- loc, String.format("Unable to parse toolchain %s: %s", key, e.getMessage()), e);
+ throw Starlark.errorf("Unable to parse toolchain %s: %s", key, e.getMessage());
}
} else {
- throw new EvalException(
- loc,
- String.format(
- "Toolchains only supports indexing by toolchain type, got %s instead",
- EvalUtils.getDataTypeName(key)));
+ throw Starlark.errorf(
+ "Toolchains only supports indexing by toolchain type, got %s instead",
+ Starlark.type(key));
}
}
@Override
- public ToolchainInfo getIndex(Object key, Location loc) throws EvalException {
- Label toolchainTypeLabel = transformKey(key, loc, repoMapping());
+ public ToolchainInfo getIndex(StarlarkSemantics semantics, Object key) throws EvalException {
+ Label toolchainTypeLabel = transformKey(key, repoMapping());
- if (!containsKey(key, loc)) {
+ if (!containsKey(semantics, key)) {
// TODO(bazel-configurability): The list of available toolchain types is confusing in the
// presence of aliases, since it only contains the actual label, not the alias passed to the
// rule definition.
- throw new EvalException(
- loc,
- String.format(
- "In %s, toolchain type %s was requested but only types [%s] are configured",
- targetDescription(),
- toolchainTypeLabel,
- requiredToolchainTypes().stream()
- .map(ToolchainTypeInfo::typeLabel)
- .map(Label::toString)
- .collect(joining(", "))));
+ throw Starlark.errorf(
+ "In %s, toolchain type %s was requested but only types [%s] are configured",
+ targetDescription(),
+ toolchainTypeLabel,
+ requiredToolchainTypes().stream()
+ .map(ToolchainTypeInfo::typeLabel)
+ .map(Label::toString)
+ .collect(joining(", ")));
}
return forToolchainType(toolchainTypeLabel);
}
@Override
- public boolean containsKey(Object key, Location loc) throws EvalException {
- Label toolchainTypeLabel = transformKey(key, loc, repoMapping());
+ public boolean containsKey(StarlarkSemantics semantics, Object key) throws EvalException {
+ Label toolchainTypeLabel = transformKey(key, repoMapping());
return requestedToolchainTypeLabels().containsKey(toolchainTypeLabel);
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java
index d92dd03..c1f9177 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/AbstractConfiguredTarget.java
@@ -30,13 +30,11 @@
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
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.PackageSpecification.PackageGroupContents;
import com.google.devtools.build.lib.packages.Provider;
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.Printer;
import com.google.devtools.build.lib.syntax.SkylarkClassObject;
import com.google.devtools.build.lib.syntax.Starlark;
@@ -147,32 +145,30 @@
}
@Override
- public final Object getIndex(Object key, Location loc) throws EvalException {
+ public final Object getIndex(StarlarkSemantics semantics, Object key) throws EvalException {
if (!(key instanceof Provider)) {
- throw new EvalException(loc, String.format(
+ throw Starlark.errorf(
"Type Target only supports indexing by object constructors, got %s instead",
- EvalUtils.getDataTypeName(key)));
+ Starlark.type(key));
}
Provider constructor = (Provider) key;
Object declaredProvider = get(constructor.getKey());
if (declaredProvider != null) {
return declaredProvider;
}
- throw new EvalException(
- loc,
- Starlark.format(
- "%r%s doesn't contain declared provider '%s'",
- this,
- getRuleClassString().isEmpty() ? "" : " (rule '" + getRuleClassString() + "')",
- constructor.getPrintableName()));
+ throw Starlark.errorf(
+ "%s%s doesn't contain declared provider '%s'",
+ Starlark.repr(this),
+ getRuleClassString().isEmpty() ? "" : " (rule '" + getRuleClassString() + "')",
+ constructor.getPrintableName());
}
@Override
- public boolean containsKey(Object key, Location loc) throws EvalException {
+ public boolean containsKey(StarlarkSemantics semantics, Object key) throws EvalException {
if (!(key instanceof Provider)) {
- throw new EvalException(loc, String.format(
+ throw Starlark.errorf(
"Type Target only supports querying by object constructors, got %s instead",
- EvalUtils.getDataTypeName(key)));
+ Starlark.type(key));
}
return get(((Provider) key).getKey()) != null;
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintCollection.java b/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintCollection.java
index 4e40cd5..bcc1c75 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintCollection.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/platform/ConstraintCollection.java
@@ -29,7 +29,6 @@
import com.google.common.collect.Streams;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization;
import com.google.devtools.build.lib.skylarkbuildapi.platform.ConstraintCollectionApi;
@@ -39,6 +38,7 @@
import com.google.devtools.build.lib.syntax.Sequence;
import com.google.devtools.build.lib.syntax.Starlark;
import com.google.devtools.build.lib.syntax.StarlarkList;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.util.Fingerprint;
import java.util.Collection;
import java.util.Map;
@@ -168,13 +168,11 @@
return mismatchSettings.build();
}
- private ConstraintSettingInfo convertKey(Object key, Location loc) throws EvalException {
+ private static ConstraintSettingInfo convertKey(Object key) throws EvalException {
if (!(key instanceof ConstraintSettingInfo)) {
- throw new EvalException(
- loc,
- String.format(
- "Constraint names must be platform_common.ConstraintSettingInfo, got %s instead",
- EvalUtils.getDataTypeName(key)));
+ throw Starlark.errorf(
+ "Constraint names must be platform_common.ConstraintSettingInfo, got %s instead",
+ EvalUtils.getDataTypeName(key));
}
return (ConstraintSettingInfo) key;
@@ -242,15 +240,13 @@
}
@Override
- public Object getIndex(Object key, Location loc) throws EvalException {
- ConstraintSettingInfo constraint = convertKey(key, loc);
- return get(constraint);
+ public Object getIndex(StarlarkSemantics semantics, Object key) throws EvalException {
+ return get(convertKey(key));
}
@Override
- public boolean containsKey(Object key, Location loc) throws EvalException {
- ConstraintSettingInfo constraint = convertKey(key, loc);
- return has(constraint);
+ public boolean containsKey(StarlarkSemantics semantics, Object key) throws EvalException {
+ return has(convertKey(key));
}
// It's easier to use the Starlark repr as a string form, not what AutoValue produces.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java
index 43996de..bfced9a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/AliasConfiguredTarget.java
@@ -24,7 +24,6 @@
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
-import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.Info;
import com.google.devtools.build.lib.packages.Provider;
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
@@ -34,6 +33,7 @@
import com.google.devtools.build.lib.syntax.Depset;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Printer;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import javax.annotation.Nullable;
/**
@@ -109,13 +109,13 @@
}
@Override
- public Object getIndex(Object key, Location loc) throws EvalException {
- return actual.getIndex(key, loc);
+ public Object getIndex(StarlarkSemantics semantics, Object key) throws EvalException {
+ return actual.getIndex(semantics, key);
}
@Override
- public boolean containsKey(Object key, Location loc) throws EvalException {
- return actual.containsKey(key, loc);
+ public boolean containsKey(StarlarkSemantics semantics, Object key) throws EvalException {
+ return actual.containsKey(semantics, key);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Dict.java b/src/main/java/com/google/devtools/build/lib/syntax/Dict.java
index b8db285..42e341e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Dict.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Dict.java
@@ -152,9 +152,17 @@
// SkylarkInterfaceUtils.getSkylarkCallable. The two 'get' methods cause it to get
// confused as to which one has the annotation. Fix it and remove "2" suffix.
public Object get2(Object key, Object defaultValue, StarlarkThread thread) throws EvalException {
- if (containsKey(key, null, thread)) {
- return this.get(key);
+ Object v = this.get(key);
+ if (v != null) {
+ return v;
}
+
+ // This statement is executed for its effect, which is to throw "unhashable"
+ // if key is unhashable, instead of returning defaultValue.
+ // I think this is a bug: the correct behavior is simply 'return defaultValue'.
+ // See https://github.com/bazelbuild/starlark/issues/65.
+ containsKey(thread.getSemantics(), key);
+
return defaultValue;
}
@@ -508,21 +516,16 @@
}
@Override
- public final Object getIndex(Object key, Location loc) throws EvalException {
- if (!this.containsKey(key)) {
- throw new EvalException(loc, Starlark.format("key %r not found in dictionary", key));
+ public Object getIndex(StarlarkSemantics semantics, Object key) throws EvalException {
+ Object v = get(key);
+ if (v == null) {
+ throw Starlark.errorf("key %s not found in dictionary", Starlark.repr(key));
}
- return this.get(key);
+ return v;
}
@Override
- public final boolean containsKey(Object key, Location loc) throws EvalException {
- return this.containsKey(key);
- }
-
- @Override
- public final boolean containsKey(Object key, Location loc, StarlarkThread thread)
- throws EvalException {
+ public boolean containsKey(StarlarkSemantics semantics, Object key) throws EvalException {
EvalUtils.checkHashable(key);
return this.containsKey(key);
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Eval.java b/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
index 8b8900d..fbe4ebb 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
@@ -98,8 +98,12 @@
execAugmentedAssignment(node);
} else {
Object rvalue = eval(thread, node.getRHS());
- // TODO(adonovan): use location of = operator.
- assign(node.getLHS(), rvalue, thread, node.getStartLocation());
+ try {
+ assign(node.getLHS(), rvalue, thread);
+ } catch (EvalException ex) {
+ // TODO(adonovan): use location of = operator.
+ throw ex.ensureLocation(node.getStartLocation());
+ }
}
}
@@ -109,7 +113,7 @@
EvalUtils.lock(o, node.getStartLocation());
try {
for (Object it : seq) {
- assign(node.getLHS(), it, thread, node.getLHS().getStartLocation());
+ assign(node.getLHS(), it, thread);
switch (execStatements(node.getBlock(), /*indented=*/ true)) {
case PASS:
@@ -127,6 +131,8 @@
throw new IllegalStateException("unreachable");
}
}
+ } catch (EvalException ex) {
+ throw ex.ensureLocation(node.getLHS().getStartLocation());
} finally {
EvalUtils.unlock(o, node.getStartLocation());
}
@@ -269,22 +275,22 @@
/**
* Updates the environment bindings, and possibly mutates objects, so as to assign the given value
- * to the given expression. The expression must be valid for an {@code LValue}.
+ * to the given expression. May throw an EvalException without location.
*/
- private void assign(Expression expr, Object value, StarlarkThread thread, Location loc)
+ private void assign(Expression expr, Object value, StarlarkThread thread)
throws EvalException, InterruptedException {
if (expr instanceof Identifier) {
assignIdentifier((Identifier) expr, value, thread);
} else if (expr instanceof IndexExpression) {
Object object = eval(thread, ((IndexExpression) expr).getObject());
Object key = eval(thread, ((IndexExpression) expr).getKey());
- assignItem(object, key, value, loc);
+ assignItem(object, key, value);
} else if (expr instanceof ListExpression) {
ListExpression list = (ListExpression) expr;
- assignList(list.getElements(), value, thread, loc);
+ assignList(list.getElements(), value, thread);
} else {
// Not possible for validated ASTs.
- throw new EvalException(loc, "cannot assign to '" + expr + "'");
+ throw Starlark.errorf("cannot assign to '%s'", expr);
}
}
@@ -325,58 +331,52 @@
*
* @throws EvalException if the object is not a list or dict
*/
- @SuppressWarnings("unchecked")
- private void assignItem(Object object, Object key, Object value, Location loc)
- throws EvalException {
+ private static void assignItem(Object object, Object key, Object value) throws EvalException {
if (object instanceof Dict) {
+ @SuppressWarnings("unchecked")
Dict<Object, Object> dict = (Dict<Object, Object>) object;
- dict.put(key, value, loc);
+ dict.put(key, value, /*loc=*/ null);
} else if (object instanceof StarlarkList) {
+ @SuppressWarnings("unchecked")
StarlarkList<Object> list = (StarlarkList<Object>) object;
int index = Starlark.toInt(key, "list index");
- index = EvalUtils.getSequenceIndex(index, list.size(), loc);
- list.set(index, value, loc);
+ index = EvalUtils.getSequenceIndex(index, list.size());
+ list.set(index, value, /*loc=*/ null);
} else {
- throw new EvalException(
- loc,
- "can only assign an element in a dictionary or a list, not in a '"
- + EvalUtils.getDataTypeName(object)
- + "'");
+ throw Starlark.errorf(
+ "can only assign an element in a dictionary or a list, not in a '%s'",
+ Starlark.type(object));
}
}
/**
- * Recursively assigns an iterable value to a sequence of assignable expressions.
- *
- * @throws EvalException if the list literal has length 0, or if the value is not an iterable of
- * matching length
+ * Recursively assigns an iterable value to a sequence of assignable expressions. May throw an
+ * EvalException without location.
*/
- private void assignList(List<Expression> lhs, Object x, StarlarkThread thread, Location loc)
+ private void assignList(List<Expression> lhs, Object x, StarlarkThread thread)
throws EvalException, InterruptedException {
// TODO(adonovan): lock/unlock rhs during iteration so that
// assignments fail when the left side aliases the right,
// which is a tricky case in Python assignment semantics.
int nrhs = Starlark.len(x);
if (nrhs < 0) {
- throw new EvalException(loc, "type '" + EvalUtils.getDataTypeName(x) + "' is not iterable");
+ throw Starlark.errorf("type '%s' is not iterable", Starlark.type(x));
}
Iterable<?> rhs = Starlark.toIterable(x); // fails if x is a string
int len = lhs.size();
if (len == 0) {
- throw new EvalException(
- loc, "lists or tuples on the left-hand side of assignments must have at least one item");
+ throw Starlark.errorf(
+ "lists or tuples on the left-hand side of assignments must have at least one item");
}
if (len != nrhs) {
- throw new EvalException(
- loc,
- String.format(
- "assignment length mismatch: left-hand side has length %d, but right-hand side"
- + " evaluates to value of length %d",
- len, nrhs));
+ throw Starlark.errorf(
+ "assignment length mismatch: left-hand side has length %d, but right-hand side evaluates"
+ + " to value of length %d",
+ len, nrhs);
}
int i = 0;
for (Object item : rhs) {
- assign(lhs.get(i), item, thread, loc);
+ assign(lhs.get(i), item, thread);
i++;
}
}
@@ -386,6 +386,8 @@
Expression lhs = stmt.getLHS();
TokenKind op = stmt.getOperator();
Expression rhs = stmt.getRHS();
+ // TODO(adonovan): don't materialize Locations before an error has occurred.
+ // (Requires syntax tree to record offsets and defer Location conversion.)
Location loc = stmt.getStartLocation(); // TODO(adonovan): use operator location
if (lhs instanceof Identifier) {
@@ -399,11 +401,15 @@
IndexExpression index = (IndexExpression) lhs;
Object object = eval(thread, index.getObject());
Object key = eval(thread, index.getKey());
- Object x = EvalUtils.index(object, key, thread, loc);
+ Object x = EvalUtils.index(thread.mutability(), thread.getSemantics(), object, key);
// Evaluate rhs after lhs.
Object y = eval(thread, rhs);
Object z = inplaceBinaryOp(op, x, y, thread, loc);
- assignItem(object, key, z, loc);
+ try {
+ assignItem(object, key, z);
+ } catch (EvalException ex) {
+ throw ex.ensureLocation(loc);
+ }
} else if (lhs instanceof ListExpression) {
throw new EvalException(loc, "cannot perform augmented assignment on a list literal");
} else {
@@ -414,7 +420,7 @@
private static Object inplaceBinaryOp(
TokenKind op, Object x, Object y, StarlarkThread thread, Location location)
- throws EvalException, InterruptedException {
+ throws EvalException {
// list += iterable behaves like list.extend(iterable)
// TODO(b/141263526): following Python, allow list+=iterable (but not list+iterable).
if (op == TokenKind.PLUS && x instanceof StarlarkList && y instanceof StarlarkList) {
@@ -507,11 +513,17 @@
Object k = eval(thread, entry.getKey());
Object v = eval(thread, entry.getValue());
int before = dict.size();
- Location loc = entry.getKey().getStartLocation(); // TODO(adonovan): use colon location
- dict.put(k, v, loc);
+ try {
+ dict.put(k, v, /*loc=*/ null);
+ } catch (EvalException ex) {
+ // TODO(adonovan): use colon location
+ throw ex.ensureLocation(entry.getKey().getStartLocation());
+ }
if (dict.size() == before) {
+ // TODO(adonovan): use colon location
throw new EvalException(
- loc, "Duplicated key " + Starlark.repr(k) + " when creating dictionary");
+ entry.getKey().getStartLocation(),
+ "Duplicated key " + Starlark.repr(k) + " when creating dictionary");
}
}
return dict;
@@ -582,7 +594,7 @@
if (!(value instanceof StarlarkIterable)) {
throw new EvalException(
star.getStartLocation(),
- "argument after * must be an iterable, not " + EvalUtils.getDataTypeName(value));
+ "argument after * must be an iterable, not " + Starlark.type(value));
}
// TODO(adonovan): opt: if value.size is known, preallocate (and skip if empty).
ArrayList<Object> list = new ArrayList<>();
@@ -597,7 +609,7 @@
if (!(value instanceof Dict)) {
throw new EvalException(
starstar.getStartLocation(),
- "argument after ** must be a dict, not " + EvalUtils.getDataTypeName(value));
+ "argument after ** must be a dict, not " + Starlark.type(value));
}
Dict<?, ?> kwargs = (Dict<?, ?>) value;
int j = named.length;
@@ -606,7 +618,7 @@
if (!(e.getKey() instanceof String)) {
throw new EvalException(
starstar.getStartLocation(),
- "keywords must be strings, not " + EvalUtils.getDataTypeName(e.getKey()));
+ "keywords must be strings, not " + Starlark.type(e.getKey()));
}
named[j++] = e.getKey();
named[j++] = e.getValue();
@@ -673,8 +685,12 @@
IndexExpression index = (IndexExpression) expr;
Object object = eval(thread, index.getObject());
Object key = eval(thread, index.getKey());
- // TODO(adonovan): use location of lbracket token
- return EvalUtils.index(object, key, thread, index.getStartLocation());
+ try {
+ return EvalUtils.index(thread.mutability(), thread.getSemantics(), object, key);
+ } catch (EvalException ex) {
+ // TODO(adonovan): use location of lbracket token
+ throw ex.ensureLocation(index.getStartLocation());
+ }
}
case INTEGER_LITERAL:
@@ -713,7 +729,11 @@
{
UnaryOperatorExpression unop = (UnaryOperatorExpression) expr;
Object x = eval(thread, unop.getX());
- return EvalUtils.unaryOp(unop.getOperator(), x, unop.getStartLocation());
+ try {
+ return EvalUtils.unaryOp(unop.getOperator(), x);
+ } catch (EvalException ex) {
+ throw ex.ensureLocation(unop.getStartLocation());
+ }
}
}
throw new IllegalArgumentException("unexpected expression: " + expr.kind());
@@ -758,12 +778,15 @@
Object iterable = eval(thread, forClause.getIterable());
Location loc = comp.getStartLocation(); // TODO(adonovan): use location of 'for' token
Iterable<?> listValue = Starlark.toIterable(iterable);
+ // TODO(adonovan): lock should not need loc.
EvalUtils.lock(iterable, loc);
try {
for (Object elem : listValue) {
- assign(forClause.getVars(), elem, thread, loc);
+ assign(forClause.getVars(), elem, thread);
execClauses(index + 1);
}
+ } catch (EvalException ex) {
+ throw ex.ensureLocation(loc);
} finally {
EvalUtils.unlock(iterable, loc);
}
@@ -783,7 +806,12 @@
Object k = eval(thread, body.getKey());
EvalUtils.checkHashable(k);
Object v = eval(thread, body.getValue());
- dict.put(k, v, comp.getStartLocation()); // TODO(adonovan): use colon location
+ try {
+ dict.put(k, v, /*loc=*/ null);
+ } catch (EvalException ex) {
+ // TODO(adonovan): use colon location
+ throw ex.ensureLocation(comp.getStartLocation());
+ }
} else {
list.add(eval(thread, ((Expression) comp.getBody())));
}
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 825110e..4137cb6 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
@@ -101,7 +101,7 @@
return ((Comparable<Object>) o1).compareTo(o2);
} catch (ClassCastException e) {
throw new ComparisonException(
- "Cannot compare " + getDataTypeName(o1) + " with " + getDataTypeName(o2));
+ "Cannot compare " + Starlark.type(o1) + " with " + Starlark.type(o2));
}
}
};
@@ -116,7 +116,7 @@
// Object.hashCode within a try/catch, and requiring all
// unhashable Starlark values to throw a particular unchecked exception
// with a helpful error message.
- throw Starlark.errorf("unhashable type: '%s'", EvalUtils.getDataTypeName(x));
+ throw Starlark.errorf("unhashable type: '%s'", Starlark.type(x));
}
}
@@ -288,15 +288,14 @@
* Resolves a positive or negative index to an index in the range [0, length), or throws
* EvalException if it is out of range. If the index is negative, it counts backward from length.
*/
- static int getSequenceIndex(int index, int length, Location loc) throws EvalException {
+ static int getSequenceIndex(int index, int length) throws EvalException {
int actualIndex = index;
if (actualIndex < 0) {
actualIndex += length;
}
if (actualIndex < 0 || actualIndex >= length) {
- throw new EvalException(
- loc,
- "index out of range (index is " + index + ", but sequence has " + length + " elements)");
+ throw Starlark.errorf(
+ "index out of range (index is %d, but sequence has %d elements)", index, length);
}
return actualIndex;
}
@@ -380,49 +379,48 @@
suffix = SpellChecker.didYouMean(name, CallUtils.getFieldNames(semantics, object));
}
return Starlark.errorf(
- "'%s' value has no field or method '%s'%s", getDataTypeName(object), name, suffix);
+ "'%s' value has no field or method '%s'%s", Starlark.type(object), name, suffix);
}
/** Evaluates an eager binary operation, {@code x op y}. (Excludes AND and OR.) */
static Object binaryOp(TokenKind op, Object x, Object y, StarlarkThread thread, Location location)
- throws EvalException, InterruptedException {
+ throws EvalException {
try {
switch (op) {
case PLUS:
return plus(x, y, thread, location);
case PIPE:
- return pipe(x, y, thread, location);
+ return pipe(thread.getSemantics(), x, y);
case AMPERSAND:
- return and(x, y, location);
+ return and(x, y);
case CARET:
- return xor(x, y, location);
+ return xor(x, y);
case GREATER_GREATER:
- return rightShift(x, y, location);
+ return rightShift(x, y);
case LESS_LESS:
- return leftShift(x, y, location);
+ return leftShift(x, y);
case MINUS:
- return minus(x, y, location);
+ return minus(x, y);
case STAR:
- return mult(x, y, thread, location);
+ return star(thread.mutability(), x, y);
case SLASH:
- throw new EvalException(
- location,
- "The `/` operator is not allowed. Please use the `//` operator for integer "
- + "division.");
+ throw Starlark.errorf(
+ "The `/` operator is not allowed. Please use the `//` operator for integer"
+ + " division.");
case SLASH_SLASH:
- return divide(x, y, location);
+ return slash(x, y);
case PERCENT:
- return percent(x, y, location);
+ return percent(x, y);
case EQUALS_EQUALS:
return x.equals(y);
@@ -431,41 +429,43 @@
return !x.equals(y);
case LESS:
- return compare(x, y, location) < 0;
+ return compare(x, y) < 0;
case LESS_EQUALS:
- return compare(x, y, location) <= 0;
+ return compare(x, y) <= 0;
case GREATER:
- return compare(x, y, location) > 0;
+ return compare(x, y) > 0;
case GREATER_EQUALS:
- return compare(x, y, location) >= 0;
+ return compare(x, y) >= 0;
case IN:
- return in(x, y, thread, location);
+ return in(thread.getSemantics(), x, y);
case NOT_IN:
- return !in(x, y, thread, location);
+ return !in(thread.getSemantics(), x, y);
default:
throw new AssertionError("Unsupported binary operator: " + op);
}
- } catch (ArithmeticException e) {
- throw new EvalException(location, e.getMessage());
+ } catch (ArithmeticException ex) {
+ throw new EvalException(null, ex.getMessage());
}
}
/** Implements comparison operators. */
- private static int compare(Object x, Object y, Location location) throws EvalException {
+ private static int compare(Object x, Object y) throws EvalException {
try {
return SKYLARK_COMPARATOR.compare(x, y);
} catch (ComparisonException e) {
- throw new EvalException(location, e);
+ throw new EvalException(null, e.getMessage());
}
}
/** Implements 'x + y'. */
+ // StarlarkThread is needed for Mutability and Semantics.
+ // Location is exposed to Selector{List,Value} and Concatter.
static Object plus(Object x, Object y, StarlarkThread thread, Location location)
throws EvalException {
// int + int
@@ -500,7 +500,7 @@
if (concatter != null && concatter.equals(robj.getConcatter())) {
return concatter.concat(lobj, robj, location);
} else {
- throw unknownBinaryOperator(x, y, TokenKind.PLUS, location);
+ throw unknownBinaryOperator(x, y, TokenKind.PLUS);
}
}
@@ -516,18 +516,16 @@
}
return Depset.unionOf((Depset) x, y);
}
- throw unknownBinaryOperator(x, y, TokenKind.PLUS, location);
+ throw unknownBinaryOperator(x, y, TokenKind.PLUS);
}
/** Implements 'x | y'. */
- private static Object pipe(Object x, Object y, StarlarkThread thread, Location location)
- throws EvalException {
+ private static Object pipe(StarlarkSemantics semantics, Object x, Object y) throws EvalException {
if (x instanceof Integer && y instanceof Integer) {
return ((Integer) x) | ((Integer) y);
} else if (x instanceof Depset) {
- if (thread.getSemantics().incompatibleDepsetUnion()) {
- throw new EvalException(
- location,
+ if (semantics.incompatibleDepsetUnion()) {
+ throw Starlark.errorf(
"`|` operator on a depset is forbidden. See "
+ "https://docs.bazel.build/versions/master/skylark/depsets.html for "
+ "recommendations. Use --incompatible_depset_union=false "
@@ -535,20 +533,19 @@
}
return Depset.unionOf((Depset) x, y);
}
- throw unknownBinaryOperator(x, y, TokenKind.PIPE, location);
+ throw unknownBinaryOperator(x, y, TokenKind.PIPE);
}
/** Implements 'x - y'. */
- private static Object minus(Object x, Object y, Location location) throws EvalException {
+ private static Object minus(Object x, Object y) throws EvalException {
if (x instanceof Integer && y instanceof Integer) {
return Math.subtractExact((Integer) x, (Integer) y);
}
- throw unknownBinaryOperator(x, y, TokenKind.MINUS, location);
+ throw unknownBinaryOperator(x, y, TokenKind.MINUS);
}
/** Implements 'x * y'. */
- private static Object mult(Object x, Object y, StarlarkThread thread, Location location)
- throws EvalException {
+ private static Object star(Mutability mu, Object x, Object y) throws EvalException {
Integer number = null;
Object otherFactor = null;
@@ -566,20 +563,20 @@
} else if (otherFactor instanceof String) {
return Strings.repeat((String) otherFactor, Math.max(0, number));
} else if (otherFactor instanceof Tuple) {
- return ((Tuple<?>) otherFactor).repeat(number, thread.mutability());
+ return ((Tuple<?>) otherFactor).repeat(number, mu);
} else if (otherFactor instanceof StarlarkList) {
- return ((StarlarkList<?>) otherFactor).repeat(number, thread.mutability());
+ return ((StarlarkList<?>) otherFactor).repeat(number, mu);
}
}
- throw unknownBinaryOperator(x, y, TokenKind.STAR, location);
+ throw unknownBinaryOperator(x, y, TokenKind.STAR);
}
/** Implements 'x // y'. */
- private static Object divide(Object x, Object y, Location location) throws EvalException {
+ private static Object slash(Object x, Object y) throws EvalException {
// int / int
if (x instanceof Integer && y instanceof Integer) {
if (y.equals(0)) {
- throw new EvalException(location, "integer division by zero");
+ throw Starlark.errorf("integer division by zero");
}
// Integer division doesn't give the same result in Java and in Python 2 with
// negative numbers.
@@ -588,15 +585,15 @@
// We want to follow Python semantics, so we use float division and round down.
return (int) Math.floor(Double.valueOf((Integer) x) / (Integer) y);
}
- throw unknownBinaryOperator(x, y, TokenKind.SLASH_SLASH, location);
+ throw unknownBinaryOperator(x, y, TokenKind.SLASH_SLASH);
}
/** Implements 'x % y'. */
- private static Object percent(Object x, Object y, Location location) throws EvalException {
+ private static Object percent(Object x, Object y) throws EvalException {
// int % int
if (x instanceof Integer && y instanceof Integer) {
if (y.equals(0)) {
- throw new EvalException(location, "integer modulo by zero");
+ throw Starlark.errorf("integer modulo by zero");
}
// Python and Java implement division differently, wrt negative numbers.
// In Python, sign of the result is the sign of the divisor.
@@ -618,96 +615,84 @@
return Starlark.formatWithList(pattern, (Tuple) y);
}
return Starlark.format(pattern, y);
- } catch (IllegalFormatException e) {
- throw new EvalException(location, e.getMessage());
+ } catch (IllegalFormatException ex) {
+ throw new EvalException(null, ex.getMessage());
}
}
- throw unknownBinaryOperator(x, y, TokenKind.PERCENT, location);
+ throw unknownBinaryOperator(x, y, TokenKind.PERCENT);
}
/** Implements 'x & y'. */
- private static Object and(Object x, Object y, Location location) throws EvalException {
+ private static Object and(Object x, Object y) throws EvalException {
if (x instanceof Integer && y instanceof Integer) {
return (Integer) x & (Integer) y;
}
- throw unknownBinaryOperator(x, y, TokenKind.AMPERSAND, location);
+ throw unknownBinaryOperator(x, y, TokenKind.AMPERSAND);
}
/** Implements 'x ^ y'. */
- private static Object xor(Object x, Object y, Location location) throws EvalException {
+ private static Object xor(Object x, Object y) throws EvalException {
if (x instanceof Integer && y instanceof Integer) {
return (Integer) x ^ (Integer) y;
}
- throw unknownBinaryOperator(x, y, TokenKind.CARET, location);
+ throw unknownBinaryOperator(x, y, TokenKind.CARET);
}
/** Implements 'x >> y'. */
- private static Object rightShift(Object x, Object y, Location location) throws EvalException {
+ private static Object rightShift(Object x, Object y) throws EvalException {
if (x instanceof Integer && y instanceof Integer) {
if ((Integer) y < 0) {
- throw new EvalException(location, "negative shift count: " + y);
+ throw Starlark.errorf("negative shift count: %s", y);
} else if ((Integer) y >= Integer.SIZE) {
return ((Integer) x < 0) ? -1 : 0;
}
return (Integer) x >> (Integer) y;
}
- throw unknownBinaryOperator(x, y, TokenKind.GREATER_GREATER, location);
+ throw unknownBinaryOperator(x, y, TokenKind.GREATER_GREATER);
}
/** Implements 'x << y'. */
- private static Object leftShift(Object x, Object y, Location location) throws EvalException {
+ private static Object leftShift(Object x, Object y) throws EvalException {
if (x instanceof Integer && y instanceof Integer) {
if ((Integer) y < 0) {
- throw new EvalException(location, "negative shift count: " + y);
+ throw Starlark.errorf("negative shift count: %s", y);
}
Integer result = (Integer) x << (Integer) y;
- if (!rightShift(result, y, location).equals(x)) {
+ if (!rightShift(result, y).equals(x)) {
throw new ArithmeticException("integer overflow");
}
return result;
}
- throw unknownBinaryOperator(x, y, TokenKind.LESS_LESS, location);
+ throw unknownBinaryOperator(x, y, TokenKind.LESS_LESS);
}
/** Implements 'x in y'. */
- private static boolean in(Object x, Object y, StarlarkThread thread, Location location)
- throws EvalException {
+ private static boolean in(StarlarkSemantics semantics, Object x, Object y) throws EvalException {
if (y instanceof SkylarkQueryable) {
- return ((SkylarkQueryable) y).containsKey(x, location, thread);
+ return ((SkylarkQueryable) y).containsKey(semantics, x);
} else if (y instanceof String) {
if (x instanceof String) {
return ((String) y).contains((String) x);
} else {
- throw new EvalException(
- location,
- "'in <string>' requires string as left operand, not '" + getDataTypeName(x) + "'");
+ throw Starlark.errorf(
+ "'in <string>' requires string as left operand, not '%s'", Starlark.type(x));
}
} else {
- throw new EvalException(
- location,
- "argument of type '"
- + getDataTypeName(y)
- + "' is not iterable. "
- + "in operator only works on lists, tuples, dicts and strings.");
+ throw Starlark.errorf(
+ "unsupported binary operation: %s in %s (right operand must be string, sequence, or"
+ + " dict)",
+ Starlark.type(x), Starlark.type(y));
}
}
/** Returns an exception signifying incorrect types for the given operator. */
- private static EvalException unknownBinaryOperator(
- Object x, Object y, TokenKind op, Location location) {
- // NB: this message format is identical to that used by CPython 2.7.6 or 3.4.0,
- // though python raises a TypeError.
- // TODO(adonovan): make error more concise: "unsupported binary op: list + int".
- return new EvalException(
- location,
- String.format(
- "unsupported operand type(s) for %s: '%s' and '%s'",
- op, getDataTypeName(x), getDataTypeName(y)));
+ private static EvalException unknownBinaryOperator(Object x, Object y, TokenKind op) {
+ return Starlark.errorf(
+ "unsupported binary operation: %s %s %s", Starlark.type(x), op, Starlark.type(y));
}
/** Evaluates a unary operation. */
- static Object unaryOp(TokenKind op, Object x, Location loc)
- throws EvalException, InterruptedException {
+ static Object unaryOp(TokenKind op, Object x) throws EvalException {
switch (op) {
case NOT:
return !Starlark.truth(x);
@@ -718,7 +703,7 @@
return Math.negateExact((Integer) x);
} catch (ArithmeticException e) {
// Fails for -MIN_INT.
- throw new EvalException(loc, e.getMessage());
+ throw new EvalException(null, e.getMessage());
}
}
break;
@@ -738,8 +723,7 @@
default:
/* fall through */
}
- throw new EvalException(
- loc, String.format("unsupported unary operation: %s%s", op, getDataTypeName(x)));
+ throw Starlark.errorf("unsupported unary operation: %s%s", op, Starlark.type(x));
}
/**
@@ -747,24 +731,22 @@
*
* @throws EvalException if {@code object} is not a sequence or mapping.
*/
- static Object index(Object object, Object key, StarlarkThread thread, Location loc)
- throws EvalException, InterruptedException {
+ static Object index(Mutability mu, StarlarkSemantics semantics, Object object, Object key)
+ throws EvalException {
if (object instanceof SkylarkIndexable) {
- Object result = ((SkylarkIndexable) object).getIndex(key, loc);
+ Object result = ((SkylarkIndexable) object).getIndex(semantics, key);
// TODO(bazel-team): We shouldn't have this fromJava call here. If it's needed at all,
// it should go in the implementations of SkylarkIndexable#getIndex that produce non-Skylark
// values.
- return result == null ? null : Starlark.fromJava(result, thread.mutability());
+ return result == null ? null : Starlark.fromJava(result, mu);
} else if (object instanceof String) {
String string = (String) object;
int index = Starlark.toInt(key, "string index");
- index = getSequenceIndex(index, string.length(), loc);
+ index = getSequenceIndex(index, string.length());
return string.substring(index, index + 1);
} else {
- throw new EvalException(
- loc,
- String.format(
- "type '%s' has no operator [](%s)", getDataTypeName(object), getDataTypeName(key)));
+ throw Starlark.errorf(
+ "type '%s' has no operator [](%s)", Starlark.type(object), Starlark.type(key));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Sequence.java b/src/main/java/com/google/devtools/build/lib/syntax/Sequence.java
index 1b08d7a..d743ca0 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Sequence.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Sequence.java
@@ -15,7 +15,6 @@
package com.google.devtools.build.lib.syntax;
import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
import java.util.Collections;
@@ -53,21 +52,15 @@
return ImmutableList.copyOf(this);
}
- /**
- * Retrieve an entry from a Sequence.
- *
- * @param key the index
- * @param loc a {@link Location} in case of error
- * @throws EvalException if the key is invalid
- */
+ /** Retrieves an entry from a Sequence. */
@Override
- default E getIndex(Object key, Location loc) throws EvalException {
+ default E getIndex(StarlarkSemantics semantics, Object key) throws EvalException {
int index = Starlark.toInt(key, "sequence index");
- return get(EvalUtils.getSequenceIndex(index, size(), loc));
+ return get(EvalUtils.getSequenceIndex(index, size()));
}
@Override
- default boolean containsKey(Object key, Location loc) throws EvalException {
+ default boolean containsKey(StarlarkSemantics semantics, Object key) throws EvalException {
return contains(key);
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkIndexable.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkIndexable.java
index 8f67534..c27ad7e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkIndexable.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkIndexable.java
@@ -14,13 +14,9 @@
package com.google.devtools.build.lib.syntax;
-import com.google.devtools.build.lib.events.Location;
-
-/**
- * Skylark values that support index access, i.e. `object[key]`
- */
+/** A Starlark value that support indexed access, {@code object[key]}. */
public interface SkylarkIndexable extends SkylarkQueryable {
/** Returns the value associated with the given key. */
- Object getIndex(Object key, Location loc) throws EvalException;
+ Object getIndex(StarlarkSemantics semantics, Object key) throws EvalException;
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkQueryable.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkQueryable.java
index 66d0bba..638a3f0 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkQueryable.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkQueryable.java
@@ -14,25 +14,12 @@
package com.google.devtools.build.lib.syntax;
-import com.google.devtools.build.lib.events.Location;
-
-/**
- * Skylark values that support querying by other objects, i.e. `foo in object`. Semantics of the
- * operation may differ, i.e. dicts check for keys and lists for values.
- */
+/** A Starlark value that supports membership tests, {@code key in object}. */
// TODO(adonovan): merge with SkylarkIndexable: no type supports 'x in y' without y[x],
// and 'x in y' can be defined in terms of y[x], at least as a default implementation.
// (Implementations of 'x in y' may choose to interpret failure of y[x] as false or a failure.)
public interface SkylarkQueryable extends StarlarkValue {
/** Returns whether the key is in the object. */
- boolean containsKey(Object key, Location loc) throws EvalException;
-
- // Variant used when called directly from a Starlark thread.
- // This is a temporary workaround to enable --incompatible_disallow_dict_lookup_unhashable_keys.
- // TODO(adonovan): remove when that flag is removed.
- default boolean containsKey(Object key, Location loc, StarlarkThread thread)
- throws EvalException {
- return this.containsKey(key, loc);
- }
+ boolean containsKey(StarlarkSemantics semantics, Object key) throws EvalException;
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkList.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkList.java
index 73d3366..83428d5 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkList.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkList.java
@@ -444,7 +444,7 @@
})
public Object pop(Object i) throws EvalException {
int arg = i == Starlark.NONE ? -1 : (Integer) i;
- int index = EvalUtils.getSequenceIndex(arg, size, /*loc=*/ null);
+ int index = EvalUtils.getSequenceIndex(arg, size);
Object result = elems[index];
remove(index, /*loc=*/ null);
return result;
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);
}
}
diff --git a/src/test/starlark/testdata/dict.sky b/src/test/starlark/testdata/dict.sky
index f2f73f0..8195d97 100644
--- a/src/test/starlark/testdata/dict.sky
+++ b/src/test/starlark/testdata/dict.sky
@@ -91,6 +91,10 @@
assert_eq("a" in dict(a=1), True)
---
+# What's going on here? Functions _are_ hashable.
+# 'len in {}' and '{}.get(len, False)' should both successfully evaluate to False.
+# TODO(adonovan): clarify spec and fix this test (https://github.com/bazelbuild/starlark/issues/65)
+
# unhashable types
{} in {} ### unhashable type: 'dict'
diff --git a/src/test/starlark/testdata/int.sky b/src/test/starlark/testdata/int.sky
index d4710cf..ca3cc67 100644
--- a/src/test/starlark/testdata/int.sky
+++ b/src/test/starlark/testdata/int.sky
@@ -125,9 +125,9 @@
assert_eq(~8 >> 1 | 3 ^ 4 & -2 << 2 * 3 + 4 // -2, -5)
---
-1 & False ### unsupported operand type(s) for &: 'int' and 'bool'
+1 & False ### unsupported binary operation: int & bool
---
-"a" ^ 5 ### unsupported operand type(s) for ^: 'string' and 'int'
+"a" ^ 5 ### unsupported binary operation: string ^ int
---
~False ### unsupported unary operation: ~bool
---
