bazel syntax: hide EvalUtils.execToplevelStatement
All file execution now happens through EvalUtils.exec(StarlarkFile)
or EvalUtils.execOrEval (which I plan to eliminate in a followup).
Clients do not execute individual statements.
This requires a temporary hack: StarlarkThread.postAssignHook is a
hook that clients can install to be notified of each assignment at top
level. SkylarkImportLookupFunction uses it to "export" values such as
rules, aspects, and transitions during execution, instead of at the
end of execution. (The behavior will be changed soon; see unknown commit
and b/65374671.)
This may seem like two steps forward and one back, but it will allow us
to develop the EvalUtils API, in particular, turning the top-level statements
of a StarlarkFile into a StarlarkFunction, leading to a consistent treatment
of the call stack.
Also:
- EvalUtils.debugExec is now expressible in terms of the API,
and has been moved up into the debugger.
- Identifier.boundIdentifiers is no longer public.
- Minor simplifications to tests.
PiperOrigin-RevId: 273741627
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
index caace66..09f9395 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkImportLookupFunction.java
@@ -22,7 +22,6 @@
import com.google.common.cache.CacheBuilder;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.actions.InconsistentFilesystemException;
@@ -43,10 +42,8 @@
import com.google.devtools.build.lib.packages.SkylarkExportable;
import com.google.devtools.build.lib.packages.WorkspaceFileValue;
import com.google.devtools.build.lib.skyframe.SkylarkImportLookupValue.SkylarkImportLookupKey;
-import com.google.devtools.build.lib.syntax.AssignmentStatement;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
-import com.google.devtools.build.lib.syntax.Identifier;
import com.google.devtools.build.lib.syntax.LoadStatement;
import com.google.devtools.build.lib.syntax.Mutability;
import com.google.devtools.build.lib.syntax.StarlarkFile;
@@ -620,40 +617,28 @@
public static void execAndExport(
StarlarkFile file, Label extensionLabel, EventHandler handler, StarlarkThread thread)
throws InterruptedException {
- for (Statement stmt : file.getStatements()) {
- try {
- EvalUtils.execToplevelStatement(stmt, thread);
- } catch (EvalException ex) {
- handler.handle(Event.error(ex.getLocation(), ex.getMessage()));
- break;
- }
- possiblyExport(stmt, extensionLabel, handler, thread);
- }
- }
- private static void possiblyExport(
- Statement statement,
- Label extensionLabel,
- EventHandler eventHandler,
- StarlarkThread extensionThread) {
- if (!(statement instanceof AssignmentStatement)) {
- return;
- }
- AssignmentStatement assignmentStatement = (AssignmentStatement) statement;
- ImmutableSet<Identifier> boundIdentifiers =
- Identifier.boundIdentifiers(assignmentStatement.getLHS());
- for (Identifier ident : boundIdentifiers) {
- Object lookup = extensionThread.moduleLookup(ident.getName());
- if (lookup instanceof SkylarkExportable) {
- try {
- SkylarkExportable exportable = (SkylarkExportable) lookup;
- if (!exportable.isExported()) {
- exportable.export(extensionLabel, ident.getName());
+ // Intercept execution after every assignment at top level
+ // and "export" any newly assigned exportable globals.
+ // TODO(adonovan): change the semantics; see b/65374671.
+ thread.setPostAssignHook(
+ (name, value) -> {
+ if (value instanceof SkylarkExportable) {
+ SkylarkExportable exp = (SkylarkExportable) value;
+ if (!exp.isExported()) {
+ try {
+ exp.export(extensionLabel, name);
+ } catch (EvalException ex) {
+ handler.handle(Event.error(ex.getLocation(), ex.getMessage()));
+ }
+ }
}
- } catch (EvalException e) {
- eventHandler.handle(Event.error(e.getLocation(), e.getMessage()));
- }
- }
+ });
+
+ try {
+ EvalUtils.exec(file, thread);
+ } catch (EvalException ex) {
+ handler.handle(Event.error(ex.getLocation(), ex.getMessage()));
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java
index 31ae677..7f662f3 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkdebug/server/ThreadHandler.java
@@ -27,9 +27,12 @@
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
import com.google.devtools.build.lib.syntax.Expression;
+import com.google.devtools.build.lib.syntax.LoadStatement;
import com.google.devtools.build.lib.syntax.ParserInput;
import com.google.devtools.build.lib.syntax.Runtime;
+import com.google.devtools.build.lib.syntax.StarlarkFile;
import com.google.devtools.build.lib.syntax.StarlarkThread;
+import com.google.devtools.build.lib.syntax.Statement;
import com.google.devtools.build.lib.syntax.SyntaxError;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.Collection;
@@ -313,7 +316,7 @@
return thread.debugEval(expr);
} catch (SyntaxError unused) {
// Assume it is a file and execute it.
- thread.debugExec(input);
+ exec(input, thread);
return Runtime.NONE;
}
} finally {
@@ -321,6 +324,21 @@
}
}
+ /** Parses, validates, and executes a Skylark file (sequence of statements) in this thread. */
+ private static void exec(ParserInput input, StarlarkThread thread)
+ throws SyntaxError, EvalException, InterruptedException {
+ StarlarkFile file = EvalUtils.parseAndValidateSkylark(input, thread);
+ if (!file.ok()) {
+ throw new SyntaxError(file.errors());
+ }
+ for (Statement stmt : file.getStatements()) {
+ if (stmt instanceof LoadStatement) {
+ throw new EvalException(stmt.getLocation(), "cannot execute load statements in debugger");
+ }
+ }
+ EvalUtils.exec(file, thread);
+ }
+
/**
* Pauses the current thread's execution, blocking until it's resumed via a
* ContinueExecutionRequest.
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 148295f..107524a 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
@@ -47,6 +47,13 @@
}
}
+ static void execFile(StarlarkThread thread, StarlarkFile file)
+ throws EvalException, InterruptedException {
+ for (Statement stmt : file.getStatements()) {
+ execToplevelStatement(thread, stmt);
+ }
+ }
+
static Object execStatements(StarlarkThread thread, List<Statement> statements)
throws EvalException, InterruptedException {
Eval eval = new Eval(thread);
@@ -59,6 +66,18 @@
// Ignore the returned BREAK/CONTINUE/RETURN/PASS token:
// the first three don't exist at top-level, and the last is a no-op.
new Eval(thread).exec(stmt);
+
+ // Hack for SkylarkImportLookupFunction's "export" semantics.
+ if (thread.postAssignHook != null) {
+ if (stmt instanceof AssignmentStatement) {
+ AssignmentStatement assign = (AssignmentStatement) stmt;
+ for (Identifier id : Identifier.boundIdentifiers(assign.getLHS())) {
+ String name = id.getName();
+ Object value = thread.moduleLookup(name);
+ thread.postAssignHook.assign(name, value);
+ }
+ }
+ }
}
private Eval(StarlarkThread thread) {
@@ -411,9 +430,9 @@
// AND and OR require short-circuit evaluation.
switch (binop.getOperator()) {
case AND:
- return EvalUtils.toBoolean(x) ? Eval.eval(thread, binop.getY()) : x;
+ return EvalUtils.toBoolean(x) ? eval(thread, binop.getY()) : x;
case OR:
- return EvalUtils.toBoolean(x) ? x : Eval.eval(thread, binop.getY());
+ return EvalUtils.toBoolean(x) ? x : eval(thread, binop.getY());
default:
Object y = eval(thread, binop.getY());
return EvalUtils.binaryOp(binop.getOperator(), x, y, thread, binop.getLocation());
@@ -468,13 +487,13 @@
// a closure for x.f if f is a Java method.
if (call.getFunction() instanceof DotExpression) {
DotExpression dot = (DotExpression) call.getFunction();
- Object object = Eval.eval(thread, dot.getObject());
+ Object object = eval(thread, dot.getObject());
evalArguments(thread, call, posargs, kwargs);
return CallUtils.callMethod(
thread, call, object, posargs, kwargs, dot.getField().getName(), dot.getLocation());
}
- Object fn = Eval.eval(thread, call.getFunction());
+ Object fn = eval(thread, call.getFunction());
evalArguments(thread, call, posargs, kwargs);
return CallUtils.call(thread, call, fn, posargs, kwargs);
}
@@ -639,7 +658,7 @@
EvalUtils.lock(iterable, loc);
try {
for (Object elem : listValue) {
- Eval.assign(forClause.getVars(), elem, thread, loc);
+ assign(forClause.getVars(), elem, thread, loc);
execClauses(index + 1);
}
} finally {
@@ -775,7 +794,7 @@
// (O(1) for ImmutableList) to avoid the iterator overhead.
for (int i = 0; i < call.getArguments().size(); i++) {
Argument arg = call.getArguments().get(i);
- Object value = Eval.eval(thread, arg.getValue());
+ Object value = eval(thread, arg.getValue());
if (arg instanceof Argument.Positional) {
// f(expr)
posargs.add(value);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
index d80463e..8ee3592 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvalUtils.java
@@ -1112,15 +1112,7 @@
/** Executes a parsed, validated Starlark file in a given StarlarkThread. */
public static void exec(StarlarkFile file, StarlarkThread thread)
throws EvalException, InterruptedException {
- for (Statement stmt : file.getStatements()) {
- execToplevelStatement(stmt, thread);
- }
- }
-
- /** Executes a statement in a given StarlarkThread. */
- public static void execToplevelStatement(Statement stmt, StarlarkThread thread)
- throws EvalException, InterruptedException {
- Eval.execToplevelStatement(thread, stmt);
+ Eval.execFile(thread, file);
}
/**
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
index 2344873..82cdec4 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Identifier.java
@@ -110,8 +110,7 @@
* <li><{@code x[5] = ..} does not bind any names.
* </ul>
*/
- // TODO(adonovan): make this private after weaning Skyframe off it.
- public static ImmutableSet<Identifier> boundIdentifiers(Expression expr) {
+ static ImmutableSet<Identifier> boundIdentifiers(Expression expr) {
if (expr instanceof Identifier) {
// Common case/fast path - skip the builder.
return ImmutableSet.of((Identifier) expr);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkThread.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkThread.java
index 5bd720a..e1a33ab 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkThread.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkThread.java
@@ -824,6 +824,9 @@
*/
@Nullable private Continuation continuation;
+ /** A hook for notifications of assignments at top level. */
+ PostAssignHook postAssignHook;
+
/**
* Enters a scope by saving state to a new Continuation
*
@@ -1063,6 +1066,23 @@
}
/**
+ * Specifies a hook function to be run after each assignment at top level.
+ *
+ * <p>This is a short-term hack to allow us to consolidate all StarlarkFile execution in one place
+ * even while SkylarkImportLookupFunction implements the old "export" behavior, in which rules,
+ * aspects and providers are "exported" as soon as they are assigned, not at the end of file
+ * execution.
+ */
+ public void setPostAssignHook(PostAssignHook postAssignHook) {
+ this.postAssignHook = postAssignHook;
+ }
+
+ /** A hook for notifications of assignments at top level. */
+ public interface PostAssignHook {
+ void assign(String name, Object value);
+ }
+
+ /**
* Modifies a binding in the current Frame of this StarlarkThread, as would an {@link
* AssignmentStatement}. Does not try to modify an inherited binding. This will shadow any
* inherited binding, which may be an error that you want to guard against before calling this
@@ -1199,22 +1219,6 @@
return Eval.eval(this, expr);
}
- /** Executes a Skylark file (sequence of statements) in this thread. (Debugger API) */
- // TODO(adonovan): push this up into the debugger once the exec API is finalized.
- public void debugExec(ParserInput input) throws SyntaxError, EvalException, InterruptedException {
- StarlarkFile file = StarlarkFile.parse(input);
- ValidationEnvironment.validateFile(file, getGlobals(), getSemantics(), /*isBuildFile=*/ false);
- if (!file.ok()) {
- throw new SyntaxError(file.errors());
- }
- for (Statement stmt : file.getStatements()) {
- if (stmt instanceof LoadStatement) {
- throw new EvalException(stmt.getLocation(), "cannot execute load statements in debugger");
- }
- }
- Eval.execStatements(this, file.getStatements());
- }
-
/**
* Returns the stack frames corresponding of the context's current (paused) state. (Debugger API)
*
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index 1f20b01..f490bf6 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -70,11 +70,9 @@
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-/**
- * Tests for SkylarkRuleClassFunctions.
- */
+/** Tests for SkylarkRuleClassFunctions. */
@RunWith(JUnit4.class)
-public class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
+public final class SkylarkRuleClassFunctionsTest extends SkylarkTestCase {
@Rule public ExpectedException thrown = ExpectedException.none();
@Before
@@ -703,10 +701,10 @@
assertThat(c.hasAttr("a1", Type.STRING)).isTrue();
}
- protected void evalAndExport(String... lines) throws Exception {
+ private void evalAndExport(String... lines) throws Exception {
ParserInput input = ParserInput.fromLines(lines);
StarlarkFile file = EvalUtils.parseAndValidateSkylark(input, ev.getStarlarkThread());
- if (!file.errors().isEmpty()) {
+ if (!file.ok()) {
throw new SyntaxError(file.errors());
}
SkylarkImportLookupFunction.execAndExport(
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/StarlarkThreadDebuggingTest.java b/src/test/java/com/google/devtools/build/lib/syntax/StarlarkThreadDebuggingTest.java
index ffb9f37..1595e7a 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/StarlarkThreadDebuggingTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/StarlarkThreadDebuggingTest.java
@@ -191,9 +191,8 @@
StarlarkThread thread = newStarlarkThread();
thread.update("a", 1);
- Object result = thread.debugEval(Expression.parse(ParserInput.create("a", null)));
-
- assertThat(result).isEqualTo(1);
+ Object a = thread.debugEval(Expression.parse(ParserInput.fromLines("a")));
+ assertThat(a).isEqualTo(1);
}
@Test
@@ -204,7 +203,7 @@
EvalException e =
assertThrows(
EvalException.class,
- () -> thread.debugEval(Expression.parse(ParserInput.create("b", null))));
+ () -> thread.debugEval(Expression.parse(ParserInput.fromLines("b"))));
assertThat(e).hasMessageThat().isEqualTo("name 'b' is not defined");
}
@@ -215,7 +214,8 @@
assertThat(thread.debugEval(Expression.parse(ParserInput.fromLines("a.startswith('str')"))))
.isEqualTo(true);
- thread.debugExec(ParserInput.fromLines("a = 1"));
+ EvalUtils.exec(
+ EvalUtils.parseAndValidateSkylark(ParserInput.fromLines("a = 1"), thread), thread);
assertThat(thread.debugEval(Expression.parse(ParserInput.fromLines("a")))).isEqualTo(1);
}
}
