Move BazelLibrary from syntax/ to packages/ This helps the Skylark interpreter to not depend on Bazel concepts, though it adds a temporary dependency of Skylint on packages/. The fix for that will be to create a Build API interface for BazelLibrary (e.g., "BazelLibraryAPI"). Refactored some GlobalFrame construction logic to be more uniform. Instead of constructing a whole Environment just to get a frame, we build the frame directly, using ImmutableMap.Builder to accumulate bindings. This convention may further change once we convert MethodLibrary and the like to @SkylarkGlobalLibrary, but for now it's more readable. RELNOTES: None PiperOrigin-RevId: 194960824
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkModules.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkModules.java index a858931..b57b96b 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkModules.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkModules.java
@@ -15,8 +15,8 @@ package com.google.devtools.build.lib.analysis.skylark; import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.packages.BazelLibrary; import com.google.devtools.build.lib.packages.SkylarkNativeModule; -import com.google.devtools.build.lib.syntax.BazelLibrary; import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.Environment.GlobalFrame; import com.google.devtools.build.lib.syntax.Mutability;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java b/src/main/java/com/google/devtools/build/lib/packages/BazelLibrary.java similarity index 82% rename from src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java rename to src/main/java/com/google/devtools/build/lib/packages/BazelLibrary.java index 4e5845f..205f98f 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/BazelLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/packages/BazelLibrary.java
@@ -12,20 +12,35 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.devtools.build.lib.syntax; +package com.google.devtools.build.lib.packages; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.collect.nestedset.Order; import com.google.devtools.build.lib.events.Location; import com.google.devtools.build.lib.skylarkinterface.Param; import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; -import java.util.List; +import com.google.devtools.build.lib.syntax.BaseFunction; +import com.google.devtools.build.lib.syntax.BuiltinFunction; +import com.google.devtools.build.lib.syntax.Environment.GlobalFrame; +import com.google.devtools.build.lib.syntax.EvalException; +import com.google.devtools.build.lib.syntax.EvalUtils; +import com.google.devtools.build.lib.syntax.MethodLibrary; +import com.google.devtools.build.lib.syntax.Runtime; +import com.google.devtools.build.lib.syntax.SelectorList; +import com.google.devtools.build.lib.syntax.SelectorValue; +import com.google.devtools.build.lib.syntax.SkylarkDict; +import com.google.devtools.build.lib.syntax.SkylarkList; +import com.google.devtools.build.lib.syntax.SkylarkNestedSet; +import com.google.devtools.build.lib.syntax.SkylarkSignatureProcessor; +import com.google.devtools.build.lib.syntax.SkylarkType; /** * A helper class containing additional built in functions for Bazel (BUILD files and .bzl files). */ public class BazelLibrary { + // TODO(bazel-team): Move to MethodLibrary alongside other pure-Skylark builtins. @SkylarkSignature( name = "type", returnType = String.class, @@ -205,21 +220,26 @@ } }; - private static Environment.GlobalFrame createGlobals() { - List<BaseFunction> bazelGlobalFunctions = ImmutableList.of(select, depset, type); - - try (Mutability mutability = Mutability.create("BUILD")) { - Environment env = Environment.builder(mutability) - .useDefaultSemantics() - .build(); - Runtime.setupConstants(env); - Runtime.setupMethodEnvironment(env, MethodLibrary.defaultGlobalFunctions); - Runtime.setupMethodEnvironment(env, bazelGlobalFunctions); - return env.getGlobals(); + /** Adds bindings for all the builtin functions of this class to the given map builder. */ + public static void addBindingsToBuilder(ImmutableMap.Builder<String, Object> builder) { + for (BaseFunction function : allFunctions) { + builder.put(function.getName(), function); } } - public static final Environment.GlobalFrame GLOBALS = createGlobals(); + private static final ImmutableList<BaseFunction> allFunctions = + ImmutableList.of(select, depset, type); + + /** A global frame containing pure Skylark builtins and some Bazel builtins. */ + public static final GlobalFrame GLOBALS = createGlobals(); + + private static GlobalFrame createGlobals() { + ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder(); + Runtime.addConstantsToBuilder(builder); + MethodLibrary.addBindingsToBuilder(builder); + BazelLibrary.addBindingsToBuilder(builder); + return GlobalFrame.createForBuiltins(builder.build()); + } static { SkylarkSignatureProcessor.configureSkylarkFunctions(BazelLibrary.class);
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java index 873934e..ac66963 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -38,7 +38,6 @@ import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; import com.google.devtools.build.lib.skylarkinterface.SkylarkValue; import com.google.devtools.build.lib.syntax.BaseFunction; -import com.google.devtools.build.lib.syntax.BazelLibrary; import com.google.devtools.build.lib.syntax.BuildFileAST; import com.google.devtools.build.lib.syntax.BuiltinFunction; import com.google.devtools.build.lib.syntax.ClassObject;
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java index 6868a98..7335874 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java +++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -33,7 +33,6 @@ import com.google.devtools.build.lib.skylarkinterface.Param; import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; import com.google.devtools.build.lib.syntax.BaseFunction; -import com.google.devtools.build.lib.syntax.BazelLibrary; import com.google.devtools.build.lib.syntax.BuildFileAST; import com.google.devtools.build.lib.syntax.BuiltinFunction; import com.google.devtools.build.lib.syntax.ClassObject;
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 46253fa..c536a68 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
@@ -281,19 +281,36 @@ this.bindings = new LinkedHashMap<>(); } - public GlobalFrame(Mutability mutability, @Nullable GlobalFrame parent, @Nullable Label label) { + public GlobalFrame( + Mutability mutability, + @Nullable GlobalFrame parent, + @Nullable Label label, + @Nullable Map<String, Object> bindings) { this.mutability = Preconditions.checkNotNull(mutability); this.parent = parent; this.label = label; this.bindings = new LinkedHashMap<>(); + if (bindings != null) { + this.bindings.putAll(bindings); + } } public GlobalFrame(Mutability mutability) { - this(mutability, null, null); + this(mutability, null, null, null); } - public GlobalFrame(Mutability mutability, GlobalFrame parent) { - this(mutability, parent, null); + public GlobalFrame(Mutability mutability, @Nullable GlobalFrame parent) { + this(mutability, parent, null, null); + } + + public GlobalFrame(Mutability mutability, @Nullable GlobalFrame parent, @Nullable Label label) { + this(mutability, parent, label, null); + } + + /** Constructs a global frame for the given builtin bindings. */ + public static GlobalFrame createForBuiltins(Map<String, Object> bindings) { + Mutability mutability = Mutability.create("<builtins>").freeze(); + return new GlobalFrame(mutability, null, null, bindings); } private void checkInitialized() { @@ -1217,34 +1234,29 @@ return transitiveHashCode; } - /** A read-only {@link Environment.GlobalFrame} with global constants in it only */ + /** A read-only {@link Environment.GlobalFrame} with False/True/None constants only. */ static final GlobalFrame CONSTANTS_ONLY = createConstantsGlobals(); - /** A read-only {@link Environment.GlobalFrame} with initial globals */ + /** + * A read-only {@link Environment.GlobalFrame} with initial globals as defined in + * MethodLibrary. + */ public static final GlobalFrame DEFAULT_GLOBALS = createDefaultGlobals(); /** To be removed when all call-sites are updated. */ public static final GlobalFrame SKYLARK = DEFAULT_GLOBALS; private static Environment.GlobalFrame createConstantsGlobals() { - try (Mutability mutability = Mutability.create("CONSTANTS")) { - Environment env = Environment.builder(mutability) - .useDefaultSemantics() - .build(); - Runtime.setupConstants(env); - return env.getGlobals(); - } + ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder(); + Runtime.addConstantsToBuilder(builder); + return GlobalFrame.createForBuiltins(builder.build()); } private static Environment.GlobalFrame createDefaultGlobals() { - try (Mutability mutability = Mutability.create("BUILD")) { - Environment env = Environment.builder(mutability) - .useDefaultSemantics() - .build(); - Runtime.setupConstants(env); - Runtime.setupMethodEnvironment(env, MethodLibrary.defaultGlobalFunctions); - return env.getGlobals(); - } + ImmutableMap.Builder<String, Object> builder = ImmutableMap.builder(); + Runtime.addConstantsToBuilder(builder); + MethodLibrary.addBindingsToBuilder(builder); + return GlobalFrame.createForBuiltins(builder.build()); } /** An exception thrown by {@link #FAIL_FAST_HANDLER}. */
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java index fe519b0..97c582a 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/MethodLibrary.java
@@ -960,7 +960,14 @@ ) public static final class BoolModule {} - static final List<BaseFunction> defaultGlobalFunctions = + /** Adds bindings for all the builtin functions of this class to the given map builder. */ + public static void addBindingsToBuilder(ImmutableMap.Builder<String, Object> builder) { + for (BaseFunction function : allFunctions) { + builder.put(function.getName(), function); + } + } + + private static final ImmutableList<BaseFunction> allFunctions = ImmutableList.of( all, any, bool, dict, dir, fail, getattr, hasattr, hash, enumerate, int_, len, list, max, min, print, range, repr, reversed, sorted, str, tuple, zip);
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Runtime.java b/src/main/java/com/google/devtools/build/lib/syntax/Runtime.java index 86f95b3..6606029 100644 --- a/src/main/java/com/google/devtools/build/lib/syntax/Runtime.java +++ b/src/main/java/com/google/devtools/build/lib/syntax/Runtime.java
@@ -16,6 +16,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.skylarkinterface.SkylarkGlobalLibrary; @@ -137,14 +138,16 @@ ) public static final String REPOSITORY_NAME = "REPOSITORY_NAME"; - /** - * Set up a given environment for supported class methods. - */ - static Environment setupConstants(Environment env) { + /** Adds bindings for False/True/None constants to the given map builder. */ + public static void addConstantsToBuilder(ImmutableMap.Builder<String, Object> builder) { // In Python 2.x, True and False are global values and can be redefined by the user. - // In Python 3.x, they are keywords. We implement them as values, for the sake of - // simplicity. We define them as Boolean objects. - return env.setup("False", FALSE).setup("True", TRUE).setup("None", NONE); + // In Python 3.x, they are keywords. We implement them as values. Currently they can't be + // redefined because builtins can't be overridden. In the future we should permit shadowing of + // most builtins but still prevent shadowing of these constants. + builder + .put("False", FALSE) + .put("True", TRUE) + .put("None", NONE); } @@ -390,11 +393,4 @@ public static void registerModuleGlobals(Environment env, Class<?> moduleClass) { setupModuleGlobals(env, moduleClass); } - - static void setupMethodEnvironment( - Environment env, Iterable<BaseFunction> functions) { - for (BaseFunction function : functions) { - env.setup(function.getName(), function); - } - } }
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 7565fc9..7385f42 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
@@ -23,7 +23,7 @@ import com.google.devtools.build.lib.events.EventKind; import com.google.devtools.build.lib.events.ExtendedEventHandler; import com.google.devtools.build.lib.events.util.EventCollectionApparatus; -import com.google.devtools.build.lib.syntax.BazelLibrary; +import com.google.devtools.build.lib.packages.BazelLibrary; import com.google.devtools.build.lib.syntax.BuildFileAST; import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.Environment.FailFastException;
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 8ddf1f4..9b848bd 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
@@ -14,8 +14,8 @@ package com.google.devtools.build.lib.testutil; import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.packages.BazelLibrary; import com.google.devtools.build.lib.packages.SkylarkSemanticsOptions; -import com.google.devtools.build.lib.syntax.BazelLibrary; import com.google.devtools.build.lib.syntax.Environment; import com.google.devtools.build.lib.syntax.Mutability; import com.google.devtools.build.lib.syntax.SkylarkSemantics;
diff --git a/src/tools/skylark/java/com/google/devtools/skylark/skylint/BUILD b/src/tools/skylark/java/com/google/devtools/skylark/skylint/BUILD index 89841a9..9bbd103 100644 --- a/src/tools/skylark/java/com/google/devtools/skylark/skylint/BUILD +++ b/src/tools/skylark/java/com/google/devtools/skylark/skylint/BUILD
@@ -16,6 +16,8 @@ srcs = glob(["**/*.java"]), visibility = ["//src/tools/skylark/javatests/com/google/devtools/skylark/skylint:__pkg__"], deps = [ + # TODO(bazel-team): Once BazelLibrary has a Build API interface, depend + # on lib:skylarkbuildapi instead of on lib:packages. "//src/main/java/com/google/devtools/build/lib:packages", "//src/main/java/com/google/devtools/build/lib:skylarkinterface", "//third_party:guava",
diff --git a/src/tools/skylark/java/com/google/devtools/skylark/skylint/Environment.java b/src/tools/skylark/java/com/google/devtools/skylark/skylint/Environment.java index 46e74d9..68d18e9 100644 --- a/src/tools/skylark/java/com/google/devtools/skylark/skylint/Environment.java +++ b/src/tools/skylark/java/com/google/devtools/skylark/skylint/Environment.java
@@ -14,9 +14,9 @@ package com.google.devtools.skylark.skylint; +import com.google.devtools.build.lib.packages.BazelLibrary; import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature; import com.google.devtools.build.lib.syntax.ASTNode; -import com.google.devtools.build.lib.syntax.BazelLibrary; import com.google.devtools.build.lib.syntax.Comment; import com.google.devtools.build.lib.syntax.Identifier; import com.google.devtools.build.lib.syntax.MethodLibrary;