Add a flag that makes two Starlark functions equal if they are define…

…d at the same location.

This flag, when set, mostly reverts af7fc3b (only mostly because it only applies to user-defined functions)

BUG=138789815
RELNOTES: None.
PiperOrigin-RevId: 261299731

src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleClassFunctions.java

@@ -243,7 +243,8 @@ public Object getDefault(AttributeMap rule) {
       };
 
   @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 @@ public Provider provider(String doc, Object fields, Location location) throws Ev
           "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

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

@@ -78,7 +78,7 @@ protected NativeProvider(
       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;

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

@@ -42,8 +42,9 @@ abstract class ProviderFromFunction extends BaseFunction implements Provider {
   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() {

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

@@ -75,8 +75,10 @@ public class SkylarkProvider extends ProviderFromFunction implements SkylarkExpo
    * @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 @@ public static SkylarkProvider createUnexportedSchemaless(Location location) {
    *     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 @@ public static SkylarkProvider createUnexportedSchemaful(
    * @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 @@ public static SkylarkProvider createExportedSchemaless(SkylarkKey key, Location
    * @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 @@ public static SkylarkProvider createExportedSchemaful(
    * 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 =

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

@@ -704,6 +704,15 @@ public class StarlarkSemanticsOptions extends OptionsBase implements Serializabl
       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 @@ public StarlarkSemantics toSkylarkSemantics() {
             .incompatibleAssignmentIdentifiersHaveLocalScope(
                 incompatibleAssignmentIdentifiersHaveLocalScope)
             .incompatibleDisallowHashingFrozenMutables(incompatibleDisallowHashingFrozenMutables)
+            .experimentalFunctionEqualityByLocation(experimentalFunctionEqualityByLocation)
             .build();
     return INTERNER.intern(semantics);
   }

src/main/java/com/google/devtools/build/lib/skylarkbuildapi/SkylarkRuleFunctionsApi.java

@@ -86,8 +86,10 @@ public interface SkylarkRuleFunctionsApi<FileApiT extends FileApi> {
             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",

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 @@ public boolean isConfigured() {
    * {@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) {
   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(
   public BaseFunction(
       @Nullable String name,
       @Nullable FunctionSignature.WithValues<Object, SkylarkType> signature) {
-    this(name, signature, null);
+    this(name, signature, null, false);
   }
 
   /**
@@ -168,7 +172,7 @@ public BaseFunction(
    * @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 @@ public BaseFunction(@Nullable String name, FunctionSignature signature) {
    * @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 @@ protected String printTypeString(Object[] args, int howManyArgsToPrint) {
     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;

src/main/java/com/google/devtools/build/lib/syntax/Eval.java

@@ -126,6 +126,7 @@ void execDef(FunctionDefStatement node) throws EvalException, InterruptedExcepti
         new UserDefinedFunction(
             node.getIdentifier().getName(),
             node.getIdentifier().getLocation(),
+            env.getSemantics().experimentalFunctionEqualityByLocation(),
             FunctionSignature.WithValues.create(sig, defaultValues, /*types=*/ null),
             node.getStatements(),
             env.getGlobals()));

src/main/java/com/google/devtools/build/lib/syntax/StarlarkSemantics.java

@@ -222,6 +222,8 @@ public boolean flagValue(FlagIdentifier flagIdentifier) {
 
   public abstract boolean incompatibleDisallowHashingFrozenMutables();
 
+  public abstract boolean experimentalFunctionEqualityByLocation();
+
   @Memoized
   @Override
   public abstract int hashCode();
@@ -304,6 +306,7 @@ public static Builder builderWithDefaults() {
           .incompatibleAllowTagsPropagation(false)
           .incompatibleAssignmentIdentifiersHaveLocalScope(false)
           .incompatibleDisallowHashingFrozenMutables(false)
+          .experimentalFunctionEqualityByLocation(false)
           .build();
 
   /** Builder for {@link StarlarkSemantics}. All fields are mandatory. */
@@ -410,6 +413,8 @@ public abstract Builder incompatibleDisallowRuleExecutionPlatformConstraintsAllo
 
     public abstract Builder incompatibleDisallowHashingFrozenMutables(boolean value);
 
+    public abstract Builder experimentalFunctionEqualityByLocation(boolean value);
+
     public abstract StarlarkSemantics build();
   }
 }

src/main/java/com/google/devtools/build/lib/syntax/UserDefinedFunction.java

@@ -36,10 +36,11 @@ public class UserDefinedFunction extends BaseFunction {
   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;
   }

src/main/java/com/google/devtools/build/skydoc/fakebuildapi/FakeSkylarkRuleFunctionsApi.java

@@ -84,7 +84,8 @@ public FakeSkylarkRuleFunctionsApi(
   }
 
   @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();

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

@@ -81,7 +81,8 @@ public void nullLocationDefaultsToBuiltin() throws Exception {
   @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);

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

@@ -71,8 +71,9 @@ public void schemalessProvider_Instantiation() throws Exception {
 
   @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 @@ public void schemalessProvider_GetFields() throws Exception {
 
   @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();
   }
 

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

@@ -172,7 +172,8 @@ private static StarlarkSemanticsOptions buildRandomOptions(Random rand) throws E
         "--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 @@ private static StarlarkSemantics buildRandomSemantics(Random rand) {
         .incompatibleAssignmentIdentifiersHaveLocalScope(rand.nextBoolean())
         .incompatibleDisallowHashingFrozenMutables(rand.nextBoolean())
         .internalSkylarkFlagTestCanary(rand.nextBoolean())
+        .experimentalFunctionEqualityByLocation(rand.nextBoolean())
         .build();
   }