Implement structural equality for structs

RELNOTES: Structs in Skylark are tested for structural equality instead of reference equality.

--
MOS_MIGRATED_REVID=139583726

2 files changed, 64 insertions, 2 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObject.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObject.java
index 830021d170..65fff09f52 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObject.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObject.java
@@ -14,6 +14,7 @@
 package com.google.devtools.build.lib.packages;
 
 import com.google.common.base.Joiner;
+import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Ordering;
@@ -31,6 +32,9 @@ import com.google.devtools.build.lib.syntax.Printer;
 import com.google.devtools.build.lib.syntax.SkylarkType;
 import com.google.devtools.build.lib.util.Preconditions;
 import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
 import java.util.Map;
 import javax.annotation.Nullable;
 
@@ -175,6 +179,10 @@ public class SkylarkClassObject implements ClassObject, SkylarkValue, Concatable
 
   @Override
   public boolean isImmutable() {
+    // If the constructor is not yet exported the hash code of the object is subject to change
+    if (!constructor.isExported()) {
+      return false;
+    }
     for (Object item : values.values()) {
       if (!EvalUtils.isImmutable(item)) {
         return false;
@@ -183,6 +191,43 @@ public class SkylarkClassObject implements ClassObject, SkylarkValue, Concatable
     return true;
   }
 
+  @Override
+  public boolean equals(Object otherObject) {
+    if (!(otherObject instanceof SkylarkClassObject)) {
+      return false;
+    }
+    SkylarkClassObject other = (SkylarkClassObject) otherObject;
+    if (this == other) {
+      return true;
+    }
+    if (!this.constructor.equals(other.constructor)) {
+      return false;
+    }
+    // Compare objects' keys and values
+    if (!this.getKeys().equals(other.getKeys())) {
+      return false;
+    }
+    for (String key : getKeys()) {
+      if (!this.getValue(key).equals(other.getValue(key))) {
+        return false;
+      }
+    }
+    return true;
+  }
+
+  @Override
+  public int hashCode() {
+    List<String> keys = new ArrayList<>(getKeys());
+    Collections.sort(keys);
+    List<Object> objectsToHash = new ArrayList<>();
+    objectsToHash.add(constructor);
+    for (String key : keys) {
+      objectsToHash.add(key);
+      objectsToHash.add(getValue(key));
+    }
+    return Objects.hashCode(objectsToHash.toArray());
+  }
+
   /**
    * Convert the object to string using Skylark syntax. The output tries to be
    * reversible (but there is no guarantee, it depends on the actual values).
diff --git a/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObjectConstructor.java b/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObjectConstructor.java
index e4788334c3..8291d1f878 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObjectConstructor.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/SkylarkClassObjectConstructor.java
@@ -135,14 +135,31 @@ public final class SkylarkClassObjectConstructor extends BaseFunction implements
 
   @Override
   public int hashCode() {
+    if (isExported()) {
+      return getKey().hashCode();
+    }
     return System.identityHashCode(this);
   }
 
   @Override
-  public boolean equals(@Nullable Object other) {
-    return other == this;
+  public boolean equals(@Nullable Object otherObject) {
+    if (!(otherObject instanceof  SkylarkClassObjectConstructor)) {
+      return false;
+    }
+    SkylarkClassObjectConstructor other = (SkylarkClassObjectConstructor) otherObject;
+
+    if (this.isExported() && other.isExported()) {
+      return this.getKey().equals(other.getKey());
+    } else {
+      return this == other;
+    }
   }
 
+  @Override
+  public boolean isImmutable() {
+    // Hash code for non exported constructors may be changed
+    return isExported();
+  }
 
   /**
    * A representation of {@link SkylarkClassObjectConstructor}.