Remove isSkylark and eval from Environment.
--
MOS_MIGRATED_REVID=140371603
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index 837bad3..53db2a2 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
@@ -727,7 +727,6 @@
Map<String, Extension> importMap) {
Environment env =
Environment.builder(mutability)
- .setSkylark()
.setGlobals(globals)
.setEventHandler(eventHandler)
.setFileContentHashCode(astFileContentHashCode)
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
index f02f33a..3fd7c20 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/skylark/SkylarkRepositoryFunction.java
@@ -93,7 +93,6 @@
com.google.devtools.build.lib.syntax.Environment buildEnv =
com.google.devtools.build.lib.syntax.Environment.builder(mutability)
.setGlobals(rule.getRuleClassObject().getRuleDefinitionEnvironment().getGlobals())
- .setSkylark()
.setEventHandler(env.getListener())
.build();
SkylarkRepositoryContext skylarkRepositoryContext = new SkylarkRepositoryContext(
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java
index 3f8158a..1a66a33 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkModules.java
@@ -56,8 +56,7 @@
private static Environment.Frame createGlobals(List<Class<?>> modules) {
try (Mutability mutability = Mutability.create("SkylarkModules")) {
- Environment env =
- Environment.builder(mutability).setSkylark().setGlobals(BazelLibrary.GLOBALS).build();
+ Environment env = Environment.builder(mutability).setGlobals(BazelLibrary.GLOBALS).build();
for (Class<?> moduleClass : modules) {
Runtime.registerModuleGlobals(env, moduleClass);
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
index 84f6a5f..f78e96d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleConfiguredTargetBuilder.java
@@ -75,7 +75,6 @@
SkylarkRuleContext skylarkRuleContext = new SkylarkRuleContext(ruleContext,
null);
Environment env = Environment.builder(mutability)
- .setSkylark()
.setCallerLabel(ruleContext.getLabel())
.setGlobals(
ruleContext.getRule().getRuleClassObject().getRuleDefinitionEnvironment().getGlobals())
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
index 7a058fd..f078c3a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkylarkAspectFactory.java
@@ -59,7 +59,6 @@
}
Environment env =
Environment.builder(mutability)
- .setSkylark()
.setGlobals(skylarkAspect.getFuncallEnv().getGlobals())
.setEventHandler(ruleContext.getAnalysisEnvironment().getEventHandler())
.build(); // NB: loading phase functions are not available: this is analysis already,
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
index 473c54c..d0efaca 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BuildFileAST.java
@@ -339,6 +339,14 @@
return last;
}
+ public static Object eval(Environment env, String... input)
+ throws EvalException, InterruptedException {
+ BuildFileAST ast = parseSkylarkString(env.getEventHandler(), input);
+ ValidationEnvironment valid = new ValidationEnvironment(env);
+ valid.validateAst(ast.getStatements(), env.getEventHandler());
+ return ast.eval(env);
+ }
+
/**
* Returns a hash code calculated from the string content of the source file of this AST.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
index 5ff9a81..11515ef 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Environment.java
@@ -36,37 +36,37 @@
import javax.annotation.Nullable;
/**
- * An Environment is the main entry point to evaluating code in the BUILD language or Skylark.
- * It embodies all the state that is required to evaluate such code,
- * except for the current instruction pointer, which is an {@link ASTNode}
- * whose {@link Statement#exec exec} or {@link Expression#eval eval} method is invoked with
- * this Environment, in a straightforward direct-style AST-walking interpreter.
- * {@link Continuation}-s are explicitly represented, but only partly, with another part being
- * implicit in a series of try-catch statements, to maintain the direct style. One notable trick
- * is how a {@link UserDefinedFunction} implements returning values as the function catching a
- * {@link ReturnStatement.ReturnException} thrown by a {@link ReturnStatement} in the body.
+ * An Environment is the main entry point to evaluating code in the BUILD language or Skylark. It
+ * embodies all the state that is required to evaluate such code, except for the current instruction
+ * pointer, which is an {@link ASTNode} whose {@link Statement#exec exec} or {@link Expression#eval
+ * eval} method is invoked with this Environment, in a straightforward direct-style AST-walking
+ * interpreter. {@link Continuation}-s are explicitly represented, but only partly, with another
+ * part being implicit in a series of try-catch statements, to maintain the direct style. One
+ * notable trick is how a {@link UserDefinedFunction} implements returning values as the function
+ * catching a {@link ReturnStatement.ReturnException} thrown by a {@link ReturnStatement} in the
+ * body.
*
* <p>Every Environment has a {@link Mutability} field, and must be used within a function that
- * creates and closes this {@link Mutability} with the try-with-resource pattern.
- * This {@link Mutability} is also used when initializing mutable objects within that Environment;
- * when closed at the end of the computation freezes the Environment and all those objects that
- * then become forever immutable. The pattern enforces the discipline that there should be no
- * dangling mutable Environment, or concurrency between interacting Environment-s.
- * It is also an error to try to mutate an Environment and its objects from another Environment,
- * before the {@link Mutability} is closed.
+ * creates and closes this {@link Mutability} with the try-with-resource pattern. This {@link
+ * Mutability} is also used when initializing mutable objects within that Environment; when closed
+ * at the end of the computation freezes the Environment and all those objects that then become
+ * forever immutable. The pattern enforces the discipline that there should be no dangling mutable
+ * Environment, or concurrency between interacting Environment-s. It is also an error to try to
+ * mutate an Environment and its objects from another Environment, before the {@link Mutability} is
+ * closed.
*
- * <p>One creates an Environment using the {@link #builder} function, then
- * populates it with {@link #setup}, {@link #setupDynamic} and sometimes {@link #setupOverride},
- * before to evaluate code in it with {@link #eval}, or with {@link BuildFileAST#exec}
- * (where the AST was obtained by passing a {@link ValidationEnvironment} constructed from the
- * Environment to {@link BuildFileAST#parseBuildFile} or {@link BuildFileAST#parseSkylarkFile}).
- * When the computation is over, the frozen Environment can still be queried with {@link #lookup}.
+ * <p>One creates an Environment using the {@link #builder} function, then populates it with {@link
+ * #setup}, {@link #setupDynamic} and sometimes {@link #setupOverride}, before to evaluate code in
+ * it with {@link BuildFileAST#eval}, or with {@link BuildFileAST#exec} (where the AST was obtained
+ * by passing a {@link ValidationEnvironment} constructed from the Environment to {@link
+ * BuildFileAST#parseBuildFile} or {@link BuildFileAST#parseSkylarkFile}). When the computation is
+ * over, the frozen Environment can still be queried with {@link #lookup}.
*
* <p>Final fields of an Environment represent its dynamic state, i.e. state that remains the same
- * throughout a given evaluation context, and don't change with source code location,
- * while mutable fields embody its static state, that change with source code location.
- * The seeming paradox is that the words "dynamic" and "static" refer to the point of view
- * of the source code, and here we have a dual point of view.
+ * throughout a given evaluation context, and don't change with source code location, while mutable
+ * fields embody its static state, that change with source code location. The seeming paradox is
+ * that the words "dynamic" and "static" refer to the point of view of the source code, and here we
+ * have a dual point of view.
*/
public final class Environment implements Freezable {
@@ -313,12 +313,6 @@
private final Map<String, Extension> importedExtensions;
/**
- * Is this Environment being executed in Skylark context?
- * TODO(laurentlb): Remove from Environment
- */
- private boolean isSkylark;
-
- /**
* Is this Environment being executed during the loading phase? Many builtin functions are only
* enabled during the loading phase, and check this flag.
* TODO(laurentlb): Remove from Environment
@@ -467,7 +461,6 @@
* @param dynamicFrame a frame for the dynamic Environment
* @param eventHandler an EventHandler for warnings, errors, etc
* @param importedExtensions Extension-s from which to import bindings with load()
- * @param isSkylark true if in Skylark context
* @param fileContentHashCode a hash for the source file being evaluated, if any
* @param phase the current phase
* @param callerLabel the label this environment came from
@@ -477,7 +470,6 @@
Frame dynamicFrame,
EventHandler eventHandler,
Map<String, Extension> importedExtensions,
- boolean isSkylark,
@Nullable String fileContentHashCode,
Phase phase,
@Nullable Label callerLabel) {
@@ -487,7 +479,6 @@
Preconditions.checkArgument(dynamicFrame.mutability().isMutable());
this.eventHandler = eventHandler;
this.importedExtensions = importedExtensions;
- this.isSkylark = isSkylark;
this.phase = phase;
this.callerLabel = callerLabel;
this.transitiveHashCode =
@@ -499,7 +490,6 @@
*/
public static class Builder {
private final Mutability mutability;
- private boolean isSkylark = false;
private Phase phase = Phase.ANALYSIS;
@Nullable private Frame parent;
@Nullable private EventHandler eventHandler;
@@ -511,10 +501,11 @@
this.mutability = mutability;
}
- /** Enables Skylark for code read in this Environment. */
+ /**
+ * Obsolete, doesn't do anything.
+ * TODO(laurentlb): To be removed once call-sites have been updated
+ */
public Builder setSkylark() {
- Preconditions.checkState(!isSkylark);
- isSkylark = true;
return this;
}
@@ -568,7 +559,6 @@
dynamicFrame,
eventHandler,
importedExtensions,
- isSkylark,
fileContentHashCode,
phase,
label);
@@ -861,22 +851,4 @@
!EventKind.ERRORS_AND_WARNINGS.contains(event.getKind()), event);
}
};
-
- /**
- * Evaluates code some String input without a supporting file.
- * TODO(laurentlb): Remove from Environment
- * @param input a list of lines of code to evaluate
- * @return the value of the last statement if it's an Expression or else null
- */
- @Nullable public Object eval(String... input) throws EvalException, InterruptedException {
- BuildFileAST ast;
- if (isSkylark) {
- ast = BuildFileAST.parseSkylarkString(eventHandler, input);
- ValidationEnvironment valid = new ValidationEnvironment(this);
- valid.validateAst(ast.getStatements(), eventHandler);
- } else {
- ast = BuildFileAST.parseBuildString(eventHandler, input);
- }
- return ast.eval(this);
- }
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallbackFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallbackFunction.java
index ba88b8b..6eb95ff 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallbackFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkCallbackFunction.java
@@ -40,7 +40,6 @@
throws EvalException, InterruptedException {
try (Mutability mutability = Mutability.create("callback %s", callback)) {
Environment env = Environment.builder(mutability)
- .setSkylark()
.setEventHandler(funcallEnv.getEventHandler())
.setGlobals(funcallEnv.getGlobals())
.build();
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java
index 228ef85..add8601 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/SkylarkSignatureProcessor.java
@@ -175,13 +175,13 @@
return Runtime.NONE;
} else {
try (Mutability mutability = Mutability.create("initialization")) {
- return Environment.builder(mutability)
- .setSkylark()
- .setGlobals(Environment.CONSTANTS_ONLY)
- .setEventHandler(Environment.FAIL_FAST_HANDLER)
- .build()
- .update("unbound", Runtime.UNBOUND)
- .eval(param.defaultValue());
+ Environment env =
+ Environment.builder(mutability)
+ .setGlobals(Environment.CONSTANTS_ONLY)
+ .setEventHandler(Environment.FAIL_FAST_HANDLER)
+ .build()
+ .update("unbound", Runtime.UNBOUND);
+ return BuildFileAST.eval(env, param.defaultValue());
} catch (Exception e) {
throw new RuntimeException(String.format(
"Exception while processing @SkylarkSignature.Param %s, default value %s",
diff --git a/src/main/java/com/google/devtools/skylark/Skylark.java b/src/main/java/com/google/devtools/skylark/Skylark.java
index 12f9a16..c0c0e69 100644
--- a/src/main/java/com/google/devtools/skylark/Skylark.java
+++ b/src/main/java/com/google/devtools/skylark/Skylark.java
@@ -15,6 +15,7 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.syntax.BuildFileAST;
import com.google.devtools.build.lib.syntax.Environment;
import com.google.devtools.build.lib.syntax.Mutability;
import com.google.devtools.build.lib.syntax.Printer;
@@ -44,7 +45,6 @@
private final Mutability mutability = Mutability.create("interpreter");
private final Environment env =
Environment.builder(mutability)
- .setSkylark()
.setGlobals(Environment.DEFAULT_GLOBALS)
.setEventHandler(PRINT_HANDLER)
.build();
@@ -74,7 +74,7 @@
String input;
while ((input = prompt()) != null) {
try {
- Object result = env.eval(input);
+ Object result = BuildFileAST.eval(env, input);
if (result != null) {
System.out.println(Printer.repr(result));
}
@@ -86,7 +86,7 @@
public static void main(String[] args) {
if (args.length == 0) {
- new Skylark().readEvalPrintLoop();
+ new Skylark().readEvalPrintLoop();
} else {
System.err.println("no argument expected");
System.exit(1);
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/util/SkylarkTestCase.java b/src/test/java/com/google/devtools/build/lib/skylark/util/SkylarkTestCase.java
index d893841..8c86c07 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/util/SkylarkTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/util/SkylarkTestCase.java
@@ -54,7 +54,6 @@
public Environment newEnvironment() throws Exception {
Environment env =
Environment.builder(mutability)
- .setSkylark()
.setEventHandler(getEventHandler())
.setGlobals(SkylarkModules.getGlobals(SkylarkModules.MODULES))
.setPhase(Phase.LOADING)
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 cf0f85b..155385d 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
@@ -72,6 +72,7 @@
// Test update through API, reference through interpreter:
@Test
public void testReference() throws Exception {
+ setFailFast(false);
try {
eval("foo");
fail();
@@ -85,6 +86,7 @@
// Test assign and reference through interpreter:
@Test
public void testAssignAndReference() throws Exception {
+ setFailFast(false);
try {
eval("foo");
fail();
@@ -292,14 +294,14 @@
// We don't even get a runtime exception trying to modify these,
// because we get compile-time exceptions even before we reach runtime!
try {
- env.eval("special_var = 41");
+ BuildFileAST.eval(env, "special_var = 41");
throw new AssertionError("failed to fail");
} catch (IllegalArgumentException e) {
assertThat(e.getMessage()).contains("ERROR 1:1: Variable special_var is read only");
}
try {
- env.eval("def foo(x): x += global_var; global_var = 36; return x", "foo(1)");
+ BuildFileAST.eval(env, "def foo(x): x += global_var; global_var = 36; return x", "foo(1)");
throw new AssertionError("failed to fail");
} catch (EvalExceptionWithStackTrace e) {
assertThat(e.getMessage()).contains("Variable 'global_var' is referenced before assignment. "
diff --git a/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java b/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java
index b344607..3a04c67 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/util/EvaluationTestCase.java
@@ -86,7 +86,6 @@
*/
public Environment newSkylarkEnvironment() {
return Environment.builder(mutability)
- .setSkylark()
.setGlobals(BazelLibrary.GLOBALS)
.setEventHandler(getEventHandler())
.build();
@@ -157,7 +156,10 @@
}
public Object eval(String... input) throws Exception {
- return env.eval(input);
+ if (testMode == TestMode.SKYLARK) {
+ return BuildFileAST.eval(env, input);
+ }
+ return BuildFileAST.parseBuildString(env.getEventHandler(), input).eval(env);
}
public void checkEvalError(String msg, String... input) throws Exception {
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java b/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java
index 178e775..21132c2 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/TestMode.java
@@ -39,7 +39,6 @@
@Override
public Environment createEnvironment(EventHandler eventHandler, Environment environment) {
return Environment.builder(Mutability.create("skylark test"))
- .setSkylark()
.setGlobals(environment == null ? BazelLibrary.GLOBALS : environment.getGlobals())
.setEventHandler(eventHandler)
.build();
