bazel syntax: eliminate Operator; use TokenKind
Also:
- rename TokenKind.getPrettyName -> toString.
No-one wants to see internal enum names in error messages.
- use shorter names (op, x, y) throughout {Bi,U}naryOperatorExpression.
PiperOrigin-RevId: 255981802
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 addc790..fab391f 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
@@ -21,19 +21,19 @@
public final class AugmentedAssignmentStatement extends Statement {
private final Expression lhs; // same constraint as AssignmentStatement
- private final Operator operator;
+ private final TokenKind op;
private final Expression rhs;
/** Constructs an augmented assignment. */
- public AugmentedAssignmentStatement(Operator operator, Expression lhs, Expression rhs) {
+ public AugmentedAssignmentStatement(TokenKind op, Expression lhs, Expression rhs) {
this.lhs = lhs;
- this.operator = operator;
+ this.op = op;
this.rhs = rhs;
}
/** Returns the operator of the assignment. */
- public Operator getOperator() {
- return operator;
+ public TokenKind getOperator() {
+ return op;
}
/** Returns the LHS of the assignment. */
@@ -51,7 +51,7 @@
printIndent(buffer, indentLevel);
lhs.prettyPrint(buffer);
buffer.append(' ');
- buffer.append(operator.toString());
+ buffer.append(op.toString());
buffer.append("= ");
rhs.prettyPrint(buffer);
buffer.append('\n');
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 ac6cce3..e231416 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
@@ -19,48 +19,68 @@
import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
import java.io.IOException;
+import java.util.EnumSet;
import java.util.IllegalFormatException;
-/** Syntax node for a binary operator expression. */
+/** A BinaryExpression represents a binary operator expression 'x op y'. */
public final class BinaryOperatorExpression extends Expression {
- private final Expression lhs;
+ private final Expression x;
+ private final TokenKind op; // one of 'operators'
+ private final Expression y;
- private final Expression rhs;
+ /** operators is the set of valid binary operators. */
+ public static final EnumSet<TokenKind> operators =
+ EnumSet.of(
+ TokenKind.AND,
+ TokenKind.EQUALS_EQUALS,
+ TokenKind.GREATER,
+ TokenKind.GREATER_EQUALS,
+ TokenKind.IN,
+ TokenKind.LESS,
+ TokenKind.LESS_EQUALS,
+ TokenKind.MINUS,
+ TokenKind.NOT_EQUALS,
+ TokenKind.NOT_IN,
+ TokenKind.OR,
+ TokenKind.PERCENT,
+ TokenKind.SLASH,
+ TokenKind.SLASH_SLASH,
+ TokenKind.PLUS,
+ TokenKind.PIPE,
+ TokenKind.STAR);
- private final Operator operator;
-
- public BinaryOperatorExpression(Operator operator, Expression lhs, Expression rhs) {
- this.lhs = lhs;
- this.rhs = rhs;
- this.operator = operator;
+ public BinaryOperatorExpression(Expression x, TokenKind op, Expression y) {
+ this.x = x;
+ this.op = op;
+ this.y = y;
}
- public Expression getLhs() {
- return lhs;
+ /** getX returns the left operand. */
+ public Expression getX() {
+ return x;
}
- public Expression getRhs() {
- return rhs;
+ /** getOperator returns the operator. */
+ public TokenKind getOperator() {
+ return op;
}
- /**
- * Returns the operator kind for this binary operation.
- */
- public Operator getOperator() {
- return operator;
+ /** getY returns the right operand. */
+ public Expression getY() {
+ return y;
}
@Override
public void prettyPrint(Appendable buffer) throws IOException {
- // TODO(bazel-team): Possibly omit parentheses when they are not needed according to operator
- // precedence rules. This requires passing down more contextual information.
+ // TODO(bazel-team): retain parentheses in the syntax tree so we needn't
+ // conservatively emit them here.
buffer.append('(');
- lhs.prettyPrint(buffer);
+ x.prettyPrint(buffer);
buffer.append(' ');
- buffer.append(operator.toString());
+ buffer.append(op.toString());
buffer.append(' ');
- rhs.prettyPrint(buffer);
+ y.prettyPrint(buffer);
buffer.append(')');
}
@@ -68,47 +88,47 @@
public String toString() {
// This omits the parentheses for brevity, but is not correct in general due to operator
// precedence rules.
- return lhs + " " + operator + " " + rhs;
+ return x + " " + op + " " + y;
}
/** Implements comparison operators. */
- private static int compare(Object lval, Object rval, Location location) throws EvalException {
+ private static int compare(Object x, Object y, Location location) throws EvalException {
try {
- return EvalUtils.SKYLARK_COMPARATOR.compare(lval, rval);
+ return EvalUtils.SKYLARK_COMPARATOR.compare(x, y);
} catch (EvalUtils.ComparisonException e) {
throw new EvalException(location, e);
}
}
- /** Implements the "in" operator. */
- private static boolean in(Object lval, Object rval, Environment env, Location location)
+ /** Implements 'x in y'. */
+ private static boolean in(Object x, Object y, Environment env, Location location)
throws EvalException {
- if (env.getSemantics().incompatibleDepsetIsNotIterable() && rval instanceof SkylarkNestedSet) {
+ if (env.getSemantics().incompatibleDepsetIsNotIterable() && y instanceof SkylarkNestedSet) {
throw new EvalException(
location,
"argument of type '"
- + EvalUtils.getDataTypeName(rval)
+ + EvalUtils.getDataTypeName(y)
+ "' is not iterable. "
+ "in operator only works on lists, tuples, dicts and strings. "
+ "Use --incompatible_depset_is_not_iterable=false to temporarily disable "
+ "this check.");
- } else if (rval instanceof SkylarkQueryable) {
- return ((SkylarkQueryable) rval).containsKey(lval, location, env.getStarlarkContext());
- } else if (rval instanceof String) {
- if (lval instanceof String) {
- return ((String) rval).contains((String) lval);
+ } else if (y instanceof SkylarkQueryable) {
+ return ((SkylarkQueryable) y).containsKey(x, location, env.getStarlarkContext());
+ } 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 '"
- + EvalUtils.getDataTypeName(lval)
+ + EvalUtils.getDataTypeName(x)
+ "'");
}
} else {
throw new EvalException(
location,
"argument of type '"
- + EvalUtils.getDataTypeName(rval)
+ + EvalUtils.getDataTypeName(y)
+ "' is not iterable. "
+ "in operator only works on lists, tuples, dicts and strings.");
}
@@ -121,111 +141,98 @@
* 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}.
+ * @throws IllegalArgumentException if {@code op} is not {@link Operator#AND} or {@link
+ * Operator#OR}.
*/
public static Object evaluateWithShortCircuiting(
- Operator operator,
- Expression lhs,
- Expression rhs,
- Environment env,
- Location loc)
+ TokenKind op, Expression x, Expression y, Environment env, Location loc)
throws EvalException, InterruptedException {
- Object lval = lhs.eval(env);
- if (operator == Operator.AND) {
- 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);
+ Object xval = x.eval(env);
+ switch (op) {
+ case AND:
+ return EvalUtils.toBoolean(xval) ? y.eval(env) : xval;
+ case OR:
+ return EvalUtils.toBoolean(xval) ? xval : y.eval(env);
+ default:
+ throw new IllegalArgumentException("Not a short-circuiting operator: " + op);
}
}
/**
- * Evaluates {@code lhs @ rhs}, where {@code @} is the operator, and returns the result.
+ * Evaluates {@code x @ y}, 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)
+ public static Object evaluate(TokenKind op, Object x, Object y, Environment env, Location loc)
throws EvalException, InterruptedException {
- return evaluate(operator, lhs, rhs, env, loc, /*isAugmented=*/false);
+ return evaluate(op, x, y, env, loc, /*isAugmented=*/ false);
}
private static Object evaluate(
- Operator operator,
- Object lhs,
- Object rhs,
- Environment env,
- Location location,
- boolean isAugmented)
+ TokenKind op, Object x, Object y, Environment env, Location location, boolean isAugmented)
throws EvalException, InterruptedException {
try {
- switch (operator) {
+ switch (op) {
// 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;
+ return EvalUtils.toBoolean(x) ? y : x;
case OR:
- return EvalUtils.toBoolean(lhs) ? lhs : rhs;
+ return EvalUtils.toBoolean(x) ? x : y;
case PLUS:
- return plus(lhs, rhs, env, location, isAugmented);
+ return plus(x, y, env, location, isAugmented);
case PIPE:
- return pipe(lhs, rhs, env, location);
+ return pipe(x, y, env, location);
case MINUS:
- return minus(lhs, rhs, location);
+ return minus(x, y, location);
- case MULT:
- return mult(lhs, rhs, env, location);
+ case STAR:
+ return mult(x, y, env, location);
- case DIVIDE:
+ case SLASH:
throw new EvalException(
location,
"The `/` operator is not allowed. Please use the `//` operator for integer "
+ "division.");
- case FLOOR_DIVIDE:
- return divide(lhs, rhs, location);
+ case SLASH_SLASH:
+ return divide(x, y, location);
case PERCENT:
- return percent(lhs, rhs, location);
+ return percent(x, y, location);
case EQUALS_EQUALS:
- return lhs.equals(rhs);
+ return x.equals(y);
case NOT_EQUALS:
- return !lhs.equals(rhs);
+ return !x.equals(y);
case LESS:
- return compare(lhs, rhs, location) < 0;
+ return compare(x, y, location) < 0;
case LESS_EQUALS:
- return compare(lhs, rhs, location) <= 0;
+ return compare(x, y, location) <= 0;
case GREATER:
- return compare(lhs, rhs, location) > 0;
+ return compare(x, y, location) > 0;
case GREATER_EQUALS:
- return compare(lhs, rhs, location) >= 0;
+ return compare(x, y, location) >= 0;
case IN:
- return in(lhs, rhs, env, location);
+ return in(x, y, env, location);
case NOT_IN:
- return !in(lhs, rhs, env, location);
+ return !in(x, y, env, location);
default:
- throw new AssertionError("Unsupported binary operator: " + operator);
+ throw new AssertionError("Unsupported binary operator: " + op);
} // endswitch
} catch (ArithmeticException e) {
throw new EvalException(location, e.getMessage());
@@ -233,23 +240,23 @@
}
/**
- * Evaluates {@code lhs @= rhs} and returns the result, possibly mutating {@code lhs}.
+ * Evaluates {@code x @= y} and returns the result, possibly mutating {@code x}.
*
- * <p>Whether or not {@code lhs} is mutated depends on its type. If it is mutated, then it is also
+ * <p>Whether or not {@code x} 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)
+ TokenKind op, Object x, Object y, Environment env, Location loc)
throws EvalException, InterruptedException {
- return evaluate(operator, lhs, rhs, env, loc, /*isAugmented=*/ true);
+ return evaluate(op, x, y, env, loc, /*isAugmented=*/ true);
}
@Override
Object doEval(Environment env) throws EvalException, InterruptedException {
- if (operator == Operator.AND || operator == Operator.OR) {
- return evaluateWithShortCircuiting(operator, lhs, rhs, env, getLocation());
+ if (op == TokenKind.AND || op == TokenKind.OR) {
+ return evaluateWithShortCircuiting(op, x, y, env, getLocation());
} else {
- return evaluate(operator, lhs.eval(env), rhs.eval(env), env, getLocation());
+ return evaluate(op, x.eval(env), y.eval(env), env, getLocation());
}
}
@@ -263,42 +270,43 @@
return Kind.BINARY_OPERATOR;
}
- /** Implements Operator.PLUS. */
+ /** Implements 'x + y'. */
private static Object plus(
- Object lval, Object rval, Environment env, Location location, boolean isAugmented)
+ Object x, Object y, Environment env, Location location, boolean isAugmented)
throws EvalException {
// int + int
- if (lval instanceof Integer && rval instanceof Integer) {
- return Math.addExact((Integer) lval, (Integer) rval);
+ if (x instanceof Integer && y instanceof Integer) {
+ return Math.addExact((Integer) x, (Integer) y);
}
// string + string
- if (lval instanceof String && rval instanceof String) {
- return (String) lval + (String) rval;
+ if (x instanceof String && y instanceof String) {
+ return (String) x + (String) y;
}
- if (lval instanceof SelectorValue || rval instanceof SelectorValue
- || lval instanceof SelectorList
- || rval instanceof SelectorList) {
- return SelectorList.concat(location, lval, rval);
+ if (x instanceof SelectorValue
+ || y instanceof SelectorValue
+ || x instanceof SelectorList
+ || y instanceof SelectorList) {
+ return SelectorList.concat(location, x, y);
}
- if ((lval instanceof Tuple) && (rval instanceof Tuple)) {
- return Tuple.concat((Tuple<?>) lval, (Tuple<?>) rval);
+ if (x instanceof Tuple && y instanceof Tuple) {
+ return Tuple.concat((Tuple<?>) x, (Tuple<?>) y);
}
- if ((lval instanceof MutableList) && (rval instanceof MutableList)) {
+ if (x instanceof MutableList && y instanceof MutableList) {
if (isAugmented) {
@SuppressWarnings("unchecked")
- MutableList<Object> list = (MutableList) lval;
- list.addAll((MutableList<?>) rval, location, env.mutability());
+ MutableList<Object> list = (MutableList) x;
+ list.addAll((MutableList<?>) y, location, env.mutability());
return list;
} else {
- return MutableList.concat((MutableList<?>) lval, (MutableList<?>) rval, env.mutability());
+ return MutableList.concat((MutableList<?>) x, (MutableList<?>) y, env.mutability());
}
}
- if (lval instanceof SkylarkDict && rval instanceof SkylarkDict) {
+ if (x instanceof SkylarkDict && y instanceof SkylarkDict) {
if (env.getSemantics().incompatibleDisallowDictPlus()) {
throw new EvalException(
location,
@@ -306,22 +314,22 @@
+ "`update` method instead. You can temporarily enable the `+` operator by passing "
+ "the flag --incompatible_disallow_dict_plus=false");
}
- return SkylarkDict.plus((SkylarkDict<?, ?>) lval, (SkylarkDict<?, ?>) rval, env);
+ return SkylarkDict.plus((SkylarkDict<?, ?>) x, (SkylarkDict<?, ?>) y, env);
}
- if (lval instanceof Concatable && rval instanceof Concatable) {
- Concatable lobj = (Concatable) lval;
- Concatable robj = (Concatable) rval;
+ if (x instanceof Concatable && y instanceof Concatable) {
+ Concatable lobj = (Concatable) x;
+ Concatable robj = (Concatable) y;
Concatter concatter = lobj.getConcatter();
if (concatter != null && concatter.equals(robj.getConcatter())) {
return concatter.concat(lobj, robj, location);
} else {
- throw typeException(lval, rval, Operator.PLUS, location);
+ throw typeException(x, y, TokenKind.PLUS, location);
}
}
// TODO(bazel-team): Remove deprecated operator.
- if (lval instanceof SkylarkNestedSet) {
+ if (x instanceof SkylarkNestedSet) {
if (env.getSemantics().incompatibleDepsetUnion()) {
throw new EvalException(
location,
@@ -330,15 +338,15 @@
+ "recommendations. Use --incompatible_depset_union=false "
+ "to temporarily disable this check.");
}
- return SkylarkNestedSet.of((SkylarkNestedSet) lval, rval, location);
+ return SkylarkNestedSet.of((SkylarkNestedSet) x, y, location);
}
- throw typeException(lval, rval, Operator.PLUS, location);
+ throw typeException(x, y, TokenKind.PLUS, location);
}
- /** Implements Operator.PIPE. */
- private static Object pipe(Object lval, Object rval, Environment env, Location location)
+ /** Implements 'x | y'. */
+ private static Object pipe(Object x, Object y, Environment env, Location location)
throws EvalException {
- if (lval instanceof SkylarkNestedSet) {
+ if (x instanceof SkylarkNestedSet) {
if (env.getSemantics().incompatibleDepsetUnion()) {
throw new EvalException(
location,
@@ -347,31 +355,31 @@
+ "recommendations. Use --incompatible_depset_union=false "
+ "to temporarily disable this check.");
}
- return SkylarkNestedSet.of((SkylarkNestedSet) lval, rval, location);
+ return SkylarkNestedSet.of((SkylarkNestedSet) x, y, location);
}
- throw typeException(lval, rval, Operator.PIPE, location);
+ throw typeException(x, y, TokenKind.PIPE, location);
}
- /** Implements Operator.MINUS. */
- private static Object minus(Object lval, Object rval, Location location) throws EvalException {
- if (lval instanceof Integer && rval instanceof Integer) {
- return Math.subtractExact((Integer) lval, (Integer) rval);
+ /** Implements 'x - y'. */
+ private static Object minus(Object x, Object y, Location location) throws EvalException {
+ if (x instanceof Integer && y instanceof Integer) {
+ return Math.subtractExact((Integer) x, (Integer) y);
}
- throw typeException(lval, rval, Operator.MINUS, location);
+ throw typeException(x, y, TokenKind.MINUS, location);
}
- /** Implements Operator.MULT. */
- private static Object mult(Object lval, Object rval, Environment env, Location location)
+ /** Implements 'x * y'. */
+ private static Object mult(Object x, Object y, Environment env, Location location)
throws EvalException {
Integer number = null;
Object otherFactor = null;
- if (lval instanceof Integer) {
- number = (Integer) lval;
- otherFactor = rval;
- } else if (rval instanceof Integer) {
- number = (Integer) rval;
- otherFactor = lval;
+ if (x instanceof Integer) {
+ number = (Integer) x;
+ otherFactor = y;
+ } else if (y instanceof Integer) {
+ number = (Integer) y;
+ otherFactor = x;
}
if (number != null) {
@@ -385,14 +393,14 @@
return ((SkylarkList<?>) otherFactor).repeat(number, env.mutability());
}
}
- throw typeException(lval, rval, Operator.MULT, location);
+ throw typeException(x, y, TokenKind.STAR, location);
}
- /** Implements Operator.DIVIDE. */
- private static Object divide(Object lval, Object rval, Location location) throws EvalException {
+ /** Implements 'x // y'. */
+ private static Object divide(Object x, Object y, Location location) throws EvalException {
// int / int
- if (lval instanceof Integer && rval instanceof Integer) {
- if (rval.equals(0)) {
+ if (x instanceof Integer && y instanceof Integer) {
+ if (y.equals(0)) {
throw new EvalException(location, "integer division by zero");
}
// Integer division doesn't give the same result in Java and in Python 2 with
@@ -400,23 +408,22 @@
// Java: -7/3 = -2
// Python: -7/3 = -3
// We want to follow Python semantics, so we use float division and round down.
- return (int) Math.floor(Double.valueOf((Integer) lval) / (Integer) rval);
+ return (int) Math.floor(Double.valueOf((Integer) x) / (Integer) y);
}
- throw typeException(lval, rval, Operator.FLOOR_DIVIDE, location);
+ throw typeException(x, y, TokenKind.SLASH_SLASH, location);
}
- /** Implements Operator.PERCENT. */
- private static Object percent(Object lval, Object rval, Location location)
- throws EvalException {
+ /** Implements 'x % y'. */
+ private static Object percent(Object x, Object y, Location location) throws EvalException {
// int % int
- if (lval instanceof Integer && rval instanceof Integer) {
- if (rval.equals(0)) {
+ if (x instanceof Integer && y instanceof Integer) {
+ if (y.equals(0)) {
throw new EvalException(location, "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.
- int div = (Integer) rval;
- int result = ((Integer) lval).intValue() % Math.abs(div);
+ int div = (Integer) y;
+ int result = ((Integer) x).intValue() % Math.abs(div);
if (result > 0 && div < 0) {
result += div; // make the result negative
} else if (result < 0 && div > 0) {
@@ -426,25 +433,22 @@
}
// string % tuple, string % dict, string % anything-else
- if (lval instanceof String) {
- String pattern = (String) lval;
+ if (x instanceof String) {
+ String pattern = (String) x;
try {
- if (rval instanceof Tuple) {
- return Printer.formatWithList(pattern, (Tuple) rval);
+ if (y instanceof Tuple) {
+ return Printer.formatWithList(pattern, (Tuple) y);
}
- return Printer.format(pattern, rval);
+ return Printer.format(pattern, y);
} catch (IllegalFormatException e) {
throw new EvalException(location, e.getMessage());
}
}
- throw typeException(lval, rval, Operator.PERCENT, location);
+ throw typeException(x, y, TokenKind.PERCENT, location);
}
- /**
- * Throws an exception signifying incorrect types for the given operator.
- */
- private static EvalException typeException(
- Object lval, Object rval, Operator operator, Location location) {
+ /** Throws an exception signifying incorrect types for the given operator. */
+ private static EvalException typeException(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.
// For more details, we'll hopefully have usable stack traces at some point.
@@ -452,8 +456,6 @@
location,
String.format(
"unsupported operand type(s) for %s: '%s' and '%s'",
- operator,
- EvalUtils.getDataTypeName(lval),
- EvalUtils.getDataTypeName(rval)));
+ op, EvalUtils.getDataTypeName(x), EvalUtils.getDataTypeName(y)));
}
}
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 036210d..3199952 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
@@ -325,12 +325,11 @@
* value).
*/
private static void assignAugmented(
- Expression expr, Operator operator, Expression rhs, Environment env, Location loc)
+ Expression expr, TokenKind op, 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);
+ BinaryOperatorExpression.evaluateAugmented(op, expr.eval(env), rhs.eval(env), env, loc);
assignIdentifier((Identifier) expr, result, env);
} else if (expr instanceof IndexExpression) {
IndexExpression indexExpression = (IndexExpression) expr;
@@ -340,8 +339,7 @@
Object oldValue = IndexExpression.evaluate(object, key, env, loc);
// Evaluate rhs after lhs.
Object rhsValue = rhs.eval(env);
- Object result =
- BinaryOperatorExpression.evaluateAugmented(operator, oldValue, rhsValue, env, loc);
+ Object result = BinaryOperatorExpression.evaluateAugmented(op, 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");
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Operator.java b/src/main/java/com/google/devtools/build/lib/syntax/Operator.java
deleted file mode 100644
index c0740e9..0000000
--- a/src/main/java/com/google/devtools/build/lib/syntax/Operator.java
+++ /dev/null
@@ -1,49 +0,0 @@
-// Copyright 2014 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.syntax;
-
-/** Infix binary operators. */
-public enum Operator {
-
- AND("and"),
- DIVIDE("/"),
- EQUALS_EQUALS("=="),
- FLOOR_DIVIDE("//"),
- GREATER(">"),
- GREATER_EQUALS(">="),
- IN("in"),
- LESS("<"),
- LESS_EQUALS("<="),
- MINUS("-"),
- MULT("*"),
- NOT("not"),
- NOT_EQUALS("!="),
- NOT_IN("not in"),
- OR("or"),
- PERCENT("%"),
- PIPE("|"),
- PLUS("+");
-
- private final String name;
-
- Operator(String name) {
- this.name = name;
- }
-
- @Override
- public String toString() {
- return name;
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index a1aa1e5..9daa9a3 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -126,49 +126,38 @@
private final Lexer lexer;
private final EventHandler eventHandler;
- private static final Map<TokenKind, Operator> binaryOperators =
- new ImmutableMap.Builder<TokenKind, Operator>()
- .put(TokenKind.AND, Operator.AND)
- .put(TokenKind.EQUALS_EQUALS, Operator.EQUALS_EQUALS)
- .put(TokenKind.GREATER, Operator.GREATER)
- .put(TokenKind.GREATER_EQUALS, Operator.GREATER_EQUALS)
- .put(TokenKind.IN, Operator.IN)
- .put(TokenKind.LESS, Operator.LESS)
- .put(TokenKind.LESS_EQUALS, Operator.LESS_EQUALS)
- .put(TokenKind.MINUS, Operator.MINUS)
- .put(TokenKind.NOT_EQUALS, Operator.NOT_EQUALS)
- .put(TokenKind.NOT_IN, Operator.NOT_IN)
- .put(TokenKind.OR, Operator.OR)
- .put(TokenKind.PERCENT, Operator.PERCENT)
- .put(TokenKind.SLASH, Operator.DIVIDE)
- .put(TokenKind.SLASH_SLASH, Operator.FLOOR_DIVIDE)
- .put(TokenKind.PLUS, Operator.PLUS)
- .put(TokenKind.PIPE, Operator.PIPE)
- .put(TokenKind.STAR, Operator.MULT)
+ // TODO(adonovan): opt: compute this by subtraction.
+ private static final Map<TokenKind, TokenKind> augmentedAssignmentMethods =
+ new ImmutableMap.Builder<TokenKind, TokenKind>()
+ .put(TokenKind.PLUS_EQUALS, TokenKind.PLUS)
+ .put(TokenKind.MINUS_EQUALS, TokenKind.MINUS)
+ .put(TokenKind.STAR_EQUALS, TokenKind.STAR)
+ .put(TokenKind.SLASH_EQUALS, TokenKind.SLASH)
+ .put(TokenKind.SLASH_SLASH_EQUALS, TokenKind.SLASH_SLASH)
+ .put(TokenKind.PERCENT_EQUALS, TokenKind.PERCENT)
.build();
- private static final Map<TokenKind, Operator> augmentedAssignmentMethods =
- new ImmutableMap.Builder<TokenKind, Operator>()
- .put(TokenKind.PLUS_EQUALS, Operator.PLUS)
- .put(TokenKind.MINUS_EQUALS, Operator.MINUS)
- .put(TokenKind.STAR_EQUALS, Operator.MULT)
- .put(TokenKind.SLASH_EQUALS, Operator.DIVIDE)
- .put(TokenKind.SLASH_SLASH_EQUALS, Operator.FLOOR_DIVIDE)
- .put(TokenKind.PERCENT_EQUALS, Operator.PERCENT)
- .build();
-
- /** Highest precedence goes last.
- * Based on: http://docs.python.org/2/reference/expressions.html#operator-precedence
- **/
- private static final List<EnumSet<Operator>> operatorPrecedence = ImmutableList.of(
- EnumSet.of(Operator.OR),
- EnumSet.of(Operator.AND),
- EnumSet.of(Operator.NOT),
- EnumSet.of(Operator.EQUALS_EQUALS, Operator.NOT_EQUALS, Operator.LESS, Operator.LESS_EQUALS,
- Operator.GREATER, Operator.GREATER_EQUALS, Operator.IN, Operator.NOT_IN),
- EnumSet.of(Operator.PIPE),
- EnumSet.of(Operator.MINUS, Operator.PLUS),
- EnumSet.of(Operator.DIVIDE, Operator.FLOOR_DIVIDE, Operator.MULT, Operator.PERCENT));
+ /**
+ * Highest precedence goes last. Based on:
+ * http://docs.python.org/2/reference/expressions.html#operator-precedence
+ */
+ private static final List<EnumSet<TokenKind>> operatorPrecedence =
+ ImmutableList.of(
+ EnumSet.of(TokenKind.OR),
+ EnumSet.of(TokenKind.AND),
+ EnumSet.of(TokenKind.NOT),
+ EnumSet.of(
+ TokenKind.EQUALS_EQUALS,
+ TokenKind.NOT_EQUALS,
+ TokenKind.LESS,
+ TokenKind.LESS_EQUALS,
+ TokenKind.GREATER,
+ TokenKind.GREATER_EQUALS,
+ TokenKind.IN,
+ TokenKind.NOT_IN),
+ EnumSet.of(TokenKind.PIPE),
+ EnumSet.of(TokenKind.MINUS, TokenKind.PLUS),
+ EnumSet.of(TokenKind.SLASH, TokenKind.SLASH_SLASH, TokenKind.STAR, TokenKind.PERCENT));
private int errorsCount;
private boolean recoveryMode; // stop reporting errors until next statement
@@ -335,7 +324,7 @@
private boolean expect(TokenKind kind) {
boolean expected = token.kind == kind;
if (!expected) {
- syntaxError("expected " + kind.getPrettyName());
+ syntaxError("expected " + kind);
}
nextToken();
return expected;
@@ -425,7 +414,9 @@
case RAISE: error = "'raise' not supported, use 'fail' instead"; break;
case TRY: error = "'try' not supported, all exceptions are fatal"; break;
case WHILE: error = "'while' not supported, use 'for' instead"; break;
- default: error = "keyword '" + token.kind.getPrettyName() + "' not supported"; break;
+ default:
+ error = "keyword '" + token.kind + "' not supported";
+ break;
}
reportError(lexer.createLocation(token.left, token.right), error);
}
@@ -676,7 +667,7 @@
{
nextToken();
Expression expr = parsePrimaryWithSuffix();
- UnaryOperatorExpression minus = new UnaryOperatorExpression(UnaryOperator.MINUS, expr);
+ UnaryOperatorExpression minus = new UnaryOperatorExpression(TokenKind.MINUS, expr);
return setLocation(minus, start, expr);
}
default:
@@ -802,7 +793,7 @@
nextToken();
return expr;
} else {
- syntaxError("expected '" + closingBracket.getPrettyName() + "', 'for' or 'if'");
+ syntaxError("expected '" + closingBracket + "', 'for' or 'if'");
syncPast(LIST_TERMINATOR_SET);
return makeErrorExpression(comprehensionStartOffset, token.right);
}
@@ -927,9 +918,8 @@
Expression expr = parseNonTupleExpression(prec + 1);
// The loop is not strictly needed, but it prevents risks of stack overflow. Depth is
// limited to number of different precedence levels (operatorPrecedence.size()).
- Operator lastOp = null;
+ TokenKind lastOp = null;
for (;;) {
-
if (token.kind == TokenKind.NOT) {
// If NOT appears when we expect a binary operator, it must be followed by IN.
// Since the code expects every operator to be a single token, we push a NOT_IN token.
@@ -940,44 +930,40 @@
token.kind = TokenKind.NOT_IN;
}
- if (!binaryOperators.containsKey(token.kind)) {
- return expr;
- }
- Operator operator = binaryOperators.get(token.kind);
- if (!operatorPrecedence.get(prec).contains(operator)) {
+ TokenKind op = token.kind;
+ if (!operatorPrecedence.get(prec).contains(op)) {
return expr;
}
// Operator '==' and other operators of the same precedence (e.g. '<', 'in')
// are not associative.
- if (lastOp != null && operatorPrecedence.get(prec).contains(Operator.EQUALS_EQUALS)) {
+ if (lastOp != null && operatorPrecedence.get(prec).contains(TokenKind.EQUALS_EQUALS)) {
reportError(
lexer.createLocation(token.left, token.right),
- String.format("Operator '%s' is not associative with operator '%s'. Use parens.",
- lastOp, operator));
+ String.format(
+ "Operator '%s' is not associative with operator '%s'. Use parens.", lastOp, op));
}
nextToken();
Expression secondary = parseNonTupleExpression(prec + 1);
- expr = optimizeBinOpExpression(operator, expr, secondary);
+ expr = optimizeBinOpExpression(expr, op, secondary);
setLocation(expr, start, secondary);
- lastOp = operator;
+ lastOp = op;
}
}
// Optimize binary expressions.
// string literal + string literal can be concatenated into one string literal
// so we don't have to do the expensive string concatenation at runtime.
- private Expression optimizeBinOpExpression(
- Operator operator, Expression expr, Expression secondary) {
- if (operator == Operator.PLUS) {
- if (expr instanceof StringLiteral && secondary instanceof StringLiteral) {
- StringLiteral left = (StringLiteral) expr;
- StringLiteral right = (StringLiteral) secondary;
+ private Expression optimizeBinOpExpression(Expression x, TokenKind op, Expression y) {
+ if (op == TokenKind.PLUS) {
+ if (x instanceof StringLiteral && y instanceof StringLiteral) {
+ StringLiteral left = (StringLiteral) x;
+ StringLiteral right = (StringLiteral) y;
return new StringLiteral(stringInterner.intern(left.getValue() + right.getValue()));
}
}
- return new BinaryOperatorExpression(operator, expr, secondary);
+ return new BinaryOperatorExpression(x, op, y);
}
// Equivalent to 'test' rule in Python grammar.
@@ -1005,7 +991,7 @@
if (prec >= operatorPrecedence.size()) {
return parsePrimaryWithSuffix();
}
- if (token.kind == TokenKind.NOT && operatorPrecedence.get(prec).contains(Operator.NOT)) {
+ if (token.kind == TokenKind.NOT && operatorPrecedence.get(prec).contains(TokenKind.NOT)) {
return parseNotExpression(prec);
}
return parseBinOpExpression(prec);
@@ -1016,9 +1002,8 @@
int start = token.left;
expect(TokenKind.NOT);
Expression expression = parseNonTupleExpression(prec);
- UnaryOperatorExpression notExpression =
- new UnaryOperatorExpression(UnaryOperator.NOT, expression);
- return setLocation(notExpression, start, expression);
+ UnaryOperatorExpression not = new UnaryOperatorExpression(TokenKind.NOT, expression);
+ return setLocation(not, start, expression);
}
// file_input ::= ('\n' | stmt)* EOF
@@ -1171,11 +1156,10 @@
Expression rhs = parseExpression();
return setLocation(new AssignmentStatement(expression, rhs), start, rhs);
} else if (augmentedAssignmentMethods.containsKey(token.kind)) {
- Operator operator = augmentedAssignmentMethods.get(token.kind);
+ TokenKind op = augmentedAssignmentMethods.get(token.kind);
nextToken();
Expression operand = parseExpression();
- return setLocation(
- new AugmentedAssignmentStatement(operator, expression, operand), start, operand);
+ return setLocation(new AugmentedAssignmentStatement(op, expression, operand), start, operand);
} else {
return setLocation(new ExpressionStatement(expression), start, expression);
}
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 1f5d6e0..751d551 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
@@ -64,8 +64,8 @@
}
public void visit(BinaryOperatorExpression node) {
- visit(node.getLhs());
- visit(node.getRhs());
+ visit(node.getX());
+ visit(node.getY());
}
public void visit(FuncallExpression node) {
@@ -160,7 +160,7 @@
}
public void visit(UnaryOperatorExpression node) {
- visit(node.getOperand());
+ visit(node.getX());
}
public void visit(DotExpression node) {
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Token.java b/src/main/java/com/google/devtools/build/lib/syntax/Token.java
index 733b538..e836f992 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Token.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Token.java
@@ -53,9 +53,9 @@
@Override
public String toString() {
// TODO(bazel-team): do proper escaping of string literals
- return kind == TokenKind.STRING ? ("\"" + value + "\"")
- : value == null ? kind.getPrettyName()
- : value.toString();
+ return kind == TokenKind.STRING
+ ? ("\"" + value + "\"")
+ : value == null ? kind.toString() : value.toString();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/TokenKind.java b/src/main/java/com/google/devtools/build/lib/syntax/TokenKind.java
index e2a4dcd..b85743c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/TokenKind.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/TokenKind.java
@@ -48,7 +48,7 @@
IMPORT("import"),
IN("in"),
INDENT("indent"),
- INT("integer"),
+ INT("integer literal"),
IS("is"),
LAMBDA("lambda"),
LBRACE("{"),
@@ -85,22 +85,20 @@
STAR("*"),
STAR_EQUALS("*="),
STAR_STAR("**"),
- STRING("string"),
+ STRING("string literal"),
TRY("try"),
WHILE("while"),
WITH("with"),
YIELD("yield");
- private final String prettyName;
+ private final String name;
- private TokenKind(String prettyName) {
- this.prettyName = prettyName;
+ private TokenKind(String name) {
+ this.name = name;
}
- /**
- * Returns the pretty name for this token, for use in error messages for the user.
- */
- public String getPrettyName() {
- return prettyName;
+ @Override
+ public String toString() {
+ return name;
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperator.java b/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperator.java
deleted file mode 100644
index 85fe921..0000000
--- a/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperator.java
+++ /dev/null
@@ -1,34 +0,0 @@
-// Copyright 2017 The Bazel Authors. All rights reserved.
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package com.google.devtools.build.lib.syntax;
-
-/** Unary operators. */
-public enum UnaryOperator {
-
- // Include trailing whitespace in name for non-symbolic operators (for pretty printing).
- NOT("not "),
- MINUS("-");
-
- private final String name;
-
- UnaryOperator(String name) {
- this.name = name;
- }
-
- @Override
- public String toString() {
- return name;
- }
-}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java
index 824333a..7341708 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java
@@ -16,53 +16,49 @@
import com.google.devtools.build.lib.events.Location;
import java.io.IOException;
-/** Syntax node for a unary operator expression. */
+/** A UnaryOperatorExpression represents a unary operator expression, 'op x'. */
public final class UnaryOperatorExpression extends Expression {
- private final UnaryOperator operator;
+ private final TokenKind op; // NOT or MINUS
+ private final Expression x;
- private final Expression operand;
-
- public UnaryOperatorExpression(UnaryOperator operator, Expression operand) {
- this.operator = operator;
- this.operand = operand;
+ public UnaryOperatorExpression(TokenKind op, Expression x) {
+ this.op = op;
+ this.x = x;
}
- public UnaryOperator getOperator() {
- return operator;
+ /** getOperator returns the operator. */
+ public TokenKind getOperator() {
+ return op;
}
- public Expression getOperand() {
- return operand;
+ /** getX returns the operand. */
+ public Expression getX() {
+ return x;
}
@Override
public void prettyPrint(Appendable buffer) throws IOException {
- // TODO(bazel-team): Possibly omit parentheses when they are not needed according to operator
- // precedence rules. This requires passing down more contextual information.
- buffer.append(operator.toString());
+ // TODO(bazel-team): retain parentheses in the syntax tree so we needn't
+ // conservatively emit them here.
+ buffer.append(op == TokenKind.NOT ? "not " : op.toString());
buffer.append('(');
- operand.prettyPrint(buffer);
+ x.prettyPrint(buffer);
buffer.append(')');
}
@Override
public String toString() {
- // All current and planned unary operators happen to be prefix operators.
- // Non-symbolic operators have trailing whitespace built into their name.
- //
// Note that this omits the parentheses for brevity, but is not correct in general due to
// operator precedence rules. For example, "(not False) in mylist" prints as
// "not False in mylist", which evaluates to opposite results in the case that mylist is empty.
- return operator.toString() + operand;
+ // TODO(adonovan): record parentheses explicitly in syntax tree.
+ return (op == TokenKind.NOT ? "not " : op.toString()) + x;
}
- private static Object evaluate(
- UnaryOperator operator,
- Object value,
- Location loc)
+ private static Object evaluate(TokenKind op, Object value, Location loc)
throws EvalException, InterruptedException {
- switch (operator) {
+ switch (op) {
case NOT:
return !EvalUtils.toBoolean(value);
@@ -81,13 +77,13 @@
}
default:
- throw new AssertionError("Unsupported unary operator: " + operator);
+ throw new AssertionError("Unsupported unary operator: " + op);
}
}
@Override
Object doEval(Environment env) throws EvalException, InterruptedException {
- return evaluate(operator, operand.eval(env), getLocation());
+ return evaluate(op, x.eval(env), getLocation());
}
@Override
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
index 6cc5fef..cfc9641 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/ParserTest.java
@@ -105,35 +105,35 @@
BinaryOperatorExpression e =
(BinaryOperatorExpression) parseExpression("'%sx' % 'foo' + 'bar'");
- assertThat(e.getOperator()).isEqualTo(Operator.PLUS);
+ assertThat(e.getOperator()).isEqualTo(TokenKind.PLUS);
}
@Test
public void testPrecedence2() throws Exception {
BinaryOperatorExpression e =
(BinaryOperatorExpression) parseExpression("('%sx' % 'foo') + 'bar'");
- assertThat(e.getOperator()).isEqualTo(Operator.PLUS);
+ assertThat(e.getOperator()).isEqualTo(TokenKind.PLUS);
}
@Test
public void testPrecedence3() throws Exception {
BinaryOperatorExpression e =
(BinaryOperatorExpression) parseExpression("'%sx' % ('foo' + 'bar')");
- assertThat(e.getOperator()).isEqualTo(Operator.PERCENT);
+ assertThat(e.getOperator()).isEqualTo(TokenKind.PERCENT);
}
@Test
public void testPrecedence4() throws Exception {
BinaryOperatorExpression e =
(BinaryOperatorExpression) parseExpression("1 + - (2 - 3)");
- assertThat(e.getOperator()).isEqualTo(Operator.PLUS);
+ assertThat(e.getOperator()).isEqualTo(TokenKind.PLUS);
}
@Test
public void testPrecedence5() throws Exception {
BinaryOperatorExpression e =
(BinaryOperatorExpression) parseExpression("2 * x | y + 1");
- assertThat(e.getOperator()).isEqualTo(Operator.PIPE);
+ assertThat(e.getOperator()).isEqualTo(TokenKind.PIPE);
}
@Test
@@ -170,9 +170,9 @@
UnaryOperatorExpression e = (UnaryOperatorExpression) parseExpression("-5");
UnaryOperatorExpression e2 = (UnaryOperatorExpression) parseExpression("- 5");
- IntegerLiteral i = (IntegerLiteral) e.getOperand();
+ IntegerLiteral i = (IntegerLiteral) e.getX();
assertThat(i.getValue()).isEqualTo(5);
- IntegerLiteral i2 = (IntegerLiteral) e2.getOperand();
+ IntegerLiteral i2 = (IntegerLiteral) e2.getX();
assertThat(i2.getValue()).isEqualTo(5);
assertLocation(0, 2, e.getLocation());
assertLocation(0, 3, e2.getLocation());
