Implement structural equality for structs
RELNOTES: Structs in Skylark are tested for structural equality instead of reference equality.
--
MOS_MIGRATED_REVID=139583726
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 830021d..65fff09 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.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 @@
@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 @@
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 e478833..8291d1f 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 @@
@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}.
diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
index 3f2118d..7bc8bcb 100644
--- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleClassFunctionsTest.java
@@ -792,6 +792,26 @@
}
@Test
+ public void testStructEquality() throws Exception {
+ assertTrue((Boolean) eval("struct(a = 1, b = 2) == struct(b = 2, a = 1)"));
+ assertFalse((Boolean) eval("struct(a = 1) == struct(a = 1, b = 2)"));
+ assertFalse((Boolean) eval("struct(a = 1, b = 2) == struct(a = 1)"));
+ // Compare a recursive object to itself to make sure reference equality is checked
+ assertTrue((Boolean) eval("s = (struct(a = 1, b = [])); s.b.append(s); s == s"));
+ assertFalse((Boolean) eval("struct(a = 1, b = 2) == struct(a = 1, b = 3)"));
+ assertFalse((Boolean) eval("struct(a = 1) == [1]"));
+ assertFalse((Boolean) eval("[1] == struct(a = 1)"));
+ assertTrue((Boolean) eval("struct() == struct()"));
+ assertFalse((Boolean) eval("struct() == struct(a = 1)"));
+
+ eval("foo = provider(); bar = provider()");
+ assertFalse((Boolean) eval("struct(a = 1) == foo(a = 1)"));
+ assertFalse((Boolean) eval("foo(a = 1) == struct(a = 1)"));
+ assertFalse((Boolean) eval("foo(a = 1) == bar(a = 1)"));
+ assertTrue((Boolean) eval("foo(a = 1) == foo(a = 1)"));
+ }
+
+ @Test
public void testStructAccessingFieldsFromSkylark() throws Exception {
eval("x = struct(a = 1, b = 2)", "x1 = x.a", "x2 = x.b");
assertThat(lookup("x1")).isEqualTo(1);
