diff options
11 files changed, 224 insertions, 172 deletions
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 facf5dd39d..b21ea1721e 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 @@ -57,12 +57,6 @@ public final class AssignmentStatement extends Statement { } @Override - void doExec(Environment env) throws EvalException, InterruptedException { - Object rvalue = expression.eval(env); - lvalue.assign(rvalue, env, getLocation()); - } - - @Override public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } 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 25203489ec..ec23b20f86 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 @@ -59,11 +59,6 @@ public final class AugmentedAssignmentStatement extends Statement { } @Override - void doExec(Environment env) throws EvalException, InterruptedException { - lvalue.assignAugmented(operator, expression, env, getLocation()); - } - - @Override public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } 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 new file mode 100644 index 0000000000..d4c6edb0da --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/syntax/Eval.java @@ -0,0 +1,211 @@ +// 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 java.util.ArrayList; +import java.util.List; +import java.util.Map; + +/** + * Evaluation code for the Skylark AST. At the moment, it can execute only statements (and defers to + * Expression.eval for evaluating expressions). + */ +public class Eval { + private final Environment env; + + /** An exception that signals changes in the control flow (e.g. break or continue) */ + private static class FlowException extends EvalException { + FlowException(String message) { + super(null, message); + } + + @Override + public boolean canBeAddedToStackTrace() { + return false; + } + } + + private static final FlowException breakException = new FlowException("FlowException - break"); + private static final FlowException continueException = + new FlowException("FlowException - continue"); + + public Eval(Environment env) { + this.env = env; + } + + void execAssignment(AssignmentStatement node) throws EvalException, InterruptedException { + Object rvalue = node.getExpression().eval(env); + node.getLValue().assign(rvalue, env, node.getLocation()); + } + + void execAugmentedAssignment(AugmentedAssignmentStatement node) + throws EvalException, InterruptedException { + node.getLValue() + .assignAugmented(node.getOperator(), node.getExpression(), env, node.getLocation()); + } + + void execIfBranch(IfStatement.ConditionalStatements node) + throws EvalException, InterruptedException { + for (Statement stmt : node.getStatements()) { + exec(stmt); + } + } + + void execFor(ForStatement node) throws EvalException, InterruptedException { + Object o = node.getCollection().eval(env); + Iterable<?> col = EvalUtils.toIterable(o, node.getLocation(), env); + EvalUtils.lock(o, node.getLocation()); + try { + for (Object it : col) { + node.getVariable().assign(it, env, node.getLocation()); + + try { + for (Statement stmt : node.getBlock()) { + stmt.exec(env); + } + } catch (FlowException ex) { + if (ex == breakException) { + return; + } + } + } + } finally { + EvalUtils.unlock(o, node.getLocation()); + } + } + + void execDef(FunctionDefStatement node) throws EvalException, InterruptedException { + List<Expression> defaultExpressions = node.getSignature().getDefaultValues(); + ArrayList<Object> defaultValues = null; + + if (defaultExpressions != null) { + defaultValues = new ArrayList<>(defaultExpressions.size()); + for (Expression expr : defaultExpressions) { + defaultValues.add(expr.eval(env)); + } + } + + FunctionSignature sig = node.getSignature().getSignature(); + if (env.getSemantics().incompatibleDisallowKeywordOnlyArgs + && sig.getShape().getMandatoryNamedOnly() > 0) { + throw new EvalException( + node.getLocation(), + "Keyword-only argument is forbidden. You can temporarily disable this " + + "error using the flag --incompatible_disallow_keyword_only_args=false"); + } + + env.update( + node.getIdentifier().getName(), + new UserDefinedFunction( + node.getIdentifier().getName(), + node.getIdentifier().getLocation(), + FunctionSignature.WithValues.create(sig, defaultValues, /*types=*/ null), + node.getStatements(), + env.getGlobals())); + } + + void execIf(IfStatement node) throws EvalException, InterruptedException { + for (IfStatement.ConditionalStatements stmt : node.getThenBlocks()) { + if (EvalUtils.toBoolean(stmt.getCondition().eval(env))) { + exec(stmt); + return; + } + } + for (Statement stmt : node.getElseBlock()) { + exec(stmt); + } + } + + void execLoad(LoadStatement node) throws EvalException, InterruptedException { + if (env.getSemantics().incompatibleLoadArgumentIsLabel) { + String s = node.getImport().getValue(); + if (!s.startsWith("//") && !s.startsWith(":")) { + throw new EvalException( + node.getLocation(), + "First argument of 'load' must be a label and start with either '//' or ':'. " + + "Use --incompatible_load_argument_is_label=false to temporarily disable this " + + "check."); + } + } + + for (Map.Entry<Identifier, String> entry : node.getSymbolMap().entrySet()) { + try { + Identifier name = entry.getKey(); + Identifier declared = new Identifier(entry.getValue()); + + if (declared.isPrivate()) { + throw new EvalException( + node.getLocation(), + "symbol '" + declared.getName() + "' is private and cannot be imported."); + } + // The key is the original name that was used to define the symbol + // in the loaded bzl file. + env.importSymbol(node.getImport().getValue(), name, declared.getName()); + } catch (Environment.LoadFailedException e) { + throw new EvalException(node.getLocation(), e.getMessage()); + } + } + } + + void execReturn(ReturnStatement node) throws EvalException, InterruptedException { + Expression ret = node.getReturnExpression(); + if (ret == null) { + throw new ReturnStatement.ReturnException(node.getLocation(), Runtime.NONE); + } + throw new ReturnStatement.ReturnException(ret.getLocation(), ret.eval(env)); + } + + /** + * Execute the statement. + * + * @throws EvalException if execution of the statement could not be completed. + * @throws InterruptedException may be thrown in a sub class. + */ + public void exec(Statement st) throws EvalException, InterruptedException { + switch (st.kind()) { + case ASSIGNMENT: + execAssignment((AssignmentStatement) st); + break; + case AUGMENTED_ASSIGNMENT: + execAugmentedAssignment((AugmentedAssignmentStatement) st); + break; + case CONDITIONAL: + execIfBranch((IfStatement.ConditionalStatements) st); + break; + case EXPRESSION: + ((ExpressionStatement) st).getExpression().eval(env); + break; + case FLOW: + throw ((FlowStatement) st).getKind() == FlowStatement.Kind.BREAK + ? breakException + : continueException; + case FOR: + execFor((ForStatement) st); + break; + case FUNCTION_DEF: + execDef((FunctionDefStatement) st); + break; + case IF: + execIf((IfStatement) st); + break; + case LOAD: + execLoad((LoadStatement) st); + break; + case RETURN: + execReturn((ReturnStatement) st); + break; + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java index 7367fdc584..54f4fb72a5 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/ExpressionStatement.java @@ -39,11 +39,6 @@ public final class ExpressionStatement extends Statement { } @Override - void doExec(Environment env) throws EvalException, InterruptedException { - expression.eval(env); - } - - @Override public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java index e2d5344de4..1022caf1d6 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/FlowStatement.java @@ -29,10 +29,13 @@ public final class FlowStatement extends Statement { private Kind(String name) { this.name = name; } + + public String getName() { + return name; + } } private final Kind kind; - private final FlowException ex; /** * @@ -40,7 +43,6 @@ public final class FlowStatement extends Statement { */ public FlowStatement(Kind kind) { this.kind = kind; - this.ex = new FlowException(kind); } public Kind getKind() { @@ -48,11 +50,6 @@ public final class FlowStatement extends Statement { } @Override - void doExec(Environment env) throws EvalException { - throw ex; - } - - @Override public void prettyPrint(Appendable buffer, int indentLevel) throws IOException { printIndent(buffer, indentLevel); buffer.append(kind.name); @@ -73,30 +70,4 @@ public final class FlowStatement extends Statement { public Statement.Kind kind() { return Statement.Kind.FLOW; } - - /** - * An exception that signals changes in the control flow (e.g. break or continue) - */ - class FlowException extends EvalException { - private final Kind kind; - - public FlowException(Kind kind) { - super(FlowStatement.this.getLocation(), "FlowException with kind = " + kind.name); - this.kind = kind; - } - - /** - * Returns whether the enclosing loop should be terminated completely (break) - * - * @return {@code True} for 'break', {@code false} for 'continue' - */ - public boolean mustTerminateLoop() { - return kind == Kind.BREAK; - } - - @Override - public boolean canBeAddedToStackTrace() { - return false; - } - } } 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 32a1d26b7f..f2859355f1 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 @@ -14,7 +14,6 @@ package com.google.devtools.build.lib.syntax; import com.google.common.collect.ImmutableList; -import com.google.devtools.build.lib.syntax.FlowStatement.FlowException; import com.google.devtools.build.lib.util.Preconditions; import java.io.IOException; import java.util.List; @@ -69,30 +68,6 @@ public final class ForStatement extends Statement { } @Override - void doExec(Environment env) throws EvalException, InterruptedException { - Object o = collection.eval(env); - Iterable<?> col = EvalUtils.toIterable(o, getLocation(), env); - EvalUtils.lock(o, getLocation()); - try { - for (Object it : col) { - variable.assign(it, env, getLocation()); - - try { - for (Statement stmt : block) { - stmt.exec(env); - } - } catch (FlowException ex) { - if (ex.mustTerminateLoop()) { - return; - } - } - } - } finally { - EvalUtils.unlock(o, getLocation()); - } - } - - @Override public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java index 544870e0c7..0f27ada367 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/FunctionDefStatement.java @@ -15,8 +15,6 @@ package com.google.devtools.build.lib.syntax; import com.google.common.collect.ImmutableList; import java.io.IOException; -import java.util.ArrayList; -import java.util.List; /** * Syntax node for a function definition. @@ -39,37 +37,6 @@ public final class FunctionDefStatement extends Statement { } @Override - void doExec(Environment env) throws EvalException, InterruptedException { - List<Expression> defaultExpressions = signature.getDefaultValues(); - ArrayList<Object> defaultValues = null; - - if (defaultExpressions != null) { - defaultValues = new ArrayList<>(defaultExpressions.size()); - for (Expression expr : defaultExpressions) { - defaultValues.add(expr.eval(env)); - } - } - - FunctionSignature sig = signature.getSignature(); - if (env.getSemantics().incompatibleDisallowKeywordOnlyArgs - && sig.getShape().getMandatoryNamedOnly() > 0) { - throw new EvalException( - getLocation(), - "Keyword-only argument is forbidden. You can temporarily disable this " - + "error using the flag --incompatible_disallow_keyword_only_args=false"); - } - - env.update( - identifier.getName(), - new UserDefinedFunction( - identifier.getName(), - identifier.getLocation(), - FunctionSignature.WithValues.create(sig, defaultValues, /*types=*/null), - statements, - env.getGlobals())); - } - - @Override public void prettyPrint(Appendable buffer, int indentLevel) throws IOException { printIndent(buffer, indentLevel); buffer.append("def "); diff --git a/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java index c49613da85..f2f5c4bb15 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/IfStatement.java @@ -26,8 +26,8 @@ public final class IfStatement extends Statement { /** * Syntax node for an [el]if statement. * - * <p>This extends Statement because it implements {@code doExec}, but it is not actually an - * independent statement in the grammar. + * <p>This extends Statement, but it is not actually an independent statement in the grammar. We + * should probably eliminate it in favor of a recursive representation of if/else chains. */ public static final class ConditionalStatements extends Statement { @@ -39,13 +39,6 @@ public final class IfStatement extends Statement { this.statements = ImmutableList.copyOf(statements); } - @Override - void doExec(Environment env) throws EvalException, InterruptedException { - for (Statement stmt : statements) { - stmt.exec(env); - } - } - // No prettyPrint function; handled directly by IfStatement#prettyPrint. @Override public void prettyPrint(Appendable buffer, int indentLevel) throws IOException { @@ -122,19 +115,6 @@ public final class IfStatement extends Statement { } @Override - void doExec(Environment env) throws EvalException, InterruptedException { - for (ConditionalStatements stmt : thenBlocks) { - if (EvalUtils.toBoolean(stmt.getCondition().eval(env))) { - stmt.exec(env); - return; - } - } - for (Statement stmt : elseBlock) { - stmt.exec(env); - } - } - - @Override public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java index 084ffa81b0..180802141d 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/LoadStatement.java @@ -73,37 +73,6 @@ public final class LoadStatement extends Statement { } @Override - void doExec(Environment env) throws EvalException, InterruptedException { - if (env.getSemantics().incompatibleLoadArgumentIsLabel) { - String s = imp.getValue(); - if (!s.startsWith("//") && !s.startsWith(":")) { - throw new EvalException( - getLocation(), - "First argument of 'load' must be a label and start with either '//' or ':'. " - + "Use --incompatible_load_argument_is_label=false to temporarily disable this " - + "check."); - } - } - - for (Map.Entry<Identifier, String> entry : symbolMap.entrySet()) { - try { - Identifier name = entry.getKey(); - Identifier declared = new Identifier(entry.getValue()); - - if (declared.isPrivate()) { - throw new EvalException(getLocation(), - "symbol '" + declared.getName() + "' is private and cannot be imported."); - } - // The key is the original name that was used to define the symbol - // in the loaded bzl file. - env.importSymbol(imp.getValue(), name, declared.getName()); - } catch (Environment.LoadFailedException e) { - throw new EvalException(getLocation(), e.getMessage()); - } - } - } - - @Override public void accept(SyntaxTreeVisitor visitor) { visitor.visit(this); } diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java b/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java index 83661192dc..10b1fd26fa 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/ReturnStatement.java @@ -53,14 +53,6 @@ public final class ReturnStatement extends Statement { this.returnExpression = returnExpression; } - @Override - void doExec(Environment env) throws EvalException, InterruptedException { - if (returnExpression == null) { - throw new ReturnException(getLocation(), Runtime.NONE); - } - throw new ReturnException(returnExpression.getLocation(), returnExpression.eval(env)); - } - @Nullable public Expression getReturnExpression() { return returnExpression; diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Statement.java b/src/main/java/com/google/devtools/build/lib/syntax/Statement.java index 1795af1733..b8505c3aee 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Statement.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Statement.java @@ -37,13 +37,13 @@ public abstract class Statement extends ASTNode { } /** - * Executes the statement in the specified build environment, which may be - * modified. + * Executes the statement in the specified build environment, which may be modified. * * @throws EvalException if execution of the statement could not be completed. * @throws InterruptedException may be thrown in a sub class. */ - final void exec(Environment env) throws EvalException, InterruptedException { + @Deprecated // use Eval class instead + final void exec(Environment env) throws EvalException, InterruptedException { try { doExec(env); } catch (EvalException ex) { @@ -60,7 +60,10 @@ public abstract class Statement extends ASTNode { * @throws EvalException if execution of the statement could not be completed. * @throws InterruptedException may be thrown in a sub class. */ - abstract void doExec(Environment env) throws EvalException, InterruptedException; + @Deprecated + final void doExec(Environment env) throws EvalException, InterruptedException { + new Eval(env).exec(this); + } /** * Kind of the statement. This is similar to using instanceof, except that it's more efficient and |