diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/BazelBuildApiGlobals.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/BazelBuildApiGlobals.java index cee2be667f020f..1ca690c8abc6d2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/BazelBuildApiGlobals.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/BazelBuildApiGlobals.java @@ -52,7 +52,7 @@ public void visibility(Object value, StarlarkThread thread) throws EvalException // for static tooling to mechanically find and modify visibility() declarations.) ImmutableList callStack = thread.getCallStack(); if (!(callStack.size() == 2 - && callStack.get(0).name.equals("") + && callStack.get(0).name.equals(StarlarkThread.TOP_LEVEL) && callStack.get(1).name.equals("visibility"))) { throw Starlark.errorf( "load visibility may only be set at the top level, not inside a function"); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java index 46db0ab3413eef..98bc35e22a045b 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java @@ -34,6 +34,7 @@ import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Starlark; import net.starlark.java.eval.StarlarkSemantics; +import net.starlark.java.eval.StarlarkThread; import net.starlark.java.eval.StarlarkThread.CallStackEntry; import net.starlark.java.syntax.Location; @@ -71,7 +72,7 @@ public static Rule createRule( BuildLangTypedAttributeValuesMap attributeValues = new BuildLangTypedAttributeValuesMap(attributes); ImmutableList callStack = - ImmutableList.of(new CallStackEntry(callStackEntry, Location.BUILTIN)); + ImmutableList.of(StarlarkThread.callStackEntry(callStackEntry, Location.BUILTIN)); Rule rule; try { rule = diff --git a/src/main/java/com/google/devtools/build/lib/packages/CallStack.java b/src/main/java/com/google/devtools/build/lib/packages/CallStack.java index 25c01dbaae7869..d3498dcbe54342 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/CallStack.java +++ b/src/main/java/com/google/devtools/build/lib/packages/CallStack.java @@ -109,7 +109,7 @@ private Location toLocation() { } private StarlarkThread.CallStackEntry toCallStackEntry() { - return new StarlarkThread.CallStackEntry(name, toLocation()); + return StarlarkThread.callStackEntry(name, toLocation()); } @Nullable diff --git a/src/main/java/net/starlark/java/eval/CpuProfiler.java b/src/main/java/net/starlark/java/eval/CpuProfiler.java index fd2e8bbd1df51e..acdf800980d2cc 100644 --- a/src/main/java/net/starlark/java/eval/CpuProfiler.java +++ b/src/main/java/net/starlark/java/eval/CpuProfiler.java @@ -405,7 +405,7 @@ private long getFunctionID(StarlarkCallable fn, long addr) throws IOException { Location loc = fn.getLocation(); String filename = loc.file(); // TODO(adonovan): make WORKSPACE-relative String name = fn.getName(); - if (name.equals("")) { + if (name.equals(StarlarkThread.TOP_LEVEL)) { name = filename; } diff --git a/src/main/java/net/starlark/java/eval/StarlarkThread.java b/src/main/java/net/starlark/java/eval/StarlarkThread.java index bf57076f4e1b84..426e55294c581e 100644 --- a/src/main/java/net/starlark/java/eval/StarlarkThread.java +++ b/src/main/java/net/starlark/java/eval/StarlarkThread.java @@ -193,7 +193,7 @@ public ImmutableMap getLocals() { } } } - return env.build(); + return env.buildOrThrow(); } @Override @@ -430,7 +430,7 @@ public StarlarkSemantics getSemantics() { } /** Reports whether this thread is allowed to make recursive calls. */ - public boolean isRecursionAllowed() { + boolean isRecursionAllowed() { return allowRecursion; } @@ -455,16 +455,22 @@ StarlarkFunction getInnermostEnclosingStarlarkFunction(int depth) { return null; } - @Nullable - StarlarkFunction getInnermostEnclosingStarlarkFunction() { - return getInnermostEnclosingStarlarkFunction(0); - } - /** Returns the size of the callstack. This is needed for the debugger. */ int getCallStackSize() { return callstack.size(); } + /** + * The value of {@link CallStackEntry#name} for the implicit function that executes the top-level + * statements of a file. + */ + public static final String TOP_LEVEL = ""; + + /** Creates a new {@link CallStackEntry}. */ + public static CallStackEntry callStackEntry(String name, Location location) { + return new CallStackEntry(name, location); + } + /** * A CallStackEntry describes the name and PC location of an active function call. See {@link * #getCallStack}. @@ -474,15 +480,32 @@ public static final class CallStackEntry { public final String name; public final Location location; - public CallStackEntry(String name, Location location) { - this.location = location; - this.name = name; + private CallStackEntry(String name, Location location) { + this.name = Preconditions.checkNotNull(name); + this.location = Preconditions.checkNotNull(location); } @Override public String toString() { return name + "@" + location; } + + @Override + public int hashCode() { + return 31 * name.hashCode() + location.hashCode(); + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (!(o instanceof CallStackEntry)) { + return false; + } + CallStackEntry that = (CallStackEntry) o; + return name.equals(that.name) && location.equals(that.location); + } } /** @@ -495,7 +518,7 @@ public ImmutableList getCallStack() { ImmutableList.Builder stack = ImmutableList.builderWithExpectedSize(callstack.size()); for (Frame fr : callstack) { - stack.add(new CallStackEntry(fr.fn.getName(), fr.loc)); + stack.add(callStackEntry(fr.fn.getName(), fr.loc)); } return stack.build(); } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java index ffca31127b0fb4..27f63206f57e0f 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryContextTest.java @@ -120,12 +120,10 @@ private static Object exec(String... lines) { private static final ImmutableList DUMMY_STACK = ImmutableList.of( - new StarlarkThread.CallStackEntry( // - "", Location.fromFileLineColumn("BUILD", 10, 1)), - new StarlarkThread.CallStackEntry( // - "foo", Location.fromFileLineColumn("foo.bzl", 42, 1)), - new StarlarkThread.CallStackEntry( // - "myrule", Location.fromFileLineColumn("bar.bzl", 30, 6))); + StarlarkThread.callStackEntry( + StarlarkThread.TOP_LEVEL, Location.fromFileLineColumn("BUILD", 10, 1)), + StarlarkThread.callStackEntry("foo", Location.fromFileLineColumn("foo.bzl", 42, 1)), + StarlarkThread.callStackEntry("myrule", Location.fromFileLineColumn("bar.bzl", 30, 6))); private void setUpContextForRule( Map kwargs, diff --git a/src/test/java/com/google/devtools/build/lib/packages/CallStackTest.java b/src/test/java/com/google/devtools/build/lib/packages/CallStackTest.java index 4b62b63199e3e5..8e04fb6e6aa62c 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/CallStackTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/CallStackTest.java @@ -17,7 +17,6 @@ import static com.google.common.truth.Truth.assertThat; import com.google.common.collect.ImmutableList; -import com.google.common.truth.Correspondence; import com.google.protobuf.CodedInputStream; import com.google.protobuf.CodedOutputStream; import java.io.ByteArrayOutputStream; @@ -33,15 +32,6 @@ @RunWith(JUnit4.class) public class CallStackTest { - /** - * Compare {@link StarlarkThread.CallStackEntry} using string equality since (1) it doesn't - * currently implement equals and (2) it should have a faithful string representation anyway. - */ - private static final Correspondence - STACK_ENTRY_CORRESPONDENCE = - Correspondence.from( - (l, r) -> l.toString().equals(r.toString()), "String-representations equal"); - @Test public void testCreateFromEmptyCallStack() { CallStack result = CallStack.createFrom(ImmutableList.of()); @@ -151,10 +141,7 @@ public void testSerialization() throws Exception { /** Asserts the provided {@link CallStack} faithfully represents the expected stack. */ private static void assertCallStackContents(CallStack result, List expected) { assertThat(result.size()).isEqualTo(expected.size()); - assertThat(result.toList()) - .comparingElementsUsing(STACK_ENTRY_CORRESPONDENCE) - .containsExactlyElementsIn(expected) - .inOrder(); + assertThat(result.toList()).isEqualTo(expected); // toList and getFrame use different code paths, make sure they agree. for (int i = 0; i < expected.size(); i++) { assertThat(result.getFrame(i).toString()).isEqualTo(expected.get(i).toString()); @@ -163,6 +150,6 @@ private static void assertCallStackContents(CallStack result, List DUMMY_STACK = ImmutableList.of( - new StarlarkThread.CallStackEntry( - "", Location.fromFileLineColumn("BUILD", 10, 1)), - new StarlarkThread.CallStackEntry("bar", Location.fromFileLineColumn("bar.bzl", 42, 1)), - new StarlarkThread.CallStackEntry("rule", Location.BUILTIN)); + StarlarkThread.callStackEntry( + StarlarkThread.TOP_LEVEL, Location.fromFileLineColumn("BUILD", 10, 1)), + StarlarkThread.callStackEntry("bar", Location.fromFileLineColumn("bar.bzl", 42, 1)), + StarlarkThread.callStackEntry("rule", Location.BUILTIN)); private static final class DummyFragment extends Fragment {} diff --git a/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java index 4608f3a956abfd..997cfc49876236 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/packages/RuleFactoryTest.java @@ -51,11 +51,10 @@ public final class RuleFactoryTest extends PackageLoadingTestCase { private static final ImmutableList DUMMY_STACK = ImmutableList.of( - new StarlarkThread.CallStackEntry( - "", Location.fromFileLineColumn("BUILD", 42, 1)), - new StarlarkThread.CallStackEntry("foo", Location.fromFileLineColumn("foo.bzl", 10, 1)), - new StarlarkThread.CallStackEntry( - "myrule", Location.fromFileLineColumn("bar.bzl", 30, 6))); + StarlarkThread.callStackEntry( + StarlarkThread.TOP_LEVEL, Location.fromFileLineColumn("BUILD", 42, 1)), + StarlarkThread.callStackEntry("foo", Location.fromFileLineColumn("foo.bzl", 10, 1)), + StarlarkThread.callStackEntry("myrule", Location.fromFileLineColumn("bar.bzl", 30, 6))); private Package.Builder newBuilder(PackageIdentifier id, Path filename) { return packageFactory diff --git a/src/test/java/com/google/devtools/build/lib/starlarkdebug/server/StarlarkDebugServerTest.java b/src/test/java/com/google/devtools/build/lib/starlarkdebug/server/StarlarkDebugServerTest.java index 0df60245313926..1bf401dbb57d00 100644 --- a/src/test/java/com/google/devtools/build/lib/starlarkdebug/server/StarlarkDebugServerTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlarkdebug/server/StarlarkDebugServerTest.java @@ -331,7 +331,7 @@ public void testPauseAtInvalidConditionBreakpointWithError() throws Exception { .setPauseReason(PauseReason.CONDITIONAL_BREAKPOINT_ERROR) .setLocation(location.toBuilder().setColumnNumber(1)) .setConditionalBreakpointError( - StarlarkDebuggingProtos.Error.newBuilder().setMessage("name \'z\' is not defined")) + StarlarkDebuggingProtos.Error.newBuilder().setMessage("name 'z' is not defined")) .build(); assertThat(event).isEqualTo(DebugEventHelper.threadPausedEvent(expectedThreadState)); @@ -370,7 +370,7 @@ public void testSimpleListFramesRequest() throws Exception { assertFramesEqualIgnoringValueIdentifiers( frames.getFrame(0), Frame.newBuilder() - .setFunctionName("") + .setFunctionName(StarlarkThread.TOP_LEVEL) .setLocation(breakpoint.toBuilder().setColumnNumber(1)) .addScope( Scope.newBuilder() @@ -463,7 +463,7 @@ public void testListFramesShadowedBinding() throws Exception { assertFramesEqualIgnoringValueIdentifiers( frames.getFrame(1), Frame.newBuilder() - .setFunctionName("") + .setFunctionName(StarlarkThread.TOP_LEVEL) .setLocation( Location.newBuilder() .setPath("/a/build/file/test.bzl") @@ -802,7 +802,7 @@ private static Thread execInWorkerThread(ParserInput input, @Nullable Module mod /** * Asserts that the given frames are equal after clearing the identifier from all {@link Value}s. */ - private void assertFramesEqualIgnoringValueIdentifiers(Frame frame1, Frame frame2) { + private static void assertFramesEqualIgnoringValueIdentifiers(Frame frame1, Frame frame2) { assertThat(clearIds(frame1)).isEqualTo(clearIds(frame2)); } @@ -822,7 +822,7 @@ private static Scope clearIds(Scope scope) { return builder.build(); } - private void assertValuesEqualIgnoringId(Value value1, Value value2) { + private static void assertValuesEqualIgnoringId(Value value1, Value value2) { assertThat(clearId(value1)).isEqualTo(clearId(value2)); } diff --git a/src/test/java/net/starlark/java/eval/StarlarkThreadDebuggingTest.java b/src/test/java/net/starlark/java/eval/StarlarkThreadDebuggingTest.java index 473b8f5b95fcd5..c353b78a7fb748 100644 --- a/src/test/java/net/starlark/java/eval/StarlarkThreadDebuggingTest.java +++ b/src/test/java/net/starlark/java/eval/StarlarkThreadDebuggingTest.java @@ -50,7 +50,7 @@ private static StarlarkFunction defineFunc() throws Exception { } @Test - public void testListFramesEmptyStack() throws Exception { + public void testListFramesEmptyStack() { StarlarkThread thread = newThread(); assertThat(Debug.getCallStack(thread)).isEmpty(); assertThat(thread.getCallStack()).isEmpty(); @@ -222,7 +222,7 @@ public void testEvaluateVariableInScope() throws Exception { } @Test - public void testEvaluateVariableNotInScopeFails() throws Exception { + public void testEvaluateVariableNotInScopeFails() { Module module = Module.create(); SyntaxError.Exception e =