Add a flag that makes two Starlark functions equal if they are defined at the same location.
This flag, when set, mostly reverts https://github.com/bazelbuild/bazel/commit/af7fc3b75a62edc3b4b62de25b733b03c3ff925e (only mostly because it only applies to user-defined functions)
BUG=138789815
RELNOTES: None.
PiperOrigin-RevId: 261299731
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
index 2e384a7..061c986 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java
@@ -243,7 +243,8 @@
};
@Override
- public Provider provider(String doc, Object fields, Location location) throws EvalException {
+ public Provider provider(String doc, Object fields, Location location, Environment env)
+ throws EvalException {
Iterable<String> fieldNames = null;
if (fields instanceof SkylarkList<?>) {
@SuppressWarnings("unchecked")
@@ -260,7 +261,8 @@
"Expected list of strings or dictionary of string -> string for 'fields'");
fieldNames = dict.keySet();
}
- return SkylarkProvider.createUnexportedSchemaful(fieldNames, location);
+ return SkylarkProvider.createUnexportedSchemaful(
+ fieldNames, location, env.getSemantics().experimentalFunctionEqualityByLocation());
}
// TODO(bazel-team): implement attribute copy and other rule properties
diff --git a/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java b/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java
index e7375ec..acc739e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/NativeProvider.java
@@ -78,7 +78,7 @@
Class<V> valueClass,
String name,
FunctionSignature.WithValues<Object, SkylarkType> signature) {
- super(name, signature, Location.BUILTIN);
+ super(name, signature, Location.BUILTIN, /* equalityByLocation */ false);
Class<? extends NativeProvider<?>> clazz = (Class<? extends NativeProvider<?>>) getClass();
key = new NativeKey(name, clazz);
this.valueClass = valueClass;
diff --git a/src/main/java/com/google/devtools/build/lib/packages/ProviderFromFunction.java b/src/main/java/com/google/devtools/build/lib/packages/ProviderFromFunction.java
index a293393..6edd60e 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/ProviderFromFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/ProviderFromFunction.java
@@ -42,8 +42,9 @@
protected ProviderFromFunction(
@Nullable String name,
FunctionSignature.WithValues<Object, SkylarkType> signature,
- Location location) {
- super(name, signature, location);
+ Location location,
+ boolean equalityByLocation) {
+ super(name, signature, location, equalityByLocation);
}
public SkylarkProviderIdentifier id() {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java
index 4d42b45..c3d9c33 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkProvider.java
@@ -75,8 +75,10 @@
* @param location the location of the Skylark definition for this provider (tests may use {@link
* Location#BUILTIN})
*/
+ @VisibleForTesting
public static SkylarkProvider createUnexportedSchemaless(Location location) {
- return new SkylarkProvider(/*key=*/ null, /*fields=*/ null, location);
+ return new SkylarkProvider(
+ /*key=*/ null, /*fields=*/ null, location, /* equalityByLocation */ false);
}
/**
@@ -90,9 +92,12 @@
* Location#BUILTIN})
*/
public static SkylarkProvider createUnexportedSchemaful(
- Iterable<String> fields, Location location) {
+ Iterable<String> fields, Location location, boolean equalityByLocation) {
return new SkylarkProvider(
- /*key=*/ null, fields == null ? null : ImmutableList.copyOf(fields), location);
+ /*key=*/ null,
+ fields == null ? null : ImmutableList.copyOf(fields),
+ location,
+ equalityByLocation);
}
/**
@@ -102,8 +107,9 @@
* @param location the location of the Skylark definition for this provider (tests may use {@link
* Location#BUILTIN})
*/
+ @VisibleForTesting
public static SkylarkProvider createExportedSchemaless(SkylarkKey key, Location location) {
- return new SkylarkProvider(key, /*fields=*/ null, location);
+ return new SkylarkProvider(key, /*fields=*/ null, location, /* equalityByLocation */ false);
}
/**
@@ -115,9 +121,14 @@
* @param location the location of the Skylark definition for this provider (tests may use {@link
* Location#BUILTIN})
*/
+ @VisibleForTesting
public static SkylarkProvider createExportedSchemaful(
SkylarkKey key, Iterable<String> fields, Location location) {
- return new SkylarkProvider(key, fields == null ? null : ImmutableList.copyOf(fields), location);
+ return new SkylarkProvider(
+ key,
+ fields == null ? null : ImmutableList.copyOf(fields),
+ location, /* equalityByLocation */
+ false);
}
/**
@@ -127,10 +138,13 @@
* is schemaless.
*/
private SkylarkProvider(
- @Nullable SkylarkKey key, @Nullable ImmutableList<String> fields, Location location) {
+ @Nullable SkylarkKey key,
+ @Nullable ImmutableList<String> fields,
+ Location location,
+ boolean equalityByLocation) {
// We override getName() in order to use the name that is assigned when export() is called.
// Hence BaseFunction's constructor gets a null name.
- super(/*name=*/ null, buildSignature(fields), location);
+ super(/*name=*/ null, buildSignature(fields), location, equalityByLocation);
this.layout = fields == null ? null : new Layout(fields);
this.key = key; // possibly null
this.errorMessageFormatForUnknownField =
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
index 3211834..0a484d1 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkSemanticsOptions.java
@@ -704,6 +704,15 @@
help = "If set to true, unknown string escapes like `\\a` become rejected.")
public boolean incompatibleRestrictStringEscapes;
+ @Option(
+ name = "experimental_function_equality_by_location",
+ defaultValue = "false",
+ documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+ effectTags = OptionEffectTag.BUILD_FILE_SEMANTICS,
+ help =
+ "If set to true, two Starlark functions defined at the same place are considered equal.")
+ public boolean experimentalFunctionEqualityByLocation;
+
/**
* An interner to reduce the number of StarlarkSemantics instances. A single Blaze instance should
* never accumulate a large number of these and being able to shortcut on object identity makes a
@@ -774,6 +783,7 @@
.incompatibleAssignmentIdentifiersHaveLocalScope(
incompatibleAssignmentIdentifiersHaveLocalScope)
.incompatibleDisallowHashingFrozenMutables(incompatibleDisallowHashingFrozenMutables)
+ .experimentalFunctionEqualityByLocation(experimentalFunctionEqualityByLocation)
.build();
return INTERNER.intern(semantics);
}
diff --git a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java
index 8d85fc7..d5b5533 100644
--- a/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java
+++ b/src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java
@@ -86,8 +86,10 @@
positional = false,
defaultValue = "None")
},
- useLocation = true)
- public ProviderApi provider(String doc, Object fields, Location location) throws EvalException;
+ useLocation = true,
+ useEnvironment = true)
+ public ProviderApi provider(String doc, Object fields, Location location, Environment env)
+ throws EvalException;
@SkylarkCallable(
name = "rule",
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
index 0d3c49a..575458c 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/BaseFunction.java
@@ -26,6 +26,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
+import java.util.Objects;
import javax.annotation.Nullable;
/**
@@ -55,6 +56,7 @@
// (that FuncallExpression must supply), optimizing for the all-positional and all-keyword cases.
// Also, use better pure maps to minimize map O(n) re-creation events when processing keyword maps.
public abstract class BaseFunction implements StarlarkFunction {
+ private final boolean equalityByLocation;
/**
* The name of the function.
@@ -130,7 +132,7 @@
* {@link #getName}; otherwise it must be non-null.
*/
public BaseFunction(@Nullable String name) {
- this.name = name;
+ this(name, null, null, false);
}
/**
@@ -143,10 +145,12 @@
public BaseFunction(
@Nullable String name,
@Nullable FunctionSignature.WithValues<Object, SkylarkType> signature,
- @Nullable Location location) {
- this(name);
+ @Nullable Location location,
+ boolean equalityByLocation) {
+ this.name = name;
this.signature = signature;
this.location = location;
+ this.equalityByLocation = equalityByLocation;
}
/**
@@ -158,7 +162,7 @@
public BaseFunction(
@Nullable String name,
@Nullable FunctionSignature.WithValues<Object, SkylarkType> signature) {
- this(name, signature, null);
+ this(name, signature, null, false);
}
/**
@@ -168,7 +172,7 @@
* @param signature the signature, without default values or types
*/
public BaseFunction(@Nullable String name, FunctionSignature signature) {
- this(name, FunctionSignature.WithValues.create(signature), null);
+ this(name, FunctionSignature.WithValues.create(signature), null, false);
}
/**
@@ -178,8 +182,9 @@
* @param defaultValues a list of default values for the optional arguments to be configured.
*/
public BaseFunction(@Nullable String name, @Nullable Iterable<Object> defaultValues) {
- this(name);
+ this.name = name;
this.unconfiguredDefaultValues = defaultValues;
+ this.equalityByLocation = false;
}
/** Get parameter documentation as a list corresponding to each parameter */
@@ -574,6 +579,32 @@
return builder.toString();
}
+ @Override
+ public boolean equals(@Nullable Object other) {
+ if (equalityByLocation) {
+ if (other instanceof BaseFunction) {
+ BaseFunction that = (BaseFunction) other;
+ // In theory, the location alone unambiguously identifies a given function. However, in
+ // some test cases the location might not have a valid value, thus we also check the name.
+ return Objects.equals(this.getName(), that.getName())
+ && Objects.equals(this.location, that.location);
+ } else {
+ return false;
+ }
+ } else {
+ return super.equals(other);
+ }
+ }
+
+ @Override
+ public int hashCode() {
+ if (equalityByLocation) {
+ return Objects.hash(getName(), location);
+ } else {
+ return super.hashCode();
+ }
+ }
+
@Nullable
public Location getLocation() {
return location;
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Eval.java b/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
index 3199952..35d3350 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Eval.java
@@ -126,6 +126,7 @@
new UserDefinedFunction(
node.getIdentifier().getName(),
node.getIdentifier().getLocation(),
+ env.getSemantics().experimentalFunctionEqualityByLocation(),
FunctionSignature.WithValues.create(sig, defaultValues, /*types=*/ null),
node.getStatements(),
env.getGlobals()));
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
index 9860f6f..7454580 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java
@@ -222,6 +222,8 @@
public abstract boolean incompatibleDisallowHashingFrozenMutables();
+ public abstract boolean experimentalFunctionEqualityByLocation();
+
@Memoized
@Override
public abstract int hashCode();
@@ -304,6 +306,7 @@
.incompatibleAllowTagsPropagation(false)
.incompatibleAssignmentIdentifiersHaveLocalScope(false)
.incompatibleDisallowHashingFrozenMutables(false)
+ .experimentalFunctionEqualityByLocation(false)
.build();
/** Builder for {@link StarlarkSemantics}. All fields are mandatory. */
@@ -410,6 +413,8 @@
public abstract Builder incompatibleDisallowHashingFrozenMutables(boolean value);
+ public abstract Builder experimentalFunctionEqualityByLocation(boolean value);
+
public abstract StarlarkSemantics build();
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
index 460bc35..c39bdd2 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java
@@ -36,10 +36,11 @@
public UserDefinedFunction(
String name,
Location location,
+ boolean equalityByLocation,
FunctionSignature.WithValues<Object, SkylarkType> signature,
ImmutableList<Statement> statements,
Environment.GlobalFrame definitionGlobals) {
- super(name, signature, location);
+ super(name, signature, location, equalityByLocation);
this.statements = statements;
this.definitionGlobals = definitionGlobals;
}
diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkRuleFunctionsApi.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkRuleFunctionsApi.java
index 32a5b56..121d233 100644
--- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkRuleFunctionsApi.java
+++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkRuleFunctionsApi.java
@@ -84,7 +84,8 @@
}
@Override
- public ProviderApi provider(String doc, Object fields, Location location) throws EvalException {
+ public ProviderApi provider(String doc, Object fields, Location location, Environment env)
+ throws EvalException {
FakeProviderApi fakeProvider = new FakeProviderApi();
// Field documentation will be output preserving the order in which the fields are listed.
ImmutableList.Builder<ProviderFieldInfo> providerFieldInfos = ImmutableList.builder();
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkInfoTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkInfoTest.java
index 37e28b4..826b89c 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkInfoTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkInfoTest.java
@@ -81,7 +81,8 @@
@Test
public void givenLayoutTakesPrecedenceOverProviderLayout() throws Exception {
SkylarkProvider provider =
- SkylarkProvider.createUnexportedSchemaful(ImmutableList.of("f1", "f2"), Location.BUILTIN);
+ SkylarkProvider.createUnexportedSchemaful(
+ ImmutableList.of("f1", "f2"), Location.BUILTIN, /* equalityByLocation */ false);
SkylarkInfo info =
SkylarkInfo.createSchemaful(
provider, invertedLayoutF2F1, new Object[]{5, 4}, Location.BUILTIN);
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkProviderTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkProviderTest.java
index 0c81e9b..cde858d 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkProviderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkProviderTest.java
@@ -71,8 +71,9 @@
@Test
public void schemafulProvider_Instantiation() throws Exception {
- SkylarkProvider provider = SkylarkProvider.createUnexportedSchemaful(
- ImmutableList.of("a", "b", "c"), /*location=*/ null);
+ SkylarkProvider provider =
+ SkylarkProvider.createUnexportedSchemaful(
+ ImmutableList.of("a", "b", "c"), /*location=*/ null, false);
SkylarkInfo info = instantiateWithA1B2C3(provider);
assertThat(info.isCompact()).isTrue();
assertHasExactlyValuesA1B2C3(info);
@@ -86,8 +87,9 @@
@Test
public void schemafulProvider_GetFields() throws Exception {
- SkylarkProvider provider = SkylarkProvider.createUnexportedSchemaful(
- ImmutableList.of("a", "b", "c"), /*location=*/ null);
+ SkylarkProvider provider =
+ SkylarkProvider.createUnexportedSchemaful(
+ ImmutableList.of("a", "b", "c"), /*location=*/ null, false);
assertThat(provider.getFields()).containsExactly("a", "b", "c").inOrder();
}
diff --git a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
index ce01c46..a61b471 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/SkylarkSemanticsConsistencyTest.java
@@ -172,7 +172,8 @@
"--incompatible_disable_partition_default_parameter=" + rand.nextBoolean(),
"--incompatible_assignment_identifiers_have_local_scope=" + rand.nextBoolean(),
"--incompatible_disallow_hashing_frozen_mutables=" + rand.nextBoolean(),
- "--internal_skylark_flag_test_canary=" + rand.nextBoolean());
+ "--internal_skylark_flag_test_canary=" + rand.nextBoolean(),
+ "--experimental_function_equality_by_location=" + rand.nextBoolean());
}
/**
@@ -233,6 +234,7 @@
.incompatibleAssignmentIdentifiersHaveLocalScope(rand.nextBoolean())
.incompatibleDisallowHashingFrozenMutables(rand.nextBoolean())
.internalSkylarkFlagTestCanary(rand.nextBoolean())
+ .experimentalFunctionEqualityByLocation(rand.nextBoolean())
.build();
}
