diff options
11 files changed, 163 insertions, 130 deletions
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 2274076d88..de3bf92c2b 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 @@ -102,7 +102,6 @@ public final class BinaryOperatorExpression extends Expression { } } - /** Helper method. Reused from the LValue class. */ public static Object evaluate( Operator operator, Object lval, diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java index b9c76641f4..8f466b7ff0 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java @@ -1547,23 +1547,6 @@ public class MethodLibrary { } }; - // unary minus - @SkylarkSignature( - name = "-", - returnType = Integer.class, - documented = false, - doc = "Unary minus operator.", - parameters = { - @Param(name = "num", type = Integer.class, doc = "The number to negate.") - } - ) - private static final BuiltinFunction minus = - new BuiltinFunction("-") { - public Integer invoke(Integer num) throws ConversionException { - return -num; - } - }; - @SkylarkSignature( name = "tuple", returnType = Tuple.class, @@ -2221,7 +2204,7 @@ public class MethodLibrary { static final List<BaseFunction> defaultGlobalFunctions = ImmutableList.<BaseFunction>of( all, any, bool, dict, dir, fail, getattr, hasattr, hash, enumerate, int_, len, list, max, - min, minus, print, range, repr, reversed, sorted, str, tuple, zip); + min, print, range, repr, reversed, sorted, str, tuple, zip); static { SkylarkSignatureProcessor.configureSkylarkFunctions(MethodLibrary.class); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/NotExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/NotExpression.java deleted file mode 100644 index 069e92dbcf..0000000000 --- a/src/main/java/com/google/devtools/build/lib/syntax/NotExpression.java +++ /dev/null @@ -1,50 +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; - -/** - * Syntax node for the not boolean operation. - */ -public final class NotExpression extends Expression { - - private final Expression expression; - - public NotExpression(Expression expression) { - this.expression = expression; - } - - public Expression getExpression() { - return expression; - } - - @Override - Object doEval(Environment env) throws EvalException, InterruptedException { - return !EvalUtils.toBoolean(expression.eval(env)); - } - - @Override - public String toString() { - return "not " + expression; - } - - @Override - public void accept(SyntaxTreeVisitor visitor) { - visitor.visit(this); - } - - @Override - void validate(ValidationEnvironment env) throws EvalException { - expression.validate(env); - } -} 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 index b839865ad6..c0740e9dd9 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Operator.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Operator.java @@ -11,11 +11,10 @@ // 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 operators supported by the build language. - */ +/** Infix binary operators. */ public enum Operator { AND("and"), @@ -39,7 +38,7 @@ public enum Operator { private final String name; - private Operator(String name) { + Operator(String name) { this.name = name; } @@ -47,5 +46,4 @@ public enum Operator { 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 46de9a9933..1416d8f0c9 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 @@ -697,10 +697,9 @@ public class Parser { case MINUS: { nextToken(); - List<Argument.Passed> args = new ArrayList<>(); Expression expr = parsePrimaryWithSuffix(); - args.add(setLocation(new Argument.Positional(expr), start, expr)); - return makeFuncallExpression(null, new Identifier("-"), args, start, token.right); + UnaryOperatorExpression minus = new UnaryOperatorExpression(UnaryOperator.MINUS, expr); + return setLocation(minus, start, expr); } default: { @@ -1061,7 +1060,8 @@ public class Parser { int start = token.left; expect(TokenKind.NOT); Expression expression = parseNonTupleExpression(prec + 1); - NotExpression notExpression = new NotExpression(expression); + UnaryOperatorExpression notExpression = + new UnaryOperatorExpression(UnaryOperator.NOT, expression); return setLocation(notExpression, start, token.right); } 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 387268a590..5ef0e177e3 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 @@ -139,8 +139,8 @@ public class SyntaxTreeVisitor { visit(node.getValue()); } - public void visit(NotExpression node) { - visit(node.getExpression()); + public void visit(UnaryOperatorExpression node) { + visit(node.getOperand()); } public void visit(DotExpression node) { 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 new file mode 100644 index 0000000000..85fe9217c9 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperator.java @@ -0,0 +1,34 @@ +// 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 new file mode 100644 index 0000000000..5c36be48f9 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/syntax/UnaryOperatorExpression.java @@ -0,0 +1,83 @@ +// 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; + +import com.google.devtools.build.lib.events.Location; + +/** Syntax node for a unary operator expression. */ +public final class UnaryOperatorExpression extends Expression { + + private final UnaryOperator operator; + + private final Expression operand; + + public UnaryOperatorExpression(UnaryOperator operator, Expression operand) { + this.operator = operator; + this.operand = operand; + } + + public UnaryOperator getOperator() { + return operator; + } + + public Expression getOperand() { + return operand; + } + + @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. + return operator.toString() + operand; + } + + private static Object evaluate( + UnaryOperator operator, + Object value, + Environment env, + Location loc) + throws EvalException, InterruptedException { + switch (operator) { + case NOT: + return !EvalUtils.toBoolean(value); + + case MINUS: + if (!(value instanceof Integer)) { + throw new EvalException( + loc, + String.format("unsupported operand type for -: '%s'", + EvalUtils.getDataTypeName(value))); + } + return -((Integer) value); + + default: + throw new AssertionError("Unsupported unary operator: " + operator); + } + } + + @Override + Object doEval(Environment env) throws EvalException, InterruptedException { + return evaluate(operator, operand.eval(env), env, getLocation()); + } + + @Override + public void accept(SyntaxTreeVisitor visitor) { + visitor.visit(this); + } + + @Override + void validate(ValidationEnvironment env) throws EvalException { + operand.validate(env); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java index 8001f4e469..00319c62cc 100644 --- a/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java +++ b/src/test/java/com/google/devtools/build/lib/syntax/EnvironmentTest.java @@ -122,7 +122,6 @@ public class EnvironmentTest extends EvaluationTestCase { "False", "None", "True", - "-", "all", "any", "bool", @@ -155,7 +154,6 @@ public class EnvironmentTest extends EvaluationTestCase { "False", "None", "True", - "-", "all", "any", "bool", 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 ff5940a95e..606c4ceee8 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 @@ -58,7 +58,9 @@ public class EvaluationTest extends EvaluationTestCase { .testStatement("123 + 456", 579) .testStatement("456 - 123", 333) .testStatement("8 % 3", 2) - .testIfErrorContains("unsupported operand type(s) for %: 'int' and 'string'", "3 % 'foo'"); + .testIfErrorContains("unsupported operand type(s) for %: 'int' and 'string'", "3 % 'foo'") + .testStatement("-5", -5) + .testIfErrorContains("unsupported operand type for -: 'string'", "-'foo'"); } @Test 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 c7bec75162..04eb664d69 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 @@ -58,6 +58,17 @@ public class ParserTest extends EvaluationTestCase { node.getLocation().getEndOffset()); } + private void assertLocation(int start, int end, Location location) + throws Exception { + int actualStart = location.getStartOffset(); + int actualEnd = location.getEndOffset(); + + if (actualStart != start || actualEnd != end) { + fail("Expected location = [" + start + ", " + end + "), found [" + + actualStart + ", " + actualEnd + ")"); + } + } + // helper func for testListLiterals: private static int getIntElem(DictionaryEntryLiteral entry, boolean key) { return ((IntegerLiteral) (key ? entry.getKey() : entry.getValue())).getValue(); @@ -150,17 +161,15 @@ public class ParserTest extends EvaluationTestCase { @Test public void testUnaryMinusExpr() throws Exception { - FuncallExpression e = (FuncallExpression) parseExpression("-5"); - FuncallExpression e2 = (FuncallExpression) parseExpression("- 5"); - - assertThat(e.getFunction().getName()).isEqualTo("-"); - assertThat(e2.getFunction().getName()).isEqualTo("-"); - - assertThat(e.getArguments()).hasSize(1); - assertThat(e.getNumPositionalArguments()).isEqualTo(1); + UnaryOperatorExpression e = (UnaryOperatorExpression) parseExpression("-5"); + UnaryOperatorExpression e2 = (UnaryOperatorExpression) parseExpression("- 5"); - IntegerLiteral arg0 = (IntegerLiteral) e.getArguments().get(0).getValue(); - assertThat((int) arg0.getValue()).isEqualTo(5); + IntegerLiteral i = (IntegerLiteral) e.getOperand(); + assertThat(i.getValue()).isEqualTo(5); + IntegerLiteral i2 = (IntegerLiteral) e2.getOperand(); + assertThat(i2.getValue()).isEqualTo(5); + assertLocation(0, 2, e.getLocation()); + assertLocation(0, 3, e2.getLocation()); } @Test @@ -268,6 +277,14 @@ public class ParserTest extends EvaluationTestCase { } @Test + public void testIndex() throws Exception { + IndexExpression e = (IndexExpression) parseExpression("a[i]"); + assertThat(e.getObject().toString()).isEqualTo("a"); + assertThat(e.getKey().toString()).isEqualTo("i"); + assertLocation(0, 4, e.getLocation()); + } + + @Test public void testSubstring() throws Exception { SliceExpression s = (SliceExpression) parseExpression("'FOO.CC'[:].lower()[1:]"); assertThat(((IntegerLiteral) s.getStart()).value).isEqualTo(1); @@ -294,37 +311,18 @@ public class ParserTest extends EvaluationTestCase { evalSlice("'0123'[1::-1]", 1, Runtime.NONE, -1); evalSlice("'0123'[:3:-1]", Runtime.NONE, 3, -1); evalSlice("'0123'[1:3:-1]", 1, 3, -1); + + Expression slice = parseExpression("'0123'[1:3:-1]"); + assertLocation(0, 14, slice.getLocation()); } private void evalSlice(String statement, Object... expectedArgs) { SliceExpression e = (SliceExpression) parseExpression(statement); // There is no way to evaluate the expression here, so we rely on string comparison. - assertThat(e.getStart().toString()).isEqualTo(printSliceArg(expectedArgs[0])); - assertThat(e.getEnd().toString()).isEqualTo(printSliceArg(expectedArgs[1])); - assertThat(e.getStep().toString()).isEqualTo(printSliceArg(expectedArgs[2])); - } - - private String printSliceArg(Object arg) { - // The parser sees negative integer constants as FuncallExpressions instead of negative - // IntegerLiterals. - // Consequently, the string representation of -1 is "-(1)", not "-1". - if (arg instanceof Integer) { - int value = (int) arg; - return value < 0 ? String.format("-(%d)", -value) : String.valueOf(value); - } - return arg.toString(); - } - - private void assertLocation(int start, int end, Location location) - throws Exception { - int actualStart = location.getStartOffset(); - int actualEnd = location.getEndOffset(); - - if (actualStart != start || actualEnd != end) { - fail("Expected location = [" + start + ", " + end + "), found [" - + actualStart + ", " + actualEnd + ")"); - } + assertThat(e.getStart().toString()).isEqualTo(expectedArgs[0].toString()); + assertThat(e.getEnd().toString()).isEqualTo(expectedArgs[1].toString()); + assertThat(e.getStep().toString()).isEqualTo(expectedArgs[2].toString()); } @Test @@ -484,18 +482,6 @@ public class ParserTest extends EvaluationTestCase { } @Test - public void testSpecialFuncallLocation() throws Exception { - List<Statement> statements = parseFile("-x\n"); - assertLocation(0, 3, statements.get(0).getLocation()); - - statements = parseFile("arr[15]\n"); - assertLocation(0, 8, statements.get(0).getLocation()); - - statements = parseFile("str[1:12]\n"); - assertLocation(0, 10, statements.get(0).getLocation()); - } - - @Test public void testListPositions() throws Exception { String expr = "[0,f(1),2]"; ListLiteral list = (ListLiteral) parseExpression(expr); |