Give StarlarkValue#debugPrint access to the StarlarkThread

Since `debugPrint` is meant for emitting potentially non-deterministic, it is safe to give it access to `StarlarkThread` information. This will be used in a follow-up to let `print` and `fail` emit `Label`s in display form without having to track the reverse repo mapping for the sake of invalidation of repo rules or module extensions. Work towards bazelbuild#20486
fmeum · Apr 10, 2024 · d0555b8 · d0555b8
1 parent 750a038
commit d0555b8
Show file tree

Hide file tree

Showing 11 changed files with 34 additions and 29 deletions.
diff --git a/...n/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java b/...n/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java
@@ -54,7 +54,7 @@
 import net.starlark.java.eval.Dict;
 import net.starlark.java.eval.Printer;
 import net.starlark.java.eval.Starlark;
-import net.starlark.java.eval.StarlarkSemantics;
+import net.starlark.java.eval.StarlarkThread;
 
 /**
  * A {@link com.google.devtools.build.lib.analysis.ConfiguredTarget} that is produced by a rule.
@@ -249,7 +249,7 @@ public void repr(Printer printer) {
   }
 
   @Override
-  public void debugPrint(Printer printer, StarlarkSemantics semantics) {
+  public void debugPrint(Printer printer, StarlarkThread thread) {
     // Show the names of the provider keys that this target propagates.
     // Provider key names might potentially be *private* information, and thus a comprehensive
     // list of provider keys should not be exposed in any way other than for debug information.

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/Args.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/Args.java
@@ -71,7 +71,7 @@ public void repr(Printer printer) {
   }
 
   @Override
-  public void debugPrint(Printer printer, StarlarkSemantics semantics) {
+  public void debugPrint(Printer printer, StarlarkThread thread) {
     try {
       printer.append(
           Joiner.on(" ").join(build(/* mainRepoMappingSupplier= */ () -> null).arguments()));

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/TypeCheckedTag.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/TypeCheckedTag.java
@@ -24,7 +24,7 @@
 import net.starlark.java.annot.StarlarkBuiltin;
 import net.starlark.java.eval.EvalException;
 import net.starlark.java.eval.Printer;
-import net.starlark.java.eval.StarlarkSemantics;
+import net.starlark.java.eval.StarlarkThread;
 import net.starlark.java.eval.Structure;
 import net.starlark.java.spelling.SpellChecker;
 import net.starlark.java.syntax.Location;
@@ -150,7 +150,7 @@ public String getErrorMessageForUnknownField(String field) {
   }
 
   @Override
