From c7f99144977aa10767eed8564808d402ce4540d1 Mon Sep 17 00:00:00 2001
From: Ryan Macnak <rmacnak@google.com>
Date: Tue, 1 Oct 2024 19:35:51 +0000
Subject: [PATCH] [vm, reload] Use a more stable hash code for implicit closure
 functions.

The old hash code used the absolute token position, which made the hash code of a closure change for any insertions or deletions above it in the same file.

TEST=vm/cc/IsolateReload_ClosureHashStablity
Bug: https://github.com/flutter/flutter/issues/153536
Change-Id: I75da3f0cdca1862637179467ec23cf20b9878d8d
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/387602
Reviewed-by: Alexander Markov <alexmarkov@google.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
---
 runtime/vm/isolate_reload_test.cc | 68 +++++++++++++++++++++++++++++++
 runtime/vm/object.cc              |  8 ++--
 2 files changed, 72 insertions(+), 4 deletions(-)

diff --git a/runtime/vm/isolate_reload_test.cc b/runtime/vm/isolate_reload_test.cc
index 4f300d641d6..d30886b35d0 100644
--- a/runtime/vm/isolate_reload_test.cc
+++ b/runtime/vm/isolate_reload_test.cc
@@ -6815,6 +6815,74 @@ abstract class A5 { }
   check_class_hierarchy_state();
 }
 
+// https://github.com/flutter/flutter/issues/153536
+TEST_CASE(IsolateReload_ClosureHashStablity) {
+  const char* kScript =
+      "var retained;\n"
+      "var retainedHashes;\n"
+      "static1() {} \n"
+      "static2<T>() {} \n"
+      "static3<T extends num>() {} \n"
+      "class Foo {\n"
+      "  method1() {}\n"
+      "  method2<T>() {}\n"
+      "  method3<T extends num>() {}\n"
+      "}\n"
+      "extension Ext on Foo {\n"
+      "  extensionMethod1() {}\n"
+      "  extensionMethod2<T>() {}\n"
+      "  extensionMethod3<T extends num>() {}\n"
+      "}\n"
+      "main() {\n"
+      "  local1() {}\n"
+      "  local2<T>() {}\n"
+      "  local3<T extends num>() {}\n"
+      "  var f = new Foo();\n"
+      "  retained = [ static1, static2<String>, static3,\n"
+      "               f.method1, f.method2<String>, f.method3,\n"
+      "               f.extensionMethod1, f.extensionMethod2<String>,\n"
+      "               f.extensionMethod3,\n"
+      "               local1, local2<String>, local3 ];\n"
+      "  retainedHashes = retained.map((c) => c.hashCode).toList();\n"
+      "  return 'Setup';\n"
+      "}\n";
+
+  Dart_Handle lib = TestCase::LoadTestScript(kScript, nullptr);
+  EXPECT_VALID(lib);
+  EXPECT_VALID(Dart_FinalizeAllClasses());
+  EXPECT_STREQ("Setup", SimpleInvokeStr(lib, "main"));
+
+  const char* kReloadScript =
+      "extraFunctionShiftingDownAllTokenPositions() {}\n"
+      "var retained;\n"
+      "var retainedHashes;\n"
+      "static1() {} \n"
+      "static2<T>() {} \n"
+      "static3<T extends num>() {} \n"
+      "class Foo {\n"
+      "  method1() {}\n"
+      "  method2<T>() {}\n"
+      "  method3<T extends num>() {}\n"
+      "}\n"
+      "extension Ext on Foo {\n"
+      "  extensionMethod1() {}\n"
+      "  extensionMethod2<T>() {}\n"
+      "  extensionMethod3<T extends num>() {}\n"
+      "}\n"
+      "main() {\n"
+      "  for (var i = 0; i < retained.length; i++) {\n"
+      "    if (retained[i].hashCode != retainedHashes[i]){\n"
+      "      return 'Changed: ${retained[i]}';\n"
+      "    }\n"
+      "  }\n"
+      "  return 'Okay';\n"
+      "}\n";
+
+  lib = TestCase::ReloadTestScript(kReloadScript);
+  EXPECT_VALID(lib);
+  EXPECT_STREQ("Okay", SimpleInvokeStr(lib, "main"));
+}
+
 #endif  // !defined(PRODUCT) && !defined(DART_PRECOMPILED_RUNTIME)
 
 }  // namespace dart
diff --git a/runtime/vm/object.cc b/runtime/vm/object.cc
index bfd7450343d..5ff31ebc662 100644
--- a/runtime/vm/object.cc
+++ b/runtime/vm/object.cc
@@ -8067,12 +8067,12 @@ void PatchClass::set_script(const Script& value) const {
 }
 
 uword Function::Hash() const {
-  uword hash = String::HashRawSymbol(name());
-  if (IsClosureFunction()) {
-    hash = hash ^ token_pos().Hash();
+  uint32_t hash = String::HashRawSymbol(name());
+  if (IsNonImplicitClosureFunction()) {
+    hash = CombineHashes(hash, token_pos().Hash());
   }
   if (Owner()->IsClass()) {
-    hash = hash ^ Class::Hash(Class::RawCast(Owner()));
+    hash = CombineHashes(hash, Class::Hash(Class::RawCast(Owner())));
   }
   return hash;
 }