Compile assignments to byte code and throw errors.

Add EvalExceptions for cases in which the interpreter would throw them
during evaluation of the function definition.

--
MOS_MIGRATED_REVID=107376021

7 files changed, 150 insertions, 8 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 f731ed54b1..e3dc7d1788 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
@@ -14,6 +14,13 @@
 
 package com.google.devtools.build.lib.syntax;
 
+import com.google.common.base.Optional;
+import com.google.devtools.build.lib.syntax.compiler.DebugInfo;
+import com.google.devtools.build.lib.syntax.compiler.LoopLabels;
+import com.google.devtools.build.lib.syntax.compiler.VariableScope;
+
+import net.bytebuddy.implementation.bytecode.ByteCodeAppender;
+
 /**
  * Syntax node for an assignment statement.
  */
@@ -66,4 +73,13 @@ public final class AssignmentStatement extends Statement {
     expression.validate(env);
     lvalue.validate(env, getLocation());
   }
+
+  @Override
+  ByteCodeAppender compile(
+      VariableScope scope, Optional<LoopLabels> loopLabels, DebugInfo debugInfo)
+          throws EvalException {
+    return new ByteCodeAppender.Compound(
+        expression.compile(scope, debugInfo),
+        lvalue.compileAssignment(this, debugInfo.add(this), scope));
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Expression.java b/src/main/java/com/google/devtools/build/lib/syntax/Expression.java
index 76c2d66894..5e6a716ac3 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Expression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Expression.java
@@ -75,8 +75,12 @@ public abstract class Expression extends ASTNode {
   /**
    * Builds a {@link ByteCodeAppender} that implements this expression by consuming its operands
    * from the byte code stack and pushing its result.
+   *
+   * @throws EvalException for any error that would have occurred during evaluation of the
+   *    function definition that contains this statement, e.g. type errors.
    */
-  ByteCodeAppender compile(VariableScope scope, DebugInfo debugInfo) {
+  ByteCodeAppender compile(VariableScope scope, DebugInfo debugInfo)
+      throws EvalException {
     throw new UnsupportedOperationException(this.getClass().getSimpleName() + " unsupported.");
   }
 }
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 7fe31872dc..1b6cf65ec5 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
@@ -14,11 +14,24 @@
 
 package com.google.devtools.build.lib.syntax;
 
+import static com.google.devtools.build.lib.syntax.compiler.ByteCodeUtils.append;
+
 import com.google.common.base.Preconditions;
 import com.google.devtools.build.lib.events.Location;
+import com.google.devtools.build.lib.syntax.compiler.ByteCodeUtils;
+import com.google.devtools.build.lib.syntax.compiler.DebugInfo.AstAccessors;
+import com.google.devtools.build.lib.syntax.compiler.Variable.InternalVariable;
+import com.google.devtools.build.lib.syntax.compiler.VariableScope;
+
+import net.bytebuddy.implementation.bytecode.ByteCodeAppender;
+import net.bytebuddy.implementation.bytecode.Removal;
+import net.bytebuddy.implementation.bytecode.constant.IntegerConstant;
 
 import java.io.Serializable;
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
 
 /**
  * Class representing an LValue.
@@ -121,4 +134,103 @@ public class LValue implements Serializable {
   public String toString() {
     return expr.toString();
   }
+
+  /**
+   * Compile an assignment within the given ASTNode to these l-values.
+   *
+   * <p>The value to possibly destructure and assign must already be on the stack.
+   */
+  public ByteCodeAppender compileAssignment(ASTNode node, AstAccessors debugAccessors,
+      VariableScope scope) throws EvalException {
+    List<ByteCodeAppender> code = new ArrayList<>();
+    compileAssignment(node, debugAccessors, expr, scope, code);
+    return ByteCodeUtils.compoundAppender(code);
+  }
+
+  /**
+   * Called recursively to compile the tree of l-values we might have.
+   */
+  private static void compileAssignment(
+      ASTNode node,
+      AstAccessors debugAccessors,
+      Expression leftValue,
+      VariableScope scope,
+      List<ByteCodeAppender> code) throws EvalException {
+    if (leftValue instanceof Identifier) {
+      code.add(compileAssignment(scope, (Identifier) leftValue));
+    } else if (leftValue instanceof ListLiteral) {
+      List<Expression> lValueExpressions = ((ListLiteral) leftValue).getElements();
+      compileAssignment(node, debugAccessors, scope, lValueExpressions, code);
+    } else {
+      String message = String.format(
+          "Can't assign to expression '%s', only to variables or nested tuples of variables",
+          leftValue);
+      throw new EvalExceptionWithStackTrace(
+          new EvalException(
+              node.getLocation(),
+              message),
+              node);
+    }
+  }
+
+  /**
+   * Assumes a collection of values on the top of the stack and assigns them to the l-value
+   * expressions given.
+   */
+  private static void compileAssignment(
+      ASTNode node,
+      AstAccessors debugAccessors,
+      VariableScope scope,
+      List<Expression> lValueExpressions,
+      List<ByteCodeAppender> code) throws EvalException {
+    InternalVariable objects = scope.freshVariable(Collection.class);
+    InternalVariable iterator = scope.freshVariable(Iterator.class);
+    // convert the object on the stack into a collection and store it to a variable for loading
+    // multiple times below below
+    code.add(new ByteCodeAppender.Simple(debugAccessors.loadLocation, EvalUtils.toCollection));
+    code.add(objects.store());
+    append(code,
+        // check that we got exactly the amount of objects in the collection that we need
+        IntegerConstant.forValue(lValueExpressions.size()),
+        objects.load(),
+        debugAccessors.loadLocation, // TODO(bazel-team) load better location within tuple
+        ByteCodeUtils.invoke(
+            LValue.class, "checkSize", int.class, Collection.class, Location.class),
+        // get an iterator to assign the objects
+        objects.load(),
+        ByteCodeUtils.invoke(Collection.class, "iterator"));
+    code.add(iterator.store());
+    // assign each object to the corresponding l-value
+    for (Expression lValue : lValueExpressions) {
+      code.add(
+          new ByteCodeAppender.Simple(
+              iterator.load(), ByteCodeUtils.invoke(Iterator.class, "next")));
+      compileAssignment(node, debugAccessors, lValue, scope, code);
+    }
+  }
+
+  /**
+   * Compile assignment to a single identifier.
+   */
+  private static ByteCodeAppender compileAssignment(VariableScope scope, Identifier identifier) {
+    // don't store to/create the _ "variable" the value is not needed, just remove it
+    if (identifier.getName().equals("_")) {
+      return new ByteCodeAppender.Simple(Removal.SINGLE);
+    }
+    return scope.getVariable(identifier).store();
+  }
+
+  /**
+   * Checks that the size of a collection at runtime conforms to the amount of l-value expressions
+   * we have to assign to.
+   */
+  public static void checkSize(int expected, Collection<?> collection, Location location)
+      throws EvalException {
+    int actual = collection.size();
+    if (expected != actual) {
+      throw new EvalException(
+          location,
+          String.format("lvalue has length %d, but rvalue has has length %d", expected, actual));
+    }
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java b/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
index 18484cccb9..e3447fca83 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/ListLiteral.java
@@ -118,7 +118,7 @@ public final class ListLiteral extends Expression {
   }
 
   @Override
-  ByteCodeAppender compile(VariableScope scope, DebugInfo debugInfo) {
+  ByteCodeAppender compile(VariableScope scope, DebugInfo debugInfo) throws EvalException {
     AstAccessors debugAccessors = debugInfo.add(this);
     List<ByteCodeAppender> listConstruction = new ArrayList<>();
     if (isTuple()) {
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 82c0c97ef7..c1161554d0 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
@@ -83,7 +83,8 @@ public class ReturnStatement extends Statement {
 
   @Override
   ByteCodeAppender compile(
-      VariableScope scope, Optional<LoopLabels> loopLabels, DebugInfo debugInfo) {
+      VariableScope scope, Optional<LoopLabels> loopLabels, DebugInfo debugInfo)
+          throws EvalException {
     ByteCodeAppender compiledExpression = returnExpression.compile(scope, debugInfo);
     return new ByteCodeAppender.Compound(
         compiledExpression, new ByteCodeAppender.Simple(MethodReturn.REFERENCE));
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 3db9b1cdd5..0d8498ea8c 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
@@ -72,9 +72,13 @@ public abstract class Statement extends ASTNode {
    *
    * <p>A statement implementation should never require any particular state of the byte code
    * stack and should leave it in the state it was before.
+   *
+   * @throws EvalException for any error that would have occurred during evaluation of the
+   *    function definition that contains this statement, e.g. type errors.
    */
   ByteCodeAppender compile(
-      VariableScope scope, Optional<LoopLabels> loopLabels, DebugInfo debugInfo) {
+      VariableScope scope, Optional<LoopLabels> loopLabels, DebugInfo debugInfo)
+          throws EvalException {
     throw new UnsupportedOperationException(this.getClass().getSimpleName() + " unsupported.");
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
index cf68aeb6f4..ededb16999 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
@@ -81,7 +81,8 @@ public class UserDefinedFunction extends BaseFunction {
 
   protected UserDefinedFunction(Identifier function,
       FunctionSignature.WithValues<Object, SkylarkType> signature,
-      ImmutableList<Statement> statements, Environment.Frame definitionGlobals) {
+      ImmutableList<Statement> statements, Environment.Frame definitionGlobals)
+    throws EvalException {
     super(function.getName(), signature, function.getLocation());
     this.statements = statements;
     this.definitionGlobals = definitionGlobals;
@@ -174,7 +175,7 @@ public class UserDefinedFunction extends BaseFunction {
    *
    * <p>The "call" method contains the compiled version of this function's AST.
    */
-  private Optional<Method> buildCompiledFunction() {
+  private Optional<Method> buildCompiledFunction() throws EvalException {
     // replace the / character in the path so we have file system compatible class names
     // the java specification mentions that $ should be used in generated code
     // see http://docs.oracle.com/javase/specs/jls/se7/html/jls-3.html#jls-3.8
@@ -253,11 +254,14 @@ public class UserDefinedFunction extends BaseFunction {
                   "call",
                   parameterTypes.toArray(new Class<?>[parameterTypes.size()]))
               .getLoadedMethod());
+    } catch (EvalException e) {
+      // don't capture EvalExceptions
+      throw e;
     } catch (Throwable e) {
       compilerDebug("Error while compiling", e);
       // TODO(bazel-team) don't capture all throwables? couldn't compile this, log somewhere?
-      return Optional.absent();
     }
+    return Optional.absent();
   }
 
   /**
@@ -280,7 +284,8 @@ public class UserDefinedFunction extends BaseFunction {
   /**
    * Builds a byte code implementation of the AST.
    */
-  private Implementation compileBody(VariableScope scope, DebugInfo debugInfo) {
+  private Implementation compileBody(VariableScope scope, DebugInfo debugInfo)
+      throws EvalException {
     List<ByteCodeAppender> code = new ArrayList<>(statements.size());
     code.add(null); // reserve space for later addition of the local variable initializer