Skip to content

Commit a3e26ed

Browse files
tetrominocopybara-github
authored andcommitted
Set macro symbol's exported location as MacroFunction's callable location for Starlark stack
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
1 parent 70efe0e commit a3e26ed

File tree

16 files changed

+66
-22
lines changed

16 files changed

+66
-22
lines changed

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1426,6 +1426,9 @@ public static final class MacroFunction implements StarlarkExportable, MacroFunc
14261426
// Initially null, then non-null once exported.
14271427
@Nullable private MacroClass macroClass = null;
14281428

1429+
// Initially null, then non-null once exported.
1430+
@Nullable private Location exportedLocation = null;
1431+
14291432
/** A token used for equality that may be mutated by {@link #export}. */
14301433
private Symbol<BzlLoadValue.Key> identityToken;
14311434

@@ -1445,6 +1448,11 @@ public String getName() {
14451448
return macroClass != null ? macroClass.getName() : "unexported macro";
14461449
}
14471450

1451+
@Override
1452+
public Location getLocation() {
1453+
return exportedLocation != null ? exportedLocation : Location.BUILTIN;
1454+
}
1455+
14481456
/**
14491457
* Returns the value of the doc parameter passed to {@code macro()} in Starlark, or an empty
14501458
* Optional if a doc string was not provided.
@@ -1527,7 +1535,8 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
15271535

15281536
/** Export a MacroFunction from a Starlark file with a given name. */
15291537
@Override
1530-
public void export(EventHandler handler, Label starlarkLabel, String exportedName) {
1538+
public void export(
1539+
EventHandler handler, Label starlarkLabel, String exportedName, Location exportedLocation) {
15311540
checkState(builder != null && macroClass == null);
15321541
builder.setName(exportedName);
15331542
builder.setDefiningBzlLabel(starlarkLabel);
@@ -1540,6 +1549,7 @@ public void export(EventHandler handler, Label starlarkLabel, String exportedNam
15401549
starlarkLabel,
15411550
exportedName);
15421551
this.identityToken = identityToken.exportAs(exportedName);
1552+
this.exportedLocation = exportedLocation;
15431553
}
15441554

15451555
/**
@@ -1787,8 +1797,13 @@ private static void validateRulePropagatedAspects(RuleClass ruleClass) throws Ev
17871797
}
17881798

17891799
/** Export a RuleFunction from a Starlark file with a given name. */
1800+
// TODO(bazel-team): use exportedLocation as the callable symbol's location.
17901801
@Override
1791-
public void export(EventHandler handler, Label starlarkLabel, String ruleClassName) {
1802+
public void export(
1803+
EventHandler handler,
1804+
Label starlarkLabel,
1805+
String ruleClassName,
1806+
Location exportedLocation) {
17921807
checkState(ruleClass == null && builder != null);
17931808
var symbolToken = (Symbol<?>) identityToken; // always a Symbol before export
17941809
this.identityToken =

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import net.starlark.java.eval.StarlarkSemantics;
6161
import net.starlark.java.eval.StarlarkThread;
6262
import net.starlark.java.eval.Tuple;
63+
import net.starlark.java.syntax.Location;
6364

6465
/**
6566
* Represents a subrule which can be invoked in a Starlark rule's implementation function.
@@ -252,8 +253,10 @@ public boolean isExported() {
252253
return this.extensionLabel != null && this.exportedName != null;
253254
}
254255

256+
// TODO(bazel-team): use exportedLocation as the callable symbol's location.
255257
@Override
256-
public void export(EventHandler handler, Label extensionLabel, String exportedName) {
258+
public void export(
259+
EventHandler handler, Label extensionLabel, String exportedName, Location exportedLocation) {
257260
Preconditions.checkState(!isExported());
258261
this.extensionLabel = extensionLabel;
259262
this.exportedName = exportedName;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -554,10 +554,10 @@ private static ModuleThreadContext execModuleFile(
554554
thread.setPrintHandler(Event.makeDebugPrintHandler(eventHandler));
555555
}
556556
thread.setPostAssignHook(
557-
(name, value) -> {
557+
(name, nameStartLocation, value) -> {
558558
if (value instanceof StarlarkExportable exportable) {
559559
if (!exportable.isExported()) {
560-
exportable.export(eventHandler, null, name);
560+
exportable.export(eventHandler, null, name, nameStartLocation);
561561
}
562562
}
563563
});

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import net.starlark.java.eval.Structure;
5353
import net.starlark.java.eval.Tuple;
5454
import net.starlark.java.syntax.Identifier;
55+
import net.starlark.java.syntax.Location;
5556

5657
/** A collection of global Starlark build API functions that apply to MODULE.bazel files. */
5758
@GlobalMethods(environment = Environment.MODULE)
@@ -623,7 +624,8 @@ public boolean isExported() {
623624
}
624625

625626
@Override
626-
public void export(EventHandler handler, Label bzlFileLabel, String name) {
627+
public void export(
628+
EventHandler handler, Label bzlFileLabel, String name, Location exportedLocation) {
627629
proxyBuilder.setProxyName(name);
628630
}
629631
}

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
import net.starlark.java.eval.StarlarkCallable;
5757
import net.starlark.java.eval.StarlarkThread;
5858
import net.starlark.java.eval.Tuple;
59+
import net.starlark.java.syntax.Location;
5960

6061
/**
6162
* The Starlark module containing the definition of {@code repository_rule} function to define a
@@ -138,7 +139,7 @@ public StarlarkCallable repositoryRule(
138139
name = "repository_rule",
139140
category = DocCategory.BUILTIN,
140141
doc =
141-
"""
142+
"""
142143
A callable value that may be invoked during evaluation of the WORKSPACE file or within \
143144
the implementation function of a module extension to instantiate and return a repository \
144145
rule. Created by \
@@ -191,8 +192,13 @@ public boolean isImmutable() {
191192
return true;
192193
}
193194

195+
// TODO(bazel-team): use exportedLocation as the callable symbol's location.
194196
@Override
195-
public void export(EventHandler handler, Label extensionLabel, String exportedName) {
197+
public void export(
198+
EventHandler handler,
199+
Label extensionLabel,
200+
String exportedName,
201+
Location exportedLocation) {
196202
this.extensionLabel = extensionLabel;
197203
this.exportedName = exportedName;
198204
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import net.starlark.java.eval.StarlarkCallable;
4343
import net.starlark.java.eval.StarlarkInt;
4444
import net.starlark.java.eval.SymbolGenerator.Symbol;
45+
import net.starlark.java.syntax.Location;
4546

4647
/** A Starlark value that is a result of an 'aspect(..)' function call. */
4748
public final class StarlarkDefinedAspect implements StarlarkExportable, StarlarkAspect {
@@ -177,8 +178,10 @@ public ImmutableSet<String> getParamAttributes() {
177178
return paramAttributes;
178179
}
179180

181+
// TODO(bazel-team): use exportedLocation as the callable symbol's location.
180182
@Override
181-
public void export(EventHandler handler, Label extensionLabel, String name) {
183+
public void export(
184+
EventHandler handler, Label extensionLabel, String name, Location exportedLocation) {
182185
Preconditions.checkArgument(!isExported());
183186
@SuppressWarnings("unchecked")
184187
var identityToken = (Symbol<BzlLoadValue.Key>) aspectClassOrIdentityToken;

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import com.google.devtools.build.lib.cmdline.Label;
1818
import com.google.devtools.build.lib.events.EventHandler;
1919
import net.starlark.java.eval.StarlarkValue;
20+
import net.starlark.java.syntax.Location;
2021

2122
/**
2223
* {@link StarlarkValue}s that need special handling when they are exported from an extension file.
@@ -31,7 +32,8 @@ public interface StarlarkExportable extends StarlarkValue {
3132

3233
/**
3334
* Notify the value that it is exported from {@code extensionLabel} extension with name {@code
34-
* exportedName}.
35+
* exportedName} at {@code exportedLocation}.
3536
*/
36-
void export(EventHandler handler, Label extensionLabel, String exportedName);
37+
void export(
38+
EventHandler handler, Label extensionLabel, String exportedName, Location exportedLocation);
3739
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,8 +467,10 @@ public String getErrorMessageForUnknownField(String name) {
467467
"'%s' value has no field or method '%s'", isExported() ? getName() : "struct", name);
468468
}
469469

470+
// TODO(bazel-team): use exportedLocation as the callable symbol's location.
470471
@Override
471-
public void export(EventHandler handler, Label extensionLabel, String exportedName) {
472+
public void export(
473+
EventHandler handler, Label extensionLabel, String exportedName, Location exportedLocation) {
472474
Preconditions.checkState(!isExported());
473475
SymbolGenerator.Symbol<?> identifier = (SymbolGenerator.Symbol<?>) keyOrIdentityToken;
474476
if (identifier.getOwner() instanceof BzlLoadValue.Key bzlKey) {

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ java_library(
9898
"//src/main/java/com/google/devtools/build/lib/util",
9999
"//src/main/java/com/google/devtools/build/lib/vfs",
100100
"//src/main/java/net/starlark/java/eval",
101+
"//src/main/java/net/starlark/java/syntax",
101102
"//src/main/protobuf:test_status_java_proto",
102103
"//third_party:guava",
103104
"//third_party:jsr305",

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import net.starlark.java.eval.StarlarkList;
3636
import net.starlark.java.eval.StarlarkThread;
3737
import net.starlark.java.eval.Tuple;
38+
import net.starlark.java.syntax.Location;
3839

3940
/** A class that exposes testing infrastructure to Starlark. */
4041
public class StarlarkTestingModule implements TestingModuleApi {
@@ -160,7 +161,9 @@ public void analysisTest(
160161
starlarkRuleFunction.export(
161162
handler,
162163
pkgBuilder.getMetadata().buildFileLabel(),
163-
name + "_test"); // export in BUILD thread
164+
name + "_test",
165+
Location.fromFile(
166+
pkgBuilder.getMetadata().buildFilename().toString())); // export in BUILD thread
164167
if (handler.hasErrors()) {
165168
StringBuilder errors =
166169
handler.getEvents().stream()

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1463,10 +1463,10 @@ public static void execAndExport(
14631463
// and "export" any newly assigned exportable globals.
14641464
// TODO(adonovan): change the semantics; see b/65374671.
14651465
thread.setPostAssignHook(
1466-
(name, value) -> {
1466+
(name, nameStartLocation, value) -> {
14671467
if (value instanceof StarlarkExportable exp) {
14681468
if (!exp.isExported()) {
1469-
exp.export(handler, label, name);
1469+
exp.export(handler, label, name, nameStartLocation);
14701470
}
14711471
}
14721472
});

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ private static TokenKind execStatements(
9595
// identifier when available. This enables an name-based lookup on deserialization.
9696
func.export(fr.thread, id.getName());
9797
} else {
98-
fr.thread.postAssignHook.assign(id.getName(), value);
98+
fr.thread.postAssignHook.assign(id.getName(), id.getStartLocation(), value);
9999
}
100100
}
101101
} else if (stmt instanceof DefStatement def) {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ public void setPostAssignHook(PostAssignHook postAssignHook) {
475475
/** A hook for notifications of assignments at top level. */
476476
@FunctionalInterface
477477
public interface PostAssignHook {
478-
void assign(String name, Object value);
478+
void assign(String name, Location nameStartLocation, Object value);
479479
}
480480

481481
public StarlarkSemantics getSemantics() {

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2073,7 +2073,8 @@ def _impl(name, visibility, **kwargs):
20732073
assertThat(foo.reconstructParentCallStack())
20742074
.containsExactly(
20752075
StarlarkThread.callStackEntry(StarlarkThread.TOP_LEVEL, foo.getBuildFileLocation()),
2076-
StarlarkThread.callStackEntry("my_macro", Location.BUILTIN))
2076+
StarlarkThread.callStackEntry(
2077+
"my_macro", Location.fromFileLineColumn("/workspace/pkg/my_macro.bzl", 7, 1)))
20772078
.inOrder();
20782079

20792080
Rule fooLib = pkg.getRule("foo_lib");
@@ -2161,7 +2162,8 @@ def outer_legacy_wrapper(name, **kwargs):
21612162
StarlarkThread.callStackEntry(
21622163
"outer_legacy_wrapper",
21632164
Location.fromFileLineColumn("/workspace/pkg/outer.bzl", 9, 16)),
2164-
StarlarkThread.callStackEntry("outer_macro", Location.BUILTIN))
2165+
StarlarkThread.callStackEntry(
2166+
"outer_macro", Location.fromFileLineColumn("/workspace/pkg/outer.bzl", 6, 1)))
21652167
.inOrder();
21662168

21672169
MacroInstance fooInner = getMacroById(pkg, "foo_inner:1");
@@ -2173,7 +2175,8 @@ def outer_legacy_wrapper(name, **kwargs):
21732175
StarlarkThread.callStackEntry(
21742176
"inner_legacy_wrapper",
21752177
Location.fromFileLineColumn("/workspace/pkg/inner.bzl", 7, 16)),
2176-
StarlarkThread.callStackEntry("inner_macro", Location.BUILTIN))
2178+
StarlarkThread.callStackEntry(
2179+
"inner_macro", Location.fromFileLineColumn("/workspace/pkg/inner.bzl", 4, 1)))
21772180
.inOrder();
21782181

21792182
Rule fooLib = pkg.getRule("foo_inner");

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,11 @@ private static final class P3 implements TransitiveInfoProvider {}
4747

4848
static {
4949
try {
50-
P_STARLARK.export(ev -> {}, Label.create("foo/bar", "x.bzl"), "p_starlark");
50+
P_STARLARK.export(
51+
ev -> {},
52+
Label.create("foo/bar", "x.bzl"),
53+
"p_starlark",
54+
Location.fromFile("/workspace/foo/bar/x.bzl"));
5155
} catch (LabelSyntaxException e) {
5256
throw new AssertionError(e);
5357
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ StarlarkThread getStarlarkThread() {
9494
StarlarkThread.create(
9595
mu, semantics, /* contextDescription= */ "", SymbolGenerator.create("test"));
9696
// Sets a post-assign hook to enable global export of StarlarkFunction Symbols.
97-
this.thread.setPostAssignHook((unusedName, unusedValue) -> {});
97+
this.thread.setPostAssignHook((unusedName, unusedLocation, unusedValue) -> {});
9898
}
9999
return this.thread;
100100
}

0 commit comments

Comments
 (0)