Register builtins with Runtime
This covers all builtins in classes that use SkylarkSignatureProcessor#configureSkylarkFunctions. Generally this means things you define with @SkylarkSignature.
It is now an error to call configureSkylarkFunctions multiple times for the same class. It should only be called in static initializers.
Runtime's static knowledge of builtins has been factored into Runtime.BuiltinRegistry.
RELNOTES: None
PiperOrigin-RevId: 178295926
diff --git a/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationCollector.java b/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationCollector.java
index f3b6eae..c38bfa2 100644
--- a/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationCollector.java
+++ b/src/main/java/com/google/devtools/build/docgen/SkylarkDocumentationCollector.java
@@ -29,10 +29,10 @@
import com.google.devtools.build.lib.util.Classpath.ClassPathException;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
+import java.util.ArrayDeque;
import java.util.Deque;
import java.util.HashMap;
import java.util.HashSet;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -98,7 +98,7 @@
static void collectJavaObjects(SkylarkModule firstModule, Class<?> firstClass,
Map<String, SkylarkModuleDoc> modules) {
Set<Class<?>> done = new HashSet<>();
- Deque<Class<?>> toProcess = new LinkedList<>();
+ Deque<Class<?>> toProcess = new ArrayDeque<>();
Map<Class<?>, SkylarkModule> annotations = new HashMap<>();
toProcess.addLast(firstClass);
@@ -139,7 +139,7 @@
Class<?> moduleClass = skylarkSignature.objectType();
SkylarkModule skylarkModule = moduleClass.equals(Object.class)
? getTopLevelModule()
- : Runtime.getCanonicalRepresentation(moduleClass).getAnnotation(SkylarkModule.class);
+ : Runtime.getSkylarkNamespace(moduleClass).getAnnotation(SkylarkModule.class);
if (skylarkModule == null) {
// TODO(bazel-team): we currently have undocumented methods on undocumented data
// structures, namely java.util.List. Remove this case when we are done.
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 4688db2..d9baecf 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
@@ -61,7 +61,7 @@
.setGlobals(BazelLibrary.GLOBALS)
.build();
for (Class<?> moduleClass : modules) {
- Runtime.registerModuleGlobals(env, moduleClass);
+ Runtime.setupModuleGlobals(env, moduleClass);
}
return env.getGlobals();
}
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 1091366..b03af4f 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
@@ -1552,8 +1552,9 @@
*/
private ClassObject newNativeModule() {
ImmutableMap.Builder<String, Object> builder = new ImmutableMap.Builder<>();
- for (String nativeFunction : Runtime.getFunctionNames(SkylarkNativeModule.class)) {
- builder.put(nativeFunction, Runtime.getFunction(SkylarkNativeModule.class, nativeFunction));
+ Runtime.BuiltinRegistry builtins = Runtime.getBuiltinRegistry();
+ for (String nativeFunction : builtins.getFunctionNames(SkylarkNativeModule.class)) {
+ builder.put(nativeFunction, builtins.getFunction(SkylarkNativeModule.class, nativeFunction));
}
builder.putAll(ruleFunctions);
builder.put("package", newPackageFunction(packageArguments));
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 dae4393..3283d99 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
@@ -514,8 +514,9 @@
private static ClassObject newNativeModule(
ImmutableMap<String, BaseFunction> workspaceFunctions, String version) {
ImmutableMap.Builder<String, Object> builder = new ImmutableMap.Builder<>();
- for (String nativeFunction : Runtime.getFunctionNames(SkylarkNativeModule.class)) {
- builder.put(nativeFunction, Runtime.getFunction(SkylarkNativeModule.class, nativeFunction));
+ Runtime.BuiltinRegistry builtins = Runtime.getBuiltinRegistry();
+ for (String nativeFunction : builtins.getFunctionNames(SkylarkNativeModule.class)) {
+ builder.put(nativeFunction, builtins.getFunction(SkylarkNativeModule.class, nativeFunction));
}
for (Map.Entry<String, BaseFunction> function : workspaceFunctions.entrySet()) {
builder.put(function.getKey(), function.getValue());
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
index 2acfb21..833b20a 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/FuncallExpression.java
@@ -616,7 +616,7 @@
Location location = call.getLocation();
Object value = positionals.get(0);
ImmutableList<Object> positionalArgs = positionals.subList(1, positionals.size());
- BaseFunction function = Runtime.getFunction(EvalUtils.getSkylarkType(value.getClass()), method);
+ BaseFunction function = Runtime.getBuiltinRegistry().getFunction(value.getClass(), method);
Object fieldValue =
(value instanceof ClassObject) ? ((ClassObject) value).getValue(method) : null;
if (function != null) {
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 0c798a9..657d28d 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
@@ -33,9 +33,9 @@
import com.google.devtools.build.lib.syntax.SkylarkList.MutableList;
import com.google.devtools.build.lib.syntax.SkylarkList.Tuple;
import com.google.devtools.build.lib.syntax.Type.ConversionException;
+import java.util.ArrayDeque;
import java.util.ArrayList;
import java.util.Iterator;
-import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
@@ -374,7 +374,7 @@
maxSplits = Integer.MAX_VALUE;
}
- LinkedList<String> result = new LinkedList<>();
+ ArrayDeque<String> result = new ArrayDeque<>();
String[] parts = input.split(Pattern.quote(separator), -1);
int sepLen = separator.length();
int remainingLength = input.length();
@@ -1170,7 +1170,7 @@
throw new EvalException(
loc, "Argument to reversed() must be a sequence, not a depset.");
}
- LinkedList<Object> tmpList = new LinkedList<>();
+ ArrayDeque<Object> tmpList = new ArrayDeque<>();
for (Object element : EvalUtils.toIterable(sequence, loc, env)) {
tmpList.addFirst(element);
}
@@ -2042,7 +2042,7 @@
* Returns whether the given object has a method with the given name.
*/
private static boolean hasMethod(Object obj, String name) throws EvalException {
- if (Runtime.getFunctionNames(obj.getClass()).contains(name)) {
+ if (Runtime.getBuiltinRegistry().getFunctionNames(obj.getClass()).contains(name)) {
return true;
}
@@ -2068,7 +2068,7 @@
if (object instanceof ClassObject) {
fields.addAll(((ClassObject) object).getKeys());
}
- fields.addAll(Runtime.getFunctionNames(object.getClass()));
+ fields.addAll(Runtime.getBuiltinRegistry().getFunctionNames(object.getClass()));
fields.addAll(FuncallExpression.getMethodNames(object.getClass()));
return MutableList.copyOf(env, fields);
}
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 72b93e2..839b311 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
@@ -24,11 +24,13 @@
import java.lang.reflect.Field;
import java.util.HashMap;
import java.util.Map;
-import java.util.Set;
+import java.util.TreeMap;
+import javax.annotation.Nullable;
/**
- * Global constants and support for global namespaces of runtime functions.
+ * Global constants and support for static registration of builtin symbols.
*/
+// TODO(bazel-team): Rename to SkylarkRuntime to avoid conflict with java.lang.Runtime.
public final class Runtime {
private Runtime() {}
@@ -143,39 +145,113 @@
}
- /** Global registry of functions associated to a class or namespace */
- private static final Map<Class<?>, Map<String, BaseFunction>> functions = new HashMap<>();
-
/**
- * Registers a function with namespace to this global environment.
+ * Returns the canonical class representing the namespace associated with the given class, i.e.,
+ * the class under which builtins should be registered.
*/
- public static void registerFunction(Class<?> nameSpace, BaseFunction function) {
- Preconditions.checkNotNull(nameSpace);
- Preconditions.checkArgument(nameSpace.equals(getCanonicalRepresentation(nameSpace)));
- Preconditions.checkArgument(
- getCanonicalRepresentation(function.getObjectType()).equals(nameSpace));
- functions.computeIfAbsent(nameSpace, k -> new HashMap<String, BaseFunction>());
- functions.get(nameSpace).put(function.getName(), function);
+ public static Class<?> getSkylarkNamespace(Class<?> clazz) {
+ return String.class.isAssignableFrom(clazz)
+ ? MethodLibrary.StringModule.class
+ : EvalUtils.getSkylarkType(clazz);
}
/**
- * Returns the canonical representation of the given class, i.e. the super class for which any
- * functions were registered.
+ * A registry of builtins, including both global builtins and builtins that are under some
+ * namespace.
*
- * <p>Currently, this is only necessary for mapping the different subclasses of {@link
- * java.util.Map} to the interface.
+ * <p>This object is unsynchronized, but concurrent reads are fine.
*/
- // TODO(bazel-team): make everything a SkylarkValue, and remove this function.
- public static Class<?> getCanonicalRepresentation(Class<?> clazz) {
- if (String.class.isAssignableFrom(clazz)) {
- return MethodLibrary.StringModule.class;
+ public static class BuiltinRegistry {
+
+ /**
+ * All registered builtins, keyed and sorted by an identifying (but otherwise unimportant)
+ * string.
+ *
+ * <p>The string is typically formed from the builtin's simple name and the Java class in which
+ * it is defined. The Java class need not correspond to a namespace. (This map includes global
+ * builtins that have no namespace.)
+ */
+ private final Map<String, Object> allBuiltins = new TreeMap<>();
+
+ /** All non-global builtin functions, keyed by their namespace class and their name. */
+ private final Map<Class<?>, Map<String, BaseFunction>> functions = new HashMap<>();
+
+ /** Registers a builtin with the given simple name, that was defined in the given Java class. */
+ public void registerBuiltin(Class<?> definingClass, String name, Object builtin) {
+ String key = String.format("%s.%s", definingClass.getName(), name);
+ Preconditions.checkArgument(
+ !allBuiltins.containsKey(key),
+ "Builtin '%s' registered multiple times",
+ key);
+ allBuiltins.put(key, builtin);
}
- return EvalUtils.getSkylarkType(clazz);
+
+ /**
+ * Registers a function underneath a namespace.
+ *
+ * <p>This is independent of {@link #registerBuiltin}.
+ */
+ public void registerFunction(Class<?> namespace, BaseFunction function) {
+ Preconditions.checkNotNull(namespace);
+ Preconditions.checkNotNull(function.getObjectType());
+ Class<?> skylarkNamespace = getSkylarkNamespace(namespace);
+ Preconditions.checkArgument(skylarkNamespace.equals(namespace));
+ Class<?> objType = getSkylarkNamespace(function.getObjectType());
+ Preconditions.checkArgument(objType.equals(skylarkNamespace));
+
+ functions.computeIfAbsent(namespace, k -> new HashMap<>());
+ functions.get(namespace).put(function.getName(), function);
+ }
+
+ /** Returns a set of all registered builtins, in a deterministic order. */
+ public ImmutableSet<Object> getBuiltins() {
+ return ImmutableSet.copyOf(allBuiltins.values());
+ }
+
+ @Nullable
+ private Map<String, BaseFunction> getFunctionsInNamespace(Class<?> namespace) {
+ return functions.get(getSkylarkNamespace(namespace));
+ }
+
+ /**
+ * Given a namespace, returns the function with the given name.
+ *
+ * <p>If the namespace does not exist or has no function with that name, returns null.
+ */
+ public BaseFunction getFunction(Class<?> namespace, String name) {
+ Map<String, BaseFunction> namespaceFunctions = getFunctionsInNamespace(namespace);
+ return namespaceFunctions != null ? namespaceFunctions.get(name) : null;
+ }
+
+ /**
+ * Given a namespace, returns all function names.
+ *
+ * <p>If the namespace does not exist, returns an empty set.
+ */
+ public ImmutableSet<String> getFunctionNames(Class<?> namespace) {
+ Map<String, BaseFunction> namespaceFunctions = getFunctionsInNamespace(namespace);
+ if (namespaceFunctions == null) {
+ return ImmutableSet.of();
+ }
+ return ImmutableSet.copyOf(namespaceFunctions.keySet());
+ }
}
+ /**
+ * All Skylark builtins.
+ *
+ * <p>Note that just because a symbol is registered here does not necessarily mean that it is
+ * accessible in a particular {@link Environment}. This registry should include any builtin that
+ * is available in any environment.
+ *
+ * <p>Thread safety: This object is unsynchronized. The register functions are typically called
+ * from within static initializer blocks, which should be fine.
+ */
+ private static final BuiltinRegistry builtins = new BuiltinRegistry();
- static Map<String, BaseFunction> getNamespaceFunctions(Class<?> nameSpace) {
- return functions.get(getCanonicalRepresentation(nameSpace));
+ /** Retrieve the static instance containing information on all known Skylark builtins. */
+ public static BuiltinRegistry getBuiltinRegistry() {
+ return builtins;
}
/**
@@ -183,7 +259,7 @@
* @param env the Environment into which to register fields.
* @param moduleClass the Class object containing globals.
*/
- public static void registerModuleGlobals(Environment env, Class<?> moduleClass) {
+ public static void setupModuleGlobals(Environment env, Class<?> moduleClass) {
try {
if (moduleClass.isAnnotationPresent(SkylarkModule.class)) {
env.setup(
@@ -211,19 +287,15 @@
}
/**
- * Returns the function of the namespace of the given name or null of it does not exists.
+ * Registers global fields with SkylarkSignature into the specified Environment. Alias for
+ * {@link #setupModuleGlobals}.
+ *
+ * @deprecated Use {@link #setupModuleGlobals} instead.
*/
- public static BaseFunction getFunction(Class<?> nameSpace, String name) {
- Map<String, BaseFunction> nameSpaceFunctions = getNamespaceFunctions(nameSpace);
- return nameSpaceFunctions != null ? nameSpaceFunctions.get(name) : null;
- }
-
- /**
- * Returns the function names registered with the namespace.
- */
- public static Set<String> getFunctionNames(Class<?> nameSpace) {
- Map<String, BaseFunction> nameSpaceFunctions = getNamespaceFunctions(nameSpace);
- return nameSpaceFunctions != null ? nameSpaceFunctions.keySet() : ImmutableSet.of();
+ @Deprecated
+ // TODO(bazel-team): Remove after all callers updated.
+ public static void registerModuleGlobals(Environment env, Class<?> moduleClass) {
+ setupModuleGlobals(env, moduleClass);
}
static void setupMethodEnvironment(
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 358afae..346a913 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
@@ -215,13 +215,20 @@
}
/**
- * Configure all BaseFunction-s in a class from their @SkylarkSignature annotations
- * @param type a class containing BuiltinFunction fields that need be configured.
- * This function is typically called in a static block to initialize a class,
- * i.e. a class {@code Foo} containing @SkylarkSignature annotations would end with
- * {@code static { SkylarkSignatureProcessor.configureSkylarkFunctions(Foo.class); }}
+ * Processes all {@link SkylarkSignature}-annotated fields in a class. This function should only
+ * be called once per class, and not concurrently.
+ *
+ * <p>This includes registering these fields as builtins using {@link Runtime}, and for {@link
+ * BaseFunction} instances, calling {@link BaseFunction#configure(SkylarkSignature)}. The fields
+ * will be picked up by reflection even if they are not public.
+ *
+ * <p>A class typically calls this function from within a static initializer block, passing itself
+ * as the {@code type}. E.g., A class {@code Foo} containing {@code @SkylarkSignature} annotations
+ * would end with {@code static { SkylarkSignatureProcessor.configureSkylarkFunctions(Foo.class);
+ * }}
*/
public static void configureSkylarkFunctions(Class<?> type) {
+ Runtime.BuiltinRegistry builtins = Runtime.getBuiltinRegistry();
for (Field field : type.getDeclaredFields()) {
if (field.isAnnotationPresent(SkylarkSignature.class)) {
// The annotated fields are often private, but we need access them.
@@ -232,6 +239,7 @@
value = Preconditions.checkNotNull(field.get(null),
String.format(
"Error while trying to configure %s.%s: its value is null", type, field));
+ builtins.registerBuiltin(type, field.getName(), value);
if (BaseFunction.class.isAssignableFrom(field.getType())) {
BaseFunction function = (BaseFunction) value;
if (!function.isConfigured()) {
@@ -240,8 +248,7 @@
Class<?> nameSpace = function.getObjectType();
if (nameSpace != null) {
Preconditions.checkState(!(function instanceof BuiltinFunction.Factory));
- nameSpace = Runtime.getCanonicalRepresentation(nameSpace);
- Runtime.registerFunction(nameSpace, function);
+ builtins.registerFunction(nameSpace, function);
}
}
} catch (IllegalAccessException e) {
