[8.2.0] Set macro symbol's exported location as MacroFunction's callable location for Starlark stack (#25802)

This ensures Starlark instantiation stack for rule targets defined in a
symbolic macro shows both the macro's location and symbol.

Requires changing StarlarkThread to track the location of where symbols
are exported.

As follow-up, we should do the same for rules, providers, and aspects.

PiperOrigin-RevId: 745731053
Change-Id: I6732a9762fbc932d823a70d7753a2b6edc140df6

Commit

a3e26ed

Author: tetromino

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

@@ -1398,6 +1398,9 @@ public static final class MacroFunction implements StarlarkExportable, MacroFunc
     // Initially null, then non-null once exported.
     @Nullable private MacroClass macroClass = null;
 
+    // Initially null, then non-null once exported.
+    @Nullable private Location exportedLocation = null;
+
     /** A token used for equality that may be mutated by {@link #export}. */
     private Symbol<BzlLoadValue.Key> identityToken;
 
@@ -1417,6 +1420,11 @@ public String getName() {
       return macroClass != null ? macroClass.getName() : "unexported macro";
     }
 
+    @Override
+    public Location getLocation() {
+      return exportedLocation != null ? exportedLocation : Location.BUILTIN;
+    }
+
     /**
      * Returns the value of the doc parameter passed to {@code macro()} in Starlark, or an empty
      * Optional if a doc string was not provided.
@@ -1495,7 +1503,8 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
 
     /** Export a MacroFunction from a Starlark file with a given name. */
     @Override
-    public void export(EventHandler handler, Label starlarkLabel, String exportedName) {
+    public void export(
+        EventHandler handler, Label starlarkLabel, String exportedName, Location exportedLocation) {
       checkState(builder != null && macroClass == null);
       builder.setName(exportedName);
       builder.setDefiningBzlLabel(starlarkLabel);
@@ -1508,6 +1517,7 @@ public void export(EventHandler handler, Label starlarkLabel, String exportedNam
           starlarkLabel,
           exportedName);
       this.identityToken = identityToken.exportAs(exportedName);
+      this.exportedLocation = exportedLocation;
     }
 
     /**
@@ -1755,8 +1765,13 @@ private static void validateRulePropagatedAspects(RuleClass ruleClass) throws Ev
     }
 
     /** Export a RuleFunction from a Starlark file with a given name. */
+    // TODO(bazel-team): use exportedLocation as the callable symbol's location.
     @Override
-    public void export(EventHandler handler, Label starlarkLabel, String ruleClassName) {
+    public void export(
+        EventHandler handler,
+        Label starlarkLabel,
+        String ruleClassName,
+        Location exportedLocation) {
       checkState(ruleClass == null && builder != null);
       var symbolToken = (Symbol<?>) identityToken; // always a Symbol before export
       this.identityToken =

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubrule.java

@@ -60,6 +60,7 @@
 import net.starlark.java.eval.StarlarkSemantics;
 import net.starlark.java.eval.StarlarkThread;
 import net.starlark.java.eval.Tuple;
+import net.starlark.java.syntax.Location;
 
 /**
  * Represents a subrule which can be invoked in a Starlark rule's implementation function.
@@ -247,8 +248,10 @@ public boolean isExported() {
     return this.extensionLabel != null && this.exportedName != null;
   }
 
+  // TODO(bazel-team): use exportedLocation as the callable symbol's location.
   @Override
-  public void export(EventHandler handler, Label extensionLabel, String exportedName) {
+  public void export(
+      EventHandler handler, Label extensionLabel, String exportedName, Location exportedLocation) {
     Preconditions.checkState(!isExported());
     this.extensionLabel = extensionLabel;
     this.exportedName = exportedName;

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java

@@ -554,10 +554,10 @@ private static ModuleThreadContext execModuleFile(
         thread.setPrintHandler(Event.makeDebugPrintHandler(eventHandler));
       }
       thread.setPostAssignHook(
-          (name, value) -> {
+          (name, nameStartLocation, value) -> {
             if (value instanceof StarlarkExportable exportable) {
               if (!exportable.isExported()) {
-                exportable.export(eventHandler, null, name);
+                exportable.export(eventHandler, null, name, nameStartLocation);
               }
             }
           });

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java

@@ -52,6 +52,7 @@
 import net.starlark.java.eval.Structure;
 import net.starlark.java.eval.Tuple;
 import net.starlark.java.syntax.Identifier;
+import net.starlark.java.syntax.Location;
 
 /** A collection of global Starlark build API functions that apply to MODULE.bazel files. */
 @GlobalMethods(environment = Environment.MODULE)
@@ -624,7 +625,8 @@ public boolean isExported() {
     }
 
     @Override
-    public void export(EventHandler handler, Label bzlFileLabel, String name) {
+    public void export(
+        EventHandler handler, Label bzlFileLabel, String name, Location exportedLocation) {
       proxyBuilder.setProxyName(name);
     }
   }

src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java

@@ -56,6 +56,7 @@
 import net.starlark.java.eval.StarlarkCallable;
 import net.starlark.java.eval.StarlarkThread;
 import net.starlark.java.eval.Tuple;
+import net.starlark.java.syntax.Location;
 
 /**
  * The Starlark module containing the definition of {@code repository_rule} function to define a
@@ -138,7 +139,7 @@ public StarlarkCallable repositoryRule(
       name = "repository_rule",
       category = DocCategory.BUILTIN,
       doc =
-          """
+"""
 A callable value that may be invoked during evaluation of the WORKSPACE file or within \
 the implementation function of a module extension to instantiate and return a repository \
 rule. Created by \
@@ -191,8 +192,13 @@ public boolean isImmutable() {
       return true;
     }
 
+    // TODO(bazel-team): use exportedLocation as the callable symbol's location.
     @Override
-    public void export(EventHandler handler, Label extensionLabel, String exportedName) {
+    public void export(
+        EventHandler handler,
+        Label extensionLabel,
+        String exportedName,
+        Location exportedLocation) {
       this.extensionLabel = extensionLabel;
       this.exportedName = exportedName;
     }

src/main/java/com/google/devtools/build/lib/packages/StarlarkDefinedAspect.java

@@ -42,6 +42,7 @@
 import net.starlark.java.eval.StarlarkCallable;
 import net.starlark.java.eval.StarlarkInt;
 import net.starlark.java.eval.SymbolGenerator.Symbol;
+import net.starlark.java.syntax.Location;
 
 /** A Starlark value that is a result of an 'aspect(..)' function call. */
 public final class StarlarkDefinedAspect implements StarlarkExportable, StarlarkAspect {
@@ -168,8 +169,10 @@ public ImmutableSet<String> getParamAttributes() {
     return paramAttributes;
   }
 
+  // TODO(bazel-team): use exportedLocation as the callable symbol's location.
   @Override
-  public void export(EventHandler handler, Label extensionLabel, String name) {
+  public void export(
+      EventHandler handler, Label extensionLabel, String name, Location exportedLocation) {
     Preconditions.checkArgument(!isExported());
     @SuppressWarnings("unchecked")
     var identityToken = (Symbol<BzlLoadValue.Key>) aspectClassOrIdentityToken;

src/main/java/com/google/devtools/build/lib/packages/StarlarkExportable.java

@@ -17,6 +17,7 @@
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.EventHandler;
 import net.starlark.java.eval.StarlarkValue;
+import net.starlark.java.syntax.Location;
 
 /**
  * {@link StarlarkValue}s that need special handling when they are exported from an extension file.
@@ -31,7 +32,8 @@ public interface StarlarkExportable extends StarlarkValue {
 
   /**
    * Notify the value that it is exported from {@code extensionLabel} extension with name {@code
-   * exportedName}.
+   * exportedName} at {@code exportedLocation}.
    */
-  void export(EventHandler handler, Label extensionLabel, String exportedName);
+  void export(
+      EventHandler handler, Label extensionLabel, String exportedName, Location exportedLocation);
 }

src/main/java/com/google/devtools/build/lib/packages/StarlarkProvider.java

@@ -394,8 +394,10 @@ public String getErrorMessageForUnknownField(String name) {
         "'%s' value has no field or method '%s'", isExported() ? getName() : "struct", name);
   }
 
+  // TODO(bazel-team): use exportedLocation as the callable symbol's location.
   @Override
-  public void export(EventHandler handler, Label extensionLabel, String exportedName) {
+  public void export(
+      EventHandler handler, Label extensionLabel, String exportedName, Location exportedLocation) {
     Preconditions.checkState(!isExported());
     SymbolGenerator.Symbol<?> identifier = (SymbolGenerator.Symbol<?>) keyOrIdentityToken;
     if (identifier.getOwner() instanceof BzlLoadValue.Key bzlKey) {

src/main/java/com/google/devtools/build/lib/packages/TargetDefinitionContext.java

@@ -216,6 +216,10 @@ public T intern(T sample) {
             pkg, buildFileLabel, Location.fromFile(metadata.buildFilename().asPath().toString())));
   }
 
+  public Metadata getMetadata() {
+    return metadata;
+  }
+
   SymbolGenerator<?> getSymbolGenerator() {
     return symbolGenerator;
   }

src/main/java/com/google/devtools/build/lib/rules/BUILD

@@ -98,6 +98,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/util",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/net/starlark/java/eval",
+        "//src/main/java/net/starlark/java/syntax",
         "//src/main/protobuf:test_status_java_proto",
         "//third_party:guava",
         "//third_party:jsr305",

src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java

@@ -35,6 +35,7 @@
 import net.starlark.java.eval.StarlarkList;
 import net.starlark.java.eval.StarlarkThread;
 import net.starlark.java.eval.Tuple;
+import net.starlark.java.syntax.Location;
 
 /** A class that exposes testing infrastructure to Starlark. */
 public class StarlarkTestingModule implements TestingModuleApi {
@@ -158,7 +159,11 @@ public void analysisTest(
     // evaluation in BzlLoadFunction#execAndExport.
     StoredEventHandler handler = new StoredEventHandler();
     starlarkRuleFunction.export(
-        handler, pkgBuilder.getBuildFileLabel(), name + "_test"); // export in BUILD thread
+        handler,
+        pkgBuilder.getBuildFileLabel(),
+        name + "_test",
+        Location.fromFile(
+            pkgBuilder.getMetadata().buildFilename().toString())); // export in BUILD thread
     if (handler.hasErrors()) {
       StringBuilder errors =
           handler.getEvents().stream()

src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java

@@ -1462,10 +1462,10 @@ public static void execAndExport(
     // and "export" any newly assigned exportable globals.
     // TODO(adonovan): change the semantics; see b/65374671.
     thread.setPostAssignHook(
-        (name, value) -> {
+        (name, nameStartLocation, value) -> {
           if (value instanceof StarlarkExportable exp) {
             if (!exp.isExported()) {
-              exp.export(handler, label, name);
+              exp.export(handler, label, name, nameStartLocation);
             }
           }
         });

src/main/java/net/starlark/java/eval/Eval.java

@@ -100,7 +100,7 @@ private static TokenKind execStatements(
               // identifier when available. This enables an name-based lookup on deserialization.
               ((StarlarkFunction) value).export(fr.thread, id.getName());
             } else {
-              fr.thread.postAssignHook.assign(id.getName(), value);
+              fr.thread.postAssignHook.assign(id.getName(), id.getStartLocation(), value);
             }
           }
         } else if (stmt instanceof DefStatement) {

src/main/java/net/starlark/java/eval/StarlarkThread.java

@@ -475,7 +475,7 @@ public void setPostAssignHook(PostAssignHook postAssignHook) {
   /** A hook for notifications of assignments at top level. */
   @FunctionalInterface
   public interface PostAssignHook {
-    void assign(String name, Object value);
+    void assign(String name, Location nameStartLocation, Object value);
   }
 
   public StarlarkSemantics getSemantics() {

src/test/java/com/google/devtools/build/lib/analysis/SymbolicMacroTest.java

@@ -2020,7 +2020,8 @@ def _impl(name, visibility, **kwargs):
     assertThat(foo.reconstructParentCallStack())
         .containsExactly(
             StarlarkThread.callStackEntry(StarlarkThread.TOP_LEVEL, foo.getBuildFileLocation()),
-            StarlarkThread.callStackEntry("my_macro", Location.BUILTIN))
+            StarlarkThread.callStackEntry(
+                "my_macro", Location.fromFileLineColumn("/workspace/pkg/my_macro.bzl", 7, 1)))
         .inOrder();
 
     Rule fooLib = pkg.getRule("foo_lib");
@@ -2108,7 +2109,8 @@ def outer_legacy_wrapper(name, **kwargs):
             StarlarkThread.callStackEntry(
                 "outer_legacy_wrapper",
                 Location.fromFileLineColumn("/workspace/pkg/outer.bzl", 9, 16)),
-            StarlarkThread.callStackEntry("outer_macro", Location.BUILTIN))
+            StarlarkThread.callStackEntry(
+                "outer_macro", Location.fromFileLineColumn("/workspace/pkg/outer.bzl", 6, 1)))
         .inOrder();
 
     MacroInstance fooInner = getMacroById(pkg, "foo_inner:1");
@@ -2120,7 +2122,8 @@ def outer_legacy_wrapper(name, **kwargs):
             StarlarkThread.callStackEntry(
                 "inner_legacy_wrapper",
                 Location.fromFileLineColumn("/workspace/pkg/inner.bzl", 7, 16)),
-            StarlarkThread.callStackEntry("inner_macro", Location.BUILTIN))
+            StarlarkThread.callStackEntry(
+                "inner_macro", Location.fromFileLineColumn("/workspace/pkg/inner.bzl", 4, 1)))
         .inOrder();
 
     Rule fooLib = pkg.getRule("foo_inner");

src/test/java/com/google/devtools/build/lib/packages/RequiredProvidersTest.java

@@ -47,7 +47,11 @@ private static final class P3 implements TransitiveInfoProvider {}
 
   static {
     try {
-      P_STARLARK.export(ev -> {}, Label.create("foo/bar", "x.bzl"), "p_starlark");
+      P_STARLARK.export(
+          ev -> {},
+          Label.create("foo/bar", "x.bzl"),
+          "p_starlark",
+          Location.fromFile("/workspace/foo/bar/x.bzl"));
     } catch (LabelSyntaxException e) {
       throw new AssertionError(e);
     }

src/test/java/net/starlark/java/eval/EvaluationTestCase.java

@@ -94,7 +94,7 @@ StarlarkThread getStarlarkThread() {
           StarlarkThread.create(
               mu, semantics, /* contextDescription= */ "", SymbolGenerator.create("test"));
       // Sets a post-assign hook to enable global export of StarlarkFunction Symbols.
-      this.thread.setPostAssignHook((unusedName, unusedValue) -> {});
+      this.thread.setPostAssignHook((unusedName, unusedLocation, unusedValue) -> {});
     }
     return this.thread;
   }