-  public void debugPrint(Printer printer, StarlarkSemantics semantics) {
+  public void debugPrint(Printer printer, StarlarkThread thread) {
     printer.append(String.format("'%s' tag at %s", tagClassName, location));
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingContext.java
@@ -279,23 +279,23 @@ public Sequence<Linkstamp> getLinkstampsForStarlark() {
     }
 
     @Override
-    public void debugPrint(Printer printer, StarlarkSemantics semantics) {
+    public void debugPrint(Printer printer, StarlarkThread thread) {
       printer.append("<LinkerInput(owner=");
       if (owner == null) {
         printer.append("[null owner, uses old create_linking_context API]");
       } else {
-        owner.debugPrint(printer, semantics);
+        owner.debugPrint(printer, thread);
       }
       printer.append(", libraries=[");
       for (LibraryToLink libraryToLink : libraries) {
-        libraryToLink.debugPrint(printer, semantics);
+        libraryToLink.debugPrint(printer, thread);
         printer.append(", ");
       }
       printer.append("], userLinkFlags=[");
       printer.append(Joiner.on(", ").join(userLinkFlags));
       printer.append("], nonCodeInputs=[");
       for (Artifact nonCodeInput : nonCodeInputs) {
-        nonCodeInput.debugPrint(printer, semantics);
+        nonCodeInput.debugPrint(printer, thread);
         printer.append(", ");
       }
       // TODO(cparsons): Add debug repesentation of linkstamps.
@@ -394,10 +394,10 @@ public String toString() {
   @Nullable private final ExtraLinkTimeLibraries extraLinkTimeLibraries;
 
   @Override
-  public void debugPrint(Printer printer, StarlarkSemantics semantics) {
+  public void debugPrint(Printer printer, StarlarkThread thread) {
     printer.append("<CcLinkingContext([");
     for (LinkerInput linkerInput : linkerInputs.toList()) {
-      linkerInput.debugPrint(printer, semantics);
+      linkerInput.debugPrint(printer, thread);
       printer.append(", ");
     }
     printer.append("])>");

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FeatureConfigurationForStarlark.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FeatureConfigurationForStarlark.java
@@ -62,7 +62,7 @@ public void str(Printer printer, StarlarkSemantics semantics) {
   }
 
   @Override
-  public void debugPrint(Printer printer, StarlarkSemantics semantics) {
+  public void debugPrint(Printer printer, StarlarkThread thread) {
     printer.append("<FeatureConfiguration(");
     printer.append(Joiner.on(", ").join(featureConfiguration.getEnabledFeatureNames()));
     printer.append(")>");

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LibraryToLink.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LibraryToLink.java
@@ -31,7 +31,6 @@
 import net.starlark.java.eval.Printer;
 import net.starlark.java.eval.Sequence;
 import net.starlark.java.eval.StarlarkList;
-import net.starlark.java.eval.StarlarkSemantics;
 import net.starlark.java.eval.StarlarkThread;
 
 /** Encapsulates information for linking a library. */
@@ -224,7 +223,7 @@ LinkerInputs.LibraryToLink getInterfaceLibraryToLink() {
   abstract boolean getDisableWholeArchive();
 
   @Override
-  public final void debugPrint(Printer printer, StarlarkSemantics semantics) {
+  public final void debugPrint(Printer printer, StarlarkThread thread) {
     printer.append("<LibraryToLink(");
     printer.append(
         Joiner.on(", ")

diff --git a/src/main/java/net/starlark/java/eval/MethodLibrary.java b/src/main/java/net/starlark/java/eval/MethodLibrary.java
@@ -834,14 +834,14 @@ public void fail(Object msg, Object attr, Tuple args, StarlarkThread thread)
       if (needSeparator) {
         printer.append(" ");
       }
-      printer.debugPrint(msg, thread.getSemantics());
+      printer.debugPrint(msg, thread);
       needSeparator = true;
     }
     for (Object arg : args) {
       if (needSeparator) {
         printer.append(" ");
       }
-      printer.debugPrint(arg, thread.getSemantics());
+      printer.debugPrint(arg, thread);
       needSeparator = true;
     }
     throw Starlark.errorf("%s", printer.toString());
@@ -875,7 +875,7 @@ public void print(String sep, Sequence<?> args, StarlarkThread thread) throws Ev
     String separator = "";
     for (Object x : args) {
       p.append(separator);
-      p.debugPrint(x, thread.getSemantics());
+      p.debugPrint(x, thread);
       separator = sep;
     }
     // The PRINT_TEST_MARKER key is used in tests to verify the effects of command-line options.

diff --git a/src/main/java/net/starlark/java/eval/Printer.java b/src/main/java/net/starlark/java/eval/Printer.java
@@ -114,13 +114,13 @@ public final String toString() {
    *
    * <p>Implementations of StarlarkValue may define their own behavior of {@code debugPrint}.
    */
-  public Printer debugPrint(Object o, StarlarkSemantics semantics) {
+  public Printer debugPrint(Object o, StarlarkThread thread) {
     if (o instanceof StarlarkValue) {
-      ((StarlarkValue) o).debugPrint(this, semantics);
+      ((StarlarkValue) o).debugPrint(this, thread);
       return this;
     }
 
-    return this.str(o, semantics);
+    return this.str(o, thread.getSemantics());
   }
 
   /**

diff --git a/src/main/java/net/starlark/java/eval/StarlarkValue.java b/src/main/java/net/starlark/java/eval/StarlarkValue.java
@@ -57,8 +57,8 @@ default void str(Printer printer, StarlarkSemantics semantics) {
    *
    * @param printer a printer to be used for formatting nested values.
    */
-  default void debugPrint(Printer printer, StarlarkSemantics semantics) {
-    str(printer, semantics);
+  default void debugPrint(Printer printer, StarlarkThread thread) {
+    str(printer, thread.getSemantics());
   }
 
   /** Returns the truth-value of this Starlark value. */

diff --git a/.../java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java b/.../java/com/google/devtools/build/lib/starlark/StarlarkRuleImplementationFunctionsTest.java
@@ -74,6 +74,7 @@
 import net.starlark.java.annot.Param;
 import net.starlark.java.annot.StarlarkMethod;
 import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.Mutability;
 import net.starlark.java.eval.Printer;
 import net.starlark.java.eval.Sequence;
 import net.starlark.java.eval.Starlark;
@@ -3525,7 +3526,10 @@ public void testPrintArgs() throws Exception {
     setRuleContext(createRuleContext("//foo:foo"));
     ev.exec("args = ruleContext.actions.args()", "args.add_all(['--foo', '--bar'])");
     Args args = (Args) ev.eval("args");
-    assertThat(new Printer().debugPrint(args, getStarlarkSemantics()).toString())
+    assertThat(
+            new Printer()
+                .debugPrint(args, new StarlarkThread(Mutability.IMMUTABLE, getStarlarkSemantics()))
+                .toString())
         .isEqualTo("--foo --bar");
   }
 

diff --git a/src/test/java/net/starlark/java/eval/StarlarkEvaluationTest.java b/src/test/java/net/starlark/java/eval/StarlarkEvaluationTest.java
@@ -377,7 +377,7 @@ public Structure proxyMethodsObject(StarlarkThread thread)
         useStarlarkThread = true)
     public String withArgsAndThread(
         StarlarkInt pos1, boolean pos2, boolean named, Sequence<?> args, StarlarkThread thread) {
-      String argsString = debugPrintArgs(args, thread.getSemantics());
+      String argsString = debugPrintArgs(args, thread);
       return "with_args_and_thread("
           + pos1
           + ", "
@@ -416,9 +416,11 @@ public String withKwargs(boolean pos, boolean named, Dict<String, Object> kwargs
           @Param(name = "foo", named = true, positional = true),
         },
         extraPositionals = @Param(name = "args"),
-        extraKeywords = @Param(name = "kwargs"))
-    public String withArgsAndKwargs(String foo, Tuple args, Dict<String, Object> kwargs) {
-      String argsString = debugPrintArgs(args, StarlarkSemantics.DEFAULT);
+        extraKeywords = @Param(name = "kwargs"),
+        useStarlarkThread = true)
+    public String withArgsAndKwargs(
+        String foo, Tuple args, Dict<String, Object> kwargs, StarlarkThread thread) {
+      String argsString = debugPrintArgs(args, thread);
       String kwargsString =
           "kwargs("
               + kwargs
@@ -436,12 +438,12 @@ public String toString() {
     }
   }
 
-  private static String debugPrintArgs(Iterable<?> args, StarlarkSemantics semantics) {
+  private static String debugPrintArgs(Iterable<?> args, StarlarkThread thread) {
     Printer p = new Printer();
     p.append("args(");
     String sep = "";
     for (Object arg : args) {
-      p.append(sep).debugPrint(arg, semantics);
+      p.append(sep).debugPrint(arg, thread);
       sep = ", ";
     }
     return p.append(")").toString();