diff options
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/syntax')
8 files changed, 253 insertions, 174 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java b/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java index 613deec104..31c74c0f7a 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/AbstractComprehension.java @@ -121,7 +121,7 @@ public abstract class AbstractComprehension extends Expression { EvalUtils.lock(iterableObject, loc); try { for (Object listElement : listValue) { - lvalue.assign(env, loc, listElement); + lvalue.assign(listElement, env, loc); evalStep(env, collector, step); } } finally { 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 fc2c5fd64a..9b74f30b60 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 @@ -59,7 +59,7 @@ public final class AssignmentStatement extends Statement { @Override void doExec(Environment env) throws EvalException, InterruptedException { Object rvalue = expression.eval(env); - lvalue.assign(env, getLocation(), rvalue); + lvalue.assign(rvalue, env, getLocation()); } @Override 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 cfa2c8fce2..04f54e799b 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 @@ -60,7 +60,7 @@ public final class AugmentedAssignmentStatement extends Statement { @Override void doExec(Environment env) throws EvalException, InterruptedException { - lvalue.assign(env, getLocation(), expression, operator); + lvalue.assignAugmented(operator, expression, env, getLocation()); } @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 459858aad4..4fff1bdae3 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 @@ -74,7 +74,7 @@ public final class BinaryOperatorExpression extends Expression { return lhs + " " + operator + " " + rhs; } - /** Implements comparison operators. */ + /** Implements comparison operators. */ private static int compare(Object lval, Object rval, Location location) throws EvalException { try { return EvalUtils.SKYLARK_COMPARATOR.compare(lval, rval); @@ -117,77 +117,126 @@ public final class BinaryOperatorExpression extends Expression { } } - public static Object evaluate( + /** + * Evaluates a short-circuiting binary operator, i.e. boolean {@code and} or {@code or}. + * + * <p>In contrast to {@link #evaluate}, this method takes unevaluated expressions. The left-hand + * side expression is evaluated exactly once, and the right-hand side expression is evaluated + * either once or not at all. + * + * @throws IllegalArgumentException if {@code operator} is not {@link Operator#AND} or + * {@link Operator#OR}. + */ + public static Object evaluateWithShortCircuiting( Operator operator, - Object lval, + Expression lhs, Expression rhs, Environment env, - Location location, - boolean isAugmented) + Location loc) throws EvalException, InterruptedException { - // Short-circuit operators + Object lval = lhs.eval(env); if (operator == Operator.AND) { - if (EvalUtils.toBoolean(lval)) { - return rhs.eval(env); - } else { - return lval; - } + return EvalUtils.toBoolean(lval) ? rhs.eval(env) : lval; + } else if (operator == Operator.OR) { + return EvalUtils.toBoolean(lval) ? lval : rhs.eval(env); + } else { + throw new IllegalArgumentException("Not a short-circuiting operator: " + operator); } + } - if (operator == Operator.OR) { - if (EvalUtils.toBoolean(lval)) { - return lval; - } else { - return rhs.eval(env); - } - } + /** + * Evaluates {@code lhs @ rhs}, where {@code @} is the operator, and returns the result. + * + * <p>This method does not implement any short-circuiting logic for boolean operations, as the + * parameters are already evaluated. + */ + public static Object evaluate( + Operator operator, + Object lhs, + Object rhs, + Environment env, + Location loc) + throws EvalException, InterruptedException { + return evaluate(operator, lhs, rhs, env, loc, /*isAugmented=*/false); + } - Object rval = rhs.eval(env); + /** + * Evaluates {@code lhs @= rhs} and returns the result, possibly mutating {@code lhs}. + * + * <p>Whether or not {@code lhs} is mutated depends on its type. If it is mutated, then it is also + * the return value. + */ + public static Object evaluateAugmented( + Operator operator, + Object lhs, + Object rhs, + Environment env, + Location loc) + throws EvalException, InterruptedException { + return evaluate(operator, lhs, rhs, env, loc, /*isAugmented=*/true); + } + private static Object evaluate( + Operator operator, + Object lhs, + Object rhs, + Environment env, + Location location, + boolean isAugmented) + throws EvalException, InterruptedException { try { switch (operator) { + // AND and OR are included for completeness, but should normally be handled using + // evaluateWithShortCircuiting() instead of this method. + + case AND: + return EvalUtils.toBoolean(lhs) ? rhs : lhs; + + case OR: + return EvalUtils.toBoolean(lhs) ? lhs : rhs; + case PLUS: - return plus(lval, rval, env, location, isAugmented); + return plus(lhs, rhs, env, location, isAugmented); case PIPE: - return pipe(lval, rval, location); + return pipe(lhs, rhs, location); case MINUS: - return minus(lval, rval, env, location); + return minus(lhs, rhs, env, location); case MULT: - return mult(lval, rval, env, location); + return mult(lhs, rhs, env, location); case DIVIDE: case FLOOR_DIVIDE: - return divide(lval, rval, location); + return divide(lhs, rhs, location); case PERCENT: - return percent(lval, rval, env, location); + return percent(lhs, rhs, env, location); case EQUALS_EQUALS: - return lval.equals(rval); + return lhs.equals(rhs); case NOT_EQUALS: - return !lval.equals(rval); + return !lhs.equals(rhs); case LESS: - return compare(lval, rval, location) < 0; + return compare(lhs, rhs, location) < 0; case LESS_EQUALS: - return compare(lval, rval, location) <= 0; + return compare(lhs, rhs, location) <= 0; case GREATER: - return compare(lval, rval, location) > 0; + return compare(lhs, rhs, location) > 0; case GREATER_EQUALS: - return compare(lval, rval, location) >= 0; + return compare(lhs, rhs, location) >= 0; case IN: - return in(lval, rval, env, location); + return in(lhs, rhs, env, location); case NOT_IN: - return !in(lval, rval, env, location); + return !in(lhs, rhs, env, location); default: throw new AssertionError("Unsupported binary operator: " + operator); @@ -199,7 +248,11 @@ public final class BinaryOperatorExpression extends Expression { @Override Object doEval(Environment env) throws EvalException, InterruptedException { - return evaluate(operator, lhs.eval(env), rhs, env, getLocation(), false); + if (operator == Operator.AND || operator == Operator.OR) { + return evaluateWithShortCircuiting(operator, lhs, rhs, env, getLocation()); + } else { + return evaluate(operator, lhs.eval(env), rhs.eval(env), env, getLocation()); + } } @Override @@ -247,8 +300,9 @@ public final class BinaryOperatorExpression extends Expression { MutableList<Object> list = (MutableList) lval; list.addAll((MutableList<?>) rval, location, env); return list; + } else { + return MutableList.concat((MutableList<?>) lval, (MutableList<?>) rval, env); } - return MutableList.concat((MutableList) lval, (MutableList) rval, env); } if (lval instanceof SkylarkDict && rval instanceof SkylarkDict) { 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 0fa85706f1..0e8a49f559 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 @@ -274,7 +274,7 @@ public final class EvalUtils { } /** - * @return the truth value of an object, according to Python rules. + * Returns the truth value of an object, according to Python rules. * http://docs.python.org/2/library/stdtypes.html#truth-value-testing */ public static boolean toBoolean(Object o) { diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java index de87f9b2c4..bb640ed295 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/ForStatement.java @@ -75,7 +75,7 @@ public final class ForStatement extends Statement { EvalUtils.lock(o, getLocation()); try { for (Object it : col) { - variable.assign(env, getLocation(), it); + variable.assign(it, env, getLocation()); try { for (Statement stmt : block) { 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 d77ccb9d9d..b341b33cc1 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 @@ -16,7 +16,11 @@ package com.google.devtools.build.lib.syntax; import com.google.devtools.build.lib.events.Location; import java.io.IOException; -/** Syntax node for an index expression. e.g. obj[field], but not obj[from:to] */ +/** + * An index expression ({@code obj[field]}). Not to be confused with a slice expression ({@code + * obj[from:to]}). The object may be either a sequence or an associative mapping (most commonly + * lists and dictionaries). + */ public final class IndexExpression extends Expression { private final Expression object; @@ -46,32 +50,33 @@ public final class IndexExpression extends Expression { @Override Object doEval(Environment env) throws EvalException, InterruptedException { - Object objValue = object.eval(env); - return eval(env, objValue); + return evaluate(object.eval(env), key.eval(env), env, getLocation()); } /** - * This method can be used instead of eval(Environment) if we want to avoid `obj` being evaluated - * several times. + * Retrieves the value associated with a key in the given object. + * + * @throws EvalException if {@code object} is not a list or dictionary */ - Object eval(Environment env, Object objValue) throws EvalException, InterruptedException { - Object keyValue = key.eval(env); - Location loc = getLocation(); - - if (objValue instanceof SkylarkIndexable) { - Object result = ((SkylarkIndexable) objValue).getIndex(keyValue, loc); + public static Object evaluate(Object object, Object key, Environment env, Location loc) + throws EvalException, InterruptedException { + if (object instanceof SkylarkIndexable) { + Object result = ((SkylarkIndexable) object).getIndex(key, loc); + // TODO(bazel-team): We shouldn't have this convertToSkylark call here. If it's needed at all, + // it should go in the implementations of SkylarkIndexable#getIndex that produce non-Skylark + // values. return SkylarkType.convertToSkylark(result, env); - } else if (objValue instanceof String) { - String string = (String) objValue; - int index = EvalUtils.getSequenceIndex(keyValue, string.length(), loc); + } else if (object instanceof String) { + String string = (String) object; + int index = EvalUtils.getSequenceIndex(key, string.length(), loc); return string.substring(index, index + 1); + } else { + throw new EvalException( + loc, + String.format( + "type '%s' has no operator [](%s)", + EvalUtils.getDataTypeName(object), EvalUtils.getDataTypeName(key))); } - - throw new EvalException( - loc, - Printer.format( - "type '%s' has no operator [](%s)", - EvalUtils.getDataTypeName(objValue), EvalUtils.getDataTypeName(keyValue))); } @Override 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 a409447f78..0fe8cf32fd 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 @@ -32,13 +32,14 @@ import java.util.Collection; * <p>An {@code LValue}'s expression must have one of the following forms: * <ul> * <li>(Variable assignment) an {@link Identifier}; - * <li>(Sequence assignment) a {@link ListLiteral} (either list or tuple) of expressions that can - * themselves appear in an {@code LValue}; or - * <li>(List or dictionary item assignment) an {@link IndexExpression}. + * <li>(List or dictionary item assignment) an {@link IndexExpression}; or + * <li>(Sequence assignment) a non-empty {@link ListLiteral} (either list or tuple) of expressions + * that can themselves appear in an {@code LValue}. * </ul> * In particular and unlike Python, slice expressions, dot expressions, and starred expressions - * cannot appear in LValues. + * cannot appear in {@code LValue}s. */ +// TODO(bazel-team): Add support for assigning to slices (e.g. a[2:6] = [3]). public final class LValue extends ASTNode { private final Expression expr; @@ -53,39 +54,142 @@ public final class LValue extends ASTNode { } /** - * Assign a value to an LValue and update the environment. + * Updates the environment bindings, and possibly mutates objects, so as to assign the given value + * to this {@code LValue}. */ - public void assign(Environment env, Location loc, Object result) + public void assign(Object value, Environment env, Location loc) throws EvalException, InterruptedException { - doAssign(env, loc, expr, result); + assign(expr, value, env, loc); } /** - * Evaluate a rhs using a lhs and the operator, then assign to the lhs. + * 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}. */ - public void assign(Environment env, Location loc, Expression rhs, Operator operator) + private static void assign(Expression expr, Object value, Environment env, Location loc) throws EvalException, InterruptedException { if (expr instanceof Identifier) { - Object result = - BinaryOperatorExpression.evaluate(operator, expr.eval(env), rhs, env, loc, true); - assign(env, loc, (Identifier) expr, result); - return; + assignIdentifier((Identifier) expr, value, env, loc); + } else if (expr instanceof IndexExpression) { + Object object = ((IndexExpression) expr).getObject().eval(env); + Object key = ((IndexExpression) expr).getKey().eval(env); + assignItem(object, key, value, env, loc); + } else if (expr instanceof ListLiteral) { + ListLiteral list = (ListLiteral) expr; + assignList(list, value, env, loc); + } else { + // Not possible for validated ASTs. + throw new EvalException(loc, "cannot assign to '" + expr + "'"); + } + } + + /** + * Binds a variable to the given value in the environment. + * + * @throws EvalException if we're currently in a function's scope, and the identifier has + * previously resolved to a global variable in the same function + */ + private static void assignIdentifier( + Identifier ident, Object value, Environment env, Location loc) + throws EvalException, InterruptedException { + Preconditions.checkNotNull(value, "trying to assign null to %s", ident); + + if (env.isKnownGlobalVariable(ident.getName())) { + throw new EvalException( + loc, + String.format( + "Variable '%s' is referenced before assignment. " + + "The variable is defined in the global scope.", + ident.getName())); } + env.update(ident.getName(), value); + } - if (expr instanceof IndexExpression) { + /** + * Adds or changes an object-key-value relationship for a list or dict. + * + * <p>For a list, the key is an in-range index. For a dict, it is a hashable value. + * + * @throws EvalException if the object is not a list or dict + */ + @SuppressWarnings("unchecked") + private static void assignItem( + Object object, Object key, Object value, Environment env, Location loc) + throws EvalException, InterruptedException { + if (object instanceof SkylarkDict) { + SkylarkDict<Object, Object> dict = (SkylarkDict<Object, Object>) object; + dict.put(key, value, loc, env); + } else if (object instanceof SkylarkList) { + SkylarkList<Object> list = (SkylarkList<Object>) object; + list.set(key, value, loc, env); + } else { + throw new EvalException( + loc, + "can only assign an element in a dictionary or a list, not in a '" + + EvalUtils.getDataTypeName(object) + + "'"); + } + } + + /** + * Recursively assigns an iterable value to a list literal. + * + * @throws EvalException if the list literal has length 0, or if the value is not an iterable of + * matching length + */ + private static void assignList(ListLiteral list, Object value, Environment env, Location loc) + throws EvalException, InterruptedException { + Collection<?> collection = EvalUtils.toCollection(value, loc, env); + int len = list.getElements().size(); + if (len == 0) { + throw new EvalException( + loc, + "lists or tuples on the left-hand side of assignments must have at least one item"); + } + if (len != collection.size()) { + 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, collection.size())); + } + int i = 0; + for (Object item : collection) { + assign(list.getElements().get(i), item, env, loc); + i++; + } + } + + /** + * Evaluates an augmented assignment that mutates this {@code LValue} with the given right-hand + * side's value. + * + * <p>The left-hand side expression is evaluated only once, even when it is an {@link + * IndexExpression}. The left-hand side is evaluated before the right-hand side to match Python's + * behavior (hence why the right-hand side is passed as an expression rather than as an evaluated + * value). + */ + public void assignAugmented(Operator operator, Expression rhs, Environment env, Location loc) + throws EvalException, InterruptedException { + if (expr instanceof Identifier) { + Object result = + BinaryOperatorExpression.evaluateAugmented( + operator, expr.eval(env), rhs.eval(env), env, loc); + assignIdentifier((Identifier) expr, result, env, loc); + } else 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); + // The object and key should be evaluated only once, so we don't use expr.eval(). + Object object = indexExpression.getObject().eval(env); Object key = indexExpression.getKey().eval(env); + Object oldValue = IndexExpression.evaluate(object, key, env, loc); + // Evaluate rhs after lhs. + Object rhsValue = rhs.eval(env); Object result = - BinaryOperatorExpression.evaluate(operator, evaluatedLhs, rhs, env, loc, true); - assignItem(env, loc, evaluatedLhsObject, key, result); - return; - } - - if (expr instanceof ListLiteral) { - throw new EvalException(loc, "Cannot perform augment assignment on a list literal"); + BinaryOperatorExpression.evaluateAugmented(operator, oldValue, rhsValue, env, loc); + assignItem(object, key, result, env, loc); + } else if (expr instanceof ListLiteral) { + throw new EvalException(loc, "cannot perform augmented assignment on a list literal"); + } else { + // Not possible for validated ASTs. + throw new EvalException(loc, "cannot perform augmented assignment on '" + expr + "'"); } } @@ -118,84 +222,6 @@ public final class LValue extends ASTNode { } } - 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, env); - int len = variables.getElements().size(); - if (len != rvalue.size()) { - throw new EvalException(loc, String.format( - "lvalue has length %d, but rvalue has has length %d", len, rvalue.size())); - } - if (len == 0) { - throw new EvalException(loc, "invalid lvalue, expected at least one item"); - } - int i = 0; - for (Object o : rvalue) { - doAssign(env, loc, variables.getElements().get(i), o); - i++; - } - return; - } - - // 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 (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 '" + lhs + "'"); - } - - @SuppressWarnings("unchecked") - private static void assignItem( - Environment env, Location loc, Object o, Object key, Object value) - throws EvalException, InterruptedException { - if (o instanceof SkylarkDict) { - SkylarkDict<Object, Object> dict = (SkylarkDict<Object, Object>) o; - dict.put(key, value, loc, env); - } else if (o instanceof SkylarkList) { - SkylarkList<Object> list = (SkylarkList<Object>) o; - list.set(key, value, loc, env); - } else { - throw new EvalException( - loc, - "can only assign an element in a dictionary or a list, not in a '" - + EvalUtils.getDataTypeName(o) - + "'"); - } - } - - /** - * Assign value to a single variable. - */ - private static void assign(Environment env, Location loc, Identifier ident, Object result) - throws EvalException, InterruptedException { - Preconditions.checkNotNull(result, "trying to assign null to %s", ident); - - // The variable may have been referenced successfully if a global variable - // with the same name exists. In this case an Exception needs to be thrown. - if (env.isKnownGlobalVariable(ident.getName())) { - throw new EvalException( - loc, - String.format( - "Variable '%s' is referenced before assignment. " - + "The variable is defined in the global scope.", - ident.getName())); - } - env.update(ident.getName(), result); - } - @Override public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); @@ -208,22 +234,16 @@ public final class LValue extends ASTNode { private static void validate(ValidationEnvironment env, Location loc, Expression expr) throws EvalException { if (expr instanceof Identifier) { - Identifier ident = (Identifier) expr; - env.declare(ident.getName(), loc); - return; - } - if (expr instanceof ListLiteral) { + env.declare(((Identifier) expr).getName(), loc); + } else if (expr instanceof IndexExpression) { + expr.validate(env); + } else if (expr instanceof ListLiteral) { for (Expression e : ((ListLiteral) expr).getElements()) { validate(env, loc, e); } - return; - } - if (expr instanceof IndexExpression) { - expr.validate(env); - return; + } else { + throw new EvalException(loc, "cannot assign to '" + expr + "'"); } - throw new EvalException(loc, - "cannot assign to '" + expr + "'"); } @Override |