commit de57c42878fe926a4e98ffa77b8590d1dfd3895a
author janakr <janakr@google.com> Tue Mar 31 08:04:11 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Tue Mar 31 08:05:27 2020 -0700
tree 1b2d88e14cf9af60f22fe2e958e7f5d8221fb380
parent 732e7108ee44f9e91fc0c1d5ced2fddf82df7c50

Compute hash code correctly for StarlarkList. Elements beyond the size limit do not exist, so it is incorrect to take them into account when computing the hash code.

Note that equality was correct, so this could have led to lookups for existing lists failing, although I'm not sure StarlarkLists are used as keys anywhere.

PiperOrigin-RevId: 303966299

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

diff --git a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkList.java b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkList.java
index d722ced..d4c5854 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/StarlarkList.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/StarlarkList.java
@@ -182,7 +182,12 @@
 
   @Override
   public int hashCode() {
-    return 6047 + 4673 * Arrays.hashCode(elems);
+    // Roll our own hash code to avoid iterating through null part of elems.
+    int result = 1;
+    for (int i = 0; i < size; i++) {
+      result = 31 * result + elems[i].hashCode();
+    }
+    return 6047 + 4673 * result;
   }
 
   @Override

src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java

diff --git a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
index d4eaaeb..cd602f1 100644
--- a/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
+++ b/src/test/java/com/google/devtools/build/lib/syntax/SkylarkListTest.java
@@ -209,6 +209,14 @@
   }
 
   @Test
+  public void listAfterRemoveHasExpectedEqualsAndHashCode() throws Exception {
+    exec("l = [1, 2, 3]");
+    exec("l.remove(3)");
+    assertThat(lookup("l")).isEqualTo(eval("[1, 2]"));
+    assertThat(lookup("l").hashCode()).isEqualTo(eval("[1, 2]").hashCode());
+  }
+
+  @Test
   public void testConcatListToString() throws Exception {
     assertThat(eval("str([1, 2] + [3, 4])")).isEqualTo("[1, 2, 3, 4]");
   }