Skylark: Use LValue class in loops and comprehensions.
--
MOS_MIGRATED_REVID=89020190
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java
index f0063d0..601f128 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/AssignmentStatement.java
@@ -52,7 +52,8 @@
@Override
void exec(Environment env) throws EvalException, InterruptedException {
- lvalue.assign(env, getLocation(), expression);
+ Object rvalue = expression.eval(env);
+ lvalue.assign(env, getLocation(), rvalue);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/DictComprehension.java b/src/main/java/com/google/devtools/build/lib/syntax/DictComprehension.java
index 05190ab..4c7278b 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/DictComprehension.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/DictComprehension.java
@@ -24,14 +24,14 @@
private final Expression keyExpression;
private final Expression valueExpression;
- private final Ident loopVar;
+ private final LValue loopVar;
private final Expression listExpression;
- public DictComprehension(Expression keyExpression, Expression valueExpression, Ident loopVar,
+ public DictComprehension(Expression keyExpression, Expression valueExpression, Expression loopVar,
Expression listExpression) {
this.keyExpression = keyExpression;
this.valueExpression = valueExpression;
- this.loopVar = loopVar;
+ this.loopVar = new LValue(loopVar);
this.listExpression = listExpression;
}
@@ -43,7 +43,7 @@
return valueExpression;
}
- Ident getLoopVar() {
+ LValue getLoopVar() {
return loopVar;
}
@@ -57,7 +57,7 @@
LinkedHashMap<Object, Object> map = new LinkedHashMap<>();
Iterable<?> elements = EvalUtils.toIterable(listExpression.eval(env), getLocation());
for (Object element : elements) {
- env.update(loopVar.getName(), element);
+ loopVar.assign(env, getLocation(), element);
Object key = keyExpression.eval(env);
map.put(key, valueExpression.eval(env));
}
@@ -68,7 +68,7 @@
SkylarkType validate(ValidationEnvironment env) throws EvalException {
SkylarkType elementsType = listExpression.validate(env);
SkylarkType listElementType = SkylarkType.getGenericArgType(elementsType);
- env.update(loopVar.getName(), listElementType, getLocation());
+ loopVar.validate(env, getLocation(), listElementType);
SkylarkType keyType = keyExpression.validate(env);
if (!keyType.isSimple()) {
// TODO(bazel-team): this is most probably dead code but it's better to have it here
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/LValue.java b/src/main/java/com/google/devtools/build/lib/syntax/LValue.java
index a328118..2713154 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/LValue.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/LValue.java
@@ -42,7 +42,7 @@
/**
* Assign a value to an LValue and update the environment.
*/
- public void assign(Environment env, Location loc, Expression rvalue)
+ public void assign(Environment env, Location loc, Object result)
throws EvalException, InterruptedException {
if (!(expr instanceof Ident)) {
throw new EvalException(loc,
@@ -50,8 +50,7 @@
}
Ident ident = (Ident) expr;
- Object result = rvalue.eval(env);
- Preconditions.checkNotNull(result, "result of %s is null", rvalue);
+ Preconditions.checkNotNull(result, "trying to assign null to %s", ident);
if (env.isSkylarkEnabled()) {
// The variable may have been referenced successfully if a global variable
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java b/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java
index 3153e8c..26cbfc0 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ListComprehension.java
@@ -28,7 +28,7 @@
private final Expression elementExpression;
// This cannot be a map, because we need to both preserve order _and_ allow duplicate identifiers.
- private final List<Map.Entry<Ident, Expression>> lists;
+ private final List<Map.Entry<LValue, Expression>> lists;
/**
* [elementExpr (for var in listExpr)+]
@@ -44,9 +44,9 @@
return convert(new ArrayList<>(), env);
}
- List<Map.Entry<Ident, Iterable<?>>> listValues = Lists.newArrayListWithCapacity(lists.size());
+ List<Map.Entry<LValue, Iterable<?>>> listValues = Lists.newArrayListWithCapacity(lists.size());
int size = 1;
- for (Map.Entry<Ident, Expression> list : lists) {
+ for (Map.Entry<LValue, Expression> list : lists) {
Object listValueObject = list.getValue().eval(env);
final Iterable<?> listValue = EvalUtils.toIterable(listValueObject, getLocation());
int listSize = EvalUtils.size(listValue);
@@ -54,7 +54,7 @@
return convert(new ArrayList<>(), env);
}
size *= listSize;
- listValues.add(Maps.<Ident, Iterable<?>>immutableEntry(list.getKey(), listValue));
+ listValues.add(Maps.<LValue, Iterable<?>>immutableEntry(list.getKey(), listValue));
}
List<Object> resultList = Lists.newArrayListWithCapacity(size);
evalLists(env, listValues, resultList);
@@ -73,7 +73,7 @@
public String toString() {
StringBuilder sb = new StringBuilder();
sb.append('[').append(elementExpression);
- for (Map.Entry<Ident, Expression> list : lists) {
+ for (Map.Entry<LValue, Expression> list : lists) {
sb.append(" for ").append(list.getKey()).append(" in ").append(list.getValue());
}
sb.append(']');
@@ -84,11 +84,11 @@
return elementExpression;
}
- public void add(Ident ident, Expression listExpression) {
- lists.add(Maps.immutableEntry(ident, listExpression));
+ public void add(Expression loopVar, Expression listExpression) {
+ lists.add(Maps.immutableEntry(new LValue(loopVar), listExpression));
}
- public List<Map.Entry<Ident, Expression>> getLists() {
+ public List<Map.Entry<LValue, Expression>> getLists() {
return lists;
}
@@ -106,11 +106,11 @@
* of the element expression to the result. Otherwise calls itself recursively
* with all the lists except the outermost.
*/
- private void evalLists(Environment env, List<Map.Entry<Ident, Iterable<?>>> listValues,
+ private void evalLists(Environment env, List<Map.Entry<LValue, Iterable<?>>> listValues,
List<Object> result) throws EvalException, InterruptedException {
- Map.Entry<Ident, Iterable<?>> listValue = listValues.get(0);
+ Map.Entry<LValue, Iterable<?>> listValue = listValues.get(0);
for (Object listElement : listValue.getValue()) {
- env.update(listValue.getKey().getName(), listElement);
+ listValue.getKey().assign(env, getLocation(), listElement);
if (listValues.size() == 1) {
result.add(elementExpression.eval(env));
} else {
@@ -121,11 +121,11 @@
@Override
SkylarkType validate(ValidationEnvironment env) throws EvalException {
- for (Map.Entry<Ident, Expression> list : lists) {
+ for (Map.Entry<LValue, Expression> list : lists) {
// TODO(bazel-team): Get the type of elements
SkylarkType type = list.getValue().validate(env);
env.checkIterable(type, getLocation());
- env.update(list.getKey().getName(), SkylarkType.UNKNOWN, getLocation());
+ list.getKey().validate(env, getLocation(), SkylarkType.UNKNOWN);
}
elementExpression.validate(env);
return SkylarkType.LIST;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java b/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
index 439a4b1..a373594 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SyntaxTreeVisitor.java
@@ -61,8 +61,8 @@
public void visit(ListComprehension node) {
visit(node.getElementExpression());
- for (Map.Entry<Ident, Expression> list : node.getLists()) {
- visit(list.getKey());
+ for (Map.Entry<LValue, Expression> list : node.getLists()) {
+ visit(list.getKey().getExpression());
visit(list.getValue());
}
}
@@ -70,7 +70,7 @@
public void accept(DictComprehension node) {
visit(node.getKeyExpression());
visit(node.getValueExpression());
- visit(node.getLoopVar());
+ visit(node.getLoopVar().getExpression());
visit(node.getListExpression());
}
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 b2bfaec..2a1b495 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
@@ -363,21 +363,21 @@
@Test
public void testDictComprehensions() throws Exception {
- assertEquals(Collections.emptyMap(), eval("{x : x for x in []}"));
- assertEquals(ImmutableMap.of(1, 1, 2, 2), eval("{x : x for x in [1, 2]}"));
+ assertEquals(Collections.emptyMap(), eval("{a : a for a in []}"));
+ assertEquals(ImmutableMap.of(1, 1, 2, 2), eval("{b : b for b in [1, 2]}"));
assertEquals(ImmutableMap.of("a", "v_a", "b", "v_b"),
- eval("{x : 'v_' + x for x in ['a', 'b']}"));
+ eval("{c : 'v_' + c for c in ['a', 'b']}"));
assertEquals(ImmutableMap.of("k_a", "a", "k_b", "b"),
- eval("{'k_' + x : x for x in ['a', 'b']}"));
+ eval("{'k_' + d : d for d in ['a', 'b']}"));
assertEquals(ImmutableMap.of("k_a", "v_a", "k_b", "v_b"),
- eval("{'k_' + x : 'v_' + x for x in ['a', 'b']}"));
+ eval("{'k_' + e : 'v_' + e for e in ['a', 'b']}"));
}
@Test
public void testDictComprehensions_MultipleKey() throws Exception {
assertEquals(ImmutableMap.of(1, 1, 2, 2), eval("{x : x for x in [1, 2, 1]}"));
assertEquals(ImmutableMap.of("ab", "ab", "c", "c"),
- eval("{x : x for x in ['ab', 'c', 'a' + 'b']}"));
+ eval("{y : y for y in ['ab', 'c', 'a' + 'b']}"));
}
@Test
