bazel syntax: disentangle StarlarkThread and Module
New interpreter API (if you don't use a helper function):
To call a function:
try (Mutability mu = Mutability.create("myexpr")) {
StarlarkThread thread = new StarlarkThread(mu, semantics);
return Starlark.call(thread, fn, args, kwargs);
} catch (EvalException ex) {
...
}
To execute a file:
StarlarkFile file = ...
Module module = Module.create(); // default environment
Resolver.resolve(file, module);
try (Mutability mu = Mutability.create("myfile")) {
StarlarkThread thread = new StarlarkThread(mu, semantics);
Starlark.exec(file, thread, module);
} catch (EvalException ex) {
...
}
// Inv: module contains globals
Overview of change:
- Eliminate the concept of "a Starlark thread's module".
A module is rightly associated with a Starlark function, not a thread.
Consider a thread used just to call an existing function value, for example.
(Previously, a module would always have been created even if unused.)
- Modules are now created explicitly, from a predeclared environment
and a semantics, which is used for filtering but not retained.
Modules can now be created before threads---the logical order.
This simplifies a number of clients.
- Flatten Module. It is no longer a linked list. It contains only
(predeclared, globals, clientData),
and exportedGlobals which will go away soon.
- Simplify processing of FlagGuardedValues. They are either unwrapped
(if enabled by semantics) or left as is, if disabled.
This means they are visible through Module.getPredeclared.
- Delete Module.mutability. It is inessential and raises
questions of consistency with StarlarkThread.
What really matters is whether a module's global values are mutable.
- Delete StarlarkThread.Builder. A simple constructor now suffices:
new StarlarkThread(Mutability, StarlarkSemantics).
- EvaluationTestCase now exposes two hooks for Module and Thread creation
so that tests can predeclare bindings, set client data, and insert
thread local values. Creation of Module and Thread is now fully lazy.
A follow-up change will eliminate the regrettable use of inheritance.
Also:
- Move ModuleCodec into Module, so that we don't need to harm its API.
- Use separate UNIVERSE and predeclared buckets in Module.
The UNIVERSE is always implicitly available.
The API doesn't fully separate them yet (needs Resolver work),
but this should reduce the amount of map copying and redundant
specification.
- Add more pre-evaluated expressions to ParamDescriptor.evalDefault
so that we can bootstrap all the @Param annotation's default values
used by Starlark.UNIVERSE without JVM deadlock. This breaks a cyclic
dependency between the evaluator and UNIVERSE.
- Use composition not inheritance of EvaluationTestCase in more tests.
This is my 6th attempt at this change in as many months.
This is a breaking API change for Copybara.
PiperOrigin-RevId: 312284294
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/StarlarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/StarlarkRuleClassFunctionsTest.java
index 4f67c08..c0bc673 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/StarlarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/StarlarkRuleClassFunctionsTest.java
@@ -61,7 +61,6 @@
import com.google.devtools.build.lib.syntax.ParserInput;
import com.google.devtools.build.lib.syntax.StarlarkFile;
import com.google.devtools.build.lib.syntax.StarlarkList;
-import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.syntax.SyntaxError;
import com.google.devtools.build.lib.syntax.Tuple;
import com.google.devtools.build.lib.syntax.util.EvaluationTestCase;
@@ -772,13 +771,13 @@
private static void evalAndExport(EvaluationTestCase ev, String... lines) throws Exception {
ParserInput input = ParserInput.fromLines(lines);
- StarlarkThread thread = ev.getStarlarkThread();
- Module module = thread.getGlobals();
+ Module module = ev.getModule();
StarlarkFile file = EvalUtils.parseAndValidate(input, FileOptions.DEFAULT, module);
if (!file.ok()) {
throw new SyntaxError.Exception(file.errors());
}
- StarlarkImportLookupFunction.execAndExport(file, FAKE_LABEL, ev.getEventHandler(), thread);
+ StarlarkImportLookupFunction.execAndExport(
+ file, FAKE_LABEL, ev.getEventHandler(), module, ev.getStarlarkThread());
}
@Test
@@ -1385,7 +1384,7 @@
assertThat(EvalUtils.isImmutable(makeStruct("a", makeList(null)))).isTrue();
assertThat(EvalUtils.isImmutable(makeBigStruct(null))).isTrue();
- Mutability mu = ev.getStarlarkThread().mutability();
+ Mutability mu = Mutability.create("test");
assertThat(EvalUtils.isImmutable(Tuple.<Object>of(makeList(mu)))).isFalse();
assertThat(EvalUtils.isImmutable(makeStruct("a", makeList(mu)))).isFalse();
assertThat(EvalUtils.isImmutable(makeBigStruct(mu))).isFalse();
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/util/BazelEvaluationTestCase.java b/src/test/java/com/google/devtools/build/lib/skylark/util/BazelEvaluationTestCase.java
index 4f8f44f..212ff8d 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/util/BazelEvaluationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/util/BazelEvaluationTestCase.java
@@ -18,15 +18,11 @@
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.analysis.skylark.StarlarkModules;
import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.BazelModuleContext;
import com.google.devtools.build.lib.packages.BazelStarlarkContext;
import com.google.devtools.build.lib.packages.SymbolGenerator;
import com.google.devtools.build.lib.rules.platform.PlatformCommon;
-import com.google.devtools.build.lib.syntax.Module;
-import com.google.devtools.build.lib.syntax.Mutability;
import com.google.devtools.build.lib.syntax.Starlark;
-import com.google.devtools.build.lib.syntax.StarlarkSemantics;
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.syntax.util.EvaluationTestCase;
import com.google.devtools.build.lib.testutil.TestConstants;
@@ -39,28 +35,26 @@
// Once the production code API has disentangled Thread and Module, make this rational.
public final class BazelEvaluationTestCase extends EvaluationTestCase {
- // Caution: called by the base class constructor.
@Override
- protected StarlarkThread newStarlarkThread(StarlarkSemantics semantics) {
- ImmutableMap.Builder<String, Object> env = ImmutableMap.builder();
- StarlarkModules.addStarlarkGlobalsToBuilder(env);
- Starlark.addModule(env, new PlatformCommon());
+ protected Object newModuleHook(ImmutableMap.Builder<String, Object> predeclared) {
+ StarlarkModules.addStarlarkGlobalsToBuilder(predeclared);
+ Starlark.addModule(predeclared, new PlatformCommon());
- StarlarkThread thread =
- StarlarkThread.builder(Mutability.create("test"))
- .setSemantics(semantics)
- .setGlobals(
- Module.createForBuiltins(env.build())
- .withClientData(
- BazelModuleContext.create(
- Label.parseAbsoluteUnchecked("//test:label", /*defaultToMain=*/ false),
- /*bzlTransitiveDigest=*/ new byte[0]))) // dummy value for tests
- .build();
- thread.setPrintHandler(Event.makeDebugPrintHandler(getEventHandler()));
+ // Return the module's client data. (This one uses dummy values for tests.)
+ return BazelModuleContext.create(
+ Label.parseAbsoluteUnchecked("//test:label", /*defaultToMain=*/ false),
+ /*bzlTransitiveDigest=*/ new byte[0]);
+ }
+ @Override
+ protected void newThreadHook(StarlarkThread thread) {
// This StarlarkThread has no PackageContext, so attempts to create a rule will fail.
// Rule creation is tested by StarlarkIntegrationTest.
+ // This is a poor approximation to the thread that Blaze would create
+ // for testing rule implementation functions. It has phase LOADING, for example.
+ // TODO(adonovan): stop creating threads in tests. This is the responsibility of the
+ // production code. Tests should provide only files and commands.
new BazelStarlarkContext(
BazelStarlarkContext.Phase.LOADING,
TestConstants.TOOLS_REPOSITORY,
@@ -69,7 +63,5 @@
new SymbolGenerator<>(new Object()),
/*analysisRuleLabel=*/ null) // dummy value for tests
.storeInThread(thread);
-
- return thread;
}
}
