BEGIN_PUBLIC
starlark: delete Module.getTransitiveBindings and get
Make clients spell out what they mean instead of just
looking for key/value pairs in random places.
Starlark is a statically scoped language, after all.
END_PUBLIC
PiperOrigin-RevId: 343547890
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java
index 26cfa20..dec32c0 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java
@@ -186,7 +186,7 @@
@Nullable
private static ImmutableMap<String, Object> getDict(Module module, String dictName)
throws EvalException {
- Object value = module.get(dictName);
+ Object value = module.getGlobal(dictName);
if (value == null) {
throw Starlark.errorf("expected a '%s' dictionary to be defined", dictName);
}
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkAttrModuleApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkAttrModuleApi.java
index a854a93..c11cce8 100644
--- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkAttrModuleApi.java
+++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeStarlarkAttrModuleApi.java
@@ -21,7 +21,6 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
-import java.util.Map.Entry;
import net.starlark.java.eval.Dict;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Module;
@@ -270,11 +269,17 @@
* name will be set to "Unknown Provider".
*/
private static String providerName(ProviderApi provider, StarlarkThread thread) {
- Map<String, Object> bindings =
- Module.ofInnermostEnclosingStarlarkFunction(thread).getTransitiveBindings();
- for (Entry<String, Object> envEntry : bindings.entrySet()) {
- if (provider.equals(envEntry.getValue())) {
- return envEntry.getKey();
+ Module bzl = Module.ofInnermostEnclosingStarlarkFunction(thread);
+ // user-defined provider?
+ for (Map.Entry<String, Object> e : bzl.getGlobals().entrySet()) {
+ if (provider.equals(e.getValue())) {
+ return e.getKey();
+ }
+ }
+ // predeclared provider? (e.g. DefaultInfo)
+ for (Map.Entry<String, Object> e : bzl.getPredeclaredBindings().entrySet()) {
+ if (provider.equals(e.getValue())) {
+ return e.getKey();
}
}
return "Unknown Provider";
diff --git a/src/main/java/net/starlark/java/eval/Eval.java b/src/main/java/net/starlark/java/eval/Eval.java
index d650715..c4e6b70 100644
--- a/src/main/java/net/starlark/java/eval/Eval.java
+++ b/src/main/java/net/starlark/java/eval/Eval.java
@@ -642,8 +642,17 @@
result = fn(fr).getModule().getGlobal(id.getName());
break;
case PREDECLARED:
- // TODO(adonovan): call getPredeclared
- result = fn(fr).getModule().get(id.getName());
+ // TODO(adonovan): don't call getGlobal. This requires the Resolver to distinguish
+ // "predeclared" vars from "already defined module globals" instead of lumping them
+ // together via getNames. The latter odd category exists in the REPL, and
+ // in EvaluationTestCase, which calls Starlark.execFile repeatedly on the same Module.
+ // The REPL could just create a new module for each chunk, whose predeclared vars
+ // are the previous module's globals, but things may be trickier in EvaluationTestCase.
+ Module module = fn(fr).getModule();
+ result = module.getGlobal(id.getName());
+ if (result == null) {
+ result = module.getPredeclared(id.getName());
+ }
break;
default:
throw new IllegalStateException(bind.toString());
diff --git a/src/main/java/net/starlark/java/eval/Module.java b/src/main/java/net/starlark/java/eval/Module.java
index 83b0551..fd9cc560 100644
--- a/src/main/java/net/starlark/java/eval/Module.java
+++ b/src/main/java/net/starlark/java/eval/Module.java
@@ -172,7 +172,8 @@
* `load`).
*/
// TODO(adonovan): whether bindings are exported should be decided by the resolver;
- // non-exported bindings should never be added to the module. Delete this.
+ // non-exported bindings should never be added to the module. Delete this,
+ // once loads bind locally (then all globals will be exported).
public ImmutableMap<String, Object> getExportedGlobals() {
ImmutableMap.Builder<String, Object> result = new ImmutableMap.Builder<>();
for (Map.Entry<String, Object> entry : globals.entrySet()) {
@@ -191,12 +192,14 @@
// TODO(adonovan): opt: change the resolver to request names on
// demand to avoid all this set copying.
HashSet<String> names = new HashSet<>();
- for (Map.Entry<String, Object> bind : getTransitiveBindings().entrySet()) {
+ names.addAll(Starlark.UNIVERSE.keySet());
+ for (Map.Entry<String, Object> bind : getPredeclaredBindings().entrySet()) {
if (bind.getValue() instanceof FlagGuardedValue) {
continue; // disabled
}
names.add(bind.getKey());
}
+ names.addAll(globals.keySet());
return names;
}
@@ -210,20 +213,6 @@
}
/**
- * Returns a new map containing the predeclared (including universal) and global bindings of this
- * module.
- */
- // TODO(adonovan): eliminate; clients should explicitly choose getPredeclared or getGlobals.
- public Map<String, Object> getTransitiveBindings() {
- // Can't use ImmutableMap.Builder because it doesn't allow duplicates.
- LinkedHashMap<String, Object> env = new LinkedHashMap<>();
- env.putAll(Starlark.UNIVERSE);
- env.putAll(predeclared);
- env.putAll(globals);
- return env;
- }
-
- /**
* Returns the value of the specified global variable, or null if not bound. Does not look in the
* predeclared environment.
*/
@@ -231,22 +220,6 @@
return globals.get(name);
}
- /**
- * Returns the value of the named variable in the module global environment (as if by {@link
- * #getGlobal}), or if not bound there, in the predeclared environment (as if by {@link
- * #getPredeclared}, or if not bound there, null.
- */
- public Object get(String name) {
- // TODO(adonovan): delete this whole function, and getTransitiveBindings.
- // With proper resolution, the interpreter will know whether
- // to look in the module or the predeclared/universal environment.
- Object v = getGlobal(name);
- if (v != null) {
- return v;
- }
- return getPredeclared(name);
- }
-
/** Updates a global binding in the module environment. */
public void setGlobal(String name, Object value) {
Preconditions.checkNotNull(value, "Module.setGlobal(%s, null)", name);
