Refactor Skylark Environment-s
Make Environment-s freezable: Introduce a class Mutability
as a revokable capability to mutate objects in an Environment.
For now, only Environment-s carry this capability.
Make sure that every Mutability is revoked in the same function that create...
This reinstates a change that previously rolled-back because it broke the
serializability of SkylarkLookupValue. Bad news: serializing it succeeds for the
wrong reason, because a SkylarkEnvironment was stored as a result (now an
Environment.Extension) that was Serializable but inherited its bindings from an Environment (now an Environment.BaseExtension) which wasn't Serializable.
Apparently, Java doesn't try to serialize the bindings then (or at least doesn't
error out when it fails), because these bindings map variable names to pretty
arbitrary objects, and a lot of those we find in practice aren't Serializable.
Thus the current code passes the same tests as the previous code, but obviously
the serialization is just as ineffective as it used to be.
--
MOS_MIGRATED_REVID=102776694
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 1ad455da..074f79f 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
@@ -14,6 +14,7 @@
package com.google.devtools.build.lib.syntax;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.events.Location.LineAndColumn;
import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.profiler.ProfilerTask;
@@ -26,15 +27,16 @@
public class UserDefinedFunction extends BaseFunction {
private final ImmutableList<Statement> statements;
- private final SkylarkEnvironment definitionEnv;
+
+ // we close over the globals at the time of definition
+ private final Environment.Frame definitionGlobals;
protected UserDefinedFunction(Identifier function,
FunctionSignature.WithValues<Object, SkylarkType> signature,
- ImmutableList<Statement> statements, SkylarkEnvironment definitionEnv) {
+ ImmutableList<Statement> statements, Environment.Frame definitionGlobals) {
super(function.getName(), signature, function.getLocation());
-
this.statements = statements;
- this.definitionEnv = definitionEnv;
+ this.definitionGlobals = definitionGlobals;
}
public FunctionSignature.WithValues<Object, SkylarkType> getFunctionSignature() {
@@ -48,24 +50,37 @@
@Override
public Object call(Object[] arguments, FuncallExpression ast, Environment env)
throws EvalException, InterruptedException {
- ImmutableList<String> names = signature.getSignature().getNames();
-
- // Registering the functions's arguments as variables in the local Environment
- int i = 0;
- for (String name : names) {
- env.update(name, arguments[i++]);
+ if (!env.mutability().isMutable()) {
+ throw new EvalException(getLocation(), "Trying to call in frozen environment");
+ }
+ if (env.getStackTrace().contains(this)) {
+ throw new EvalException(getLocation(),
+ String.format("Recursion was detected when calling '%s' from '%s'",
+ getName(), Iterables.getLast(env.getStackTrace()).getName()));
}
Profiler.instance().startTask(ProfilerTask.SKYLARK_USER_FN,
getLocationPathAndLine() + "#" + getName());
Statement lastStatement = null;
try {
- for (Statement stmt : statements) {
- lastStatement = stmt;
- stmt.exec(env);
+ env.enterScope(this, ast, definitionGlobals);
+ ImmutableList<String> names = signature.getSignature().getNames();
+
+ // Registering the functions's arguments as variables in the local Environment
+ int i = 0;
+ for (String name : names) {
+ env.update(name, arguments[i++]);
}
- } catch (ReturnStatement.ReturnException e) {
- return e.getValue();
+
+ try {
+ for (Statement stmt : statements) {
+ lastStatement = stmt;
+ stmt.exec(env);
+ }
+ } catch (ReturnStatement.ReturnException e) {
+ return e.getValue();
+ }
+ return Runtime.NONE;
} catch (EvalExceptionWithStackTrace ex) {
// We need this block since the next "catch" must only catch EvalExceptions that don't have a
// stack trace yet.
@@ -77,8 +92,8 @@
throw real;
} finally {
Profiler.instance().completeTask(ProfilerTask.SKYLARK_USER_FN);
+ env.exitScope();
}
- return Runtime.NONE;
}
/**
@@ -103,14 +118,4 @@
}
return builder.toString();
}
-
-
- /**
- * Creates a new environment for the execution of this function.
- */
- @Override
- protected Environment getOrCreateChildEnvironment(Environment parent) throws EvalException {
- return SkylarkEnvironment.createEnvironmentForFunctionCalling(
- parent, definitionEnv, this);
- }
}
