diff options
Diffstat (limited to 'src')
5 files changed, 95 insertions, 28 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java index a33482f82f..c11722469e 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/AugmentedAssignmentStatement.java @@ -55,9 +55,7 @@ public final class AugmentedAssignmentStatement extends Statement { @Override void doExec(Environment env) throws EvalException, InterruptedException { Location loc = getLocation(); - Object result = - BinaryOperatorExpression.evaluate(operator, lvalue.getExpression(), expression, env, loc); - lvalue.assign(env, loc, result); + lvalue.assign(env, loc, expression, operator); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java index a2c445778f..0da5e5c4fb 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/BinaryOperatorExpression.java @@ -93,7 +93,12 @@ public final class BinaryOperatorExpression extends Expression { Operator operator, Expression lhs, Expression rhs, Environment env, Location location) throws EvalException, InterruptedException { Object lval = lhs.eval(env); + return evaluate(operator, lval, rhs, env, location); + } + public static Object evaluate( + Operator operator, Object lval, Expression rhs, Environment env, Location location) + throws EvalException, InterruptedException { // Short-circuit operators if (operator == Operator.AND) { if (EvalUtils.toBoolean(lval)) { diff --git a/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java index fd2a59b07d..5a3b81378e 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/IndexExpression.java @@ -43,6 +43,14 @@ public final class IndexExpression extends Expression { @Override Object doEval(Environment env) throws EvalException, InterruptedException { Object objValue = obj.eval(env); + return eval(env, objValue); + } + + /** + * This method can be used instead of eval(Environment) if we want to avoid `obj` being evaluated + * several times. + */ + Object eval(Environment env, Object objValue) throws EvalException, InterruptedException { Object keyValue = key.eval(env); Location loc = getLocation(); 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 8047ec81ed..22d686bf4e 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 @@ -43,18 +43,46 @@ public class LValue implements Serializable { */ public void assign(Environment env, Location loc, Object result) throws EvalException, InterruptedException { - assign(env, loc, expr, result); + doAssign(env, loc, expr, result); } - private static void assign(Environment env, Location loc, Expression lvalue, Object result) + /** + * Evaluate a rhs using a lhs and the operator, then assign to the lhs. + */ + public void assign(Environment env, Location loc, Expression rhs, Operator operator) throws EvalException, InterruptedException { - if (lvalue instanceof Identifier) { - assign(env, loc, (Identifier) lvalue, result); + if (expr instanceof Identifier) { + Object result = BinaryOperatorExpression.evaluate(operator, expr, rhs, env, loc); + assign(env, loc, (Identifier) expr, result); + return; + } + + if (expr instanceof IndexExpression) { + IndexExpression indexExpression = (IndexExpression) expr; + // This object should be evaluated only once + Object evaluatedLhsObject = indexExpression.getObject().eval(env); + Object evaluatedLhs = indexExpression.eval(env, evaluatedLhsObject); + Object key = indexExpression.getKey().eval(env); + Object result = BinaryOperatorExpression.evaluate(operator, evaluatedLhs, rhs, env, loc); + assignItem(env, loc, evaluatedLhsObject, key, result); return; } - if (lvalue instanceof ListLiteral) { - ListLiteral variables = (ListLiteral) lvalue; + if (expr instanceof ListLiteral) { + throw new EvalException(loc, "Cannot perform augment assignment on a list literal"); + } + } + + private static void doAssign( + Environment env, Location loc, Expression lhs, Object result) + throws EvalException, InterruptedException { + if (lhs instanceof Identifier) { + assign(env, loc, (Identifier) lhs, result); + return; + } + + if (lhs instanceof ListLiteral) { + ListLiteral variables = (ListLiteral) lhs; Collection<?> rvalue = EvalUtils.toCollection(result, loc); int len = variables.getElements().size(); if (len != rvalue.size()) { @@ -63,7 +91,7 @@ public class LValue implements Serializable { } int i = 0; for (Object o : rvalue) { - assign(env, loc, variables.getElements().get(i), o); + doAssign(env, loc, variables.getElements().get(i), o); i++; } return; @@ -71,15 +99,14 @@ public class LValue implements Serializable { // Support syntax for setting an element in an array, e.g. a[5] = 2 // TODO: We currently do not allow slices (e.g. a[2:6] = [3]). - if (lvalue instanceof IndexExpression) { - IndexExpression expression = (IndexExpression) lvalue; + if (lhs instanceof IndexExpression) { + IndexExpression expression = (IndexExpression) lhs; Object key = expression.getKey().eval(env); Object evaluatedObject = expression.getObject().eval(env); assignItem(env, loc, evaluatedObject, key, result); return; } - throw new EvalException(loc, - "cannot assign to '" + lvalue + "'"); + throw new EvalException(loc, "cannot assign to '" + lhs + "'"); } @SuppressWarnings("unchecked") @@ -150,18 +177,4 @@ public class LValue implements Serializable { public String toString() { return expr.toString(); } - - /** - * Checks that the size of a collection at runtime conforms to the amount of l-value expressions - * we have to assign to. - */ - public static void checkSize(int expected, Collection<?> collection, Location location) - throws EvalException { - int actual = collection.size(); - if (expected != actual) { - throw new EvalException( - location, - String.format("lvalue has length %d, but rvalue has has length %d", expected, actual)); - } - } } 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 0a701e0e5c..6353c1270e 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 @@ -909,6 +909,49 @@ public class SkylarkEvaluationTest extends EvaluationTest { } @Test + public void testAugmentedAssignmentHasNoSideEffects() throws Exception { + new SkylarkTest().setUp( + "counter = [0]", + "value = [1, 2]", + "", + "def f():", + " counter[0] = counter[0] + 1", + " return value", + "", + "f()[1] += 1") // `f()` should be called only once here + .testLookup("counter", MutableList.of(env, 1)); + } + + @Test + public void testAugmentedAssignmentNotAllowedForListLiterals() throws Exception { + new SkylarkTest().testIfErrorContains("Cannot perform augment assignment on a list literal", + "def f(a, b):", + " [a, b] += []", + "f(1, 2)"); + } + + @Test + public void testAssignmentEvaluationOrder() throws Exception { + new SkylarkTest().setUp( + "ordinary = []", + "augmented = []", + "value = [1, 2]", + "", + "def f(record):", + " record.append('f')", + " return value", + "", + "def g(record):", + " record.append('g')", + " return value", + "", + "f(ordinary)[0] = g(ordinary)[1]", + "f(augmented)[0] += g(augmented)[1]") + .testLookup("ordinary", MutableList.of(env, "g", "f")) // This order is consistent + .testLookup("augmented", MutableList.of(env, "f", "g")); // with Python + } + + @Test public void testStaticDirectJavaCall() throws Exception { new SkylarkTest().update("Mock", Mock.class).setUp("val = Mock.value_of('8')") .testLookup("val", 8); |