[ddc] Add hot reload checks for instance methods

Handles errors and type changes on a hot reload when call sites
that were statically valid are retained and run after the reload.

Change-Id: I8bebbd7bc7acc97f55ff930e8f456f99146fbf21
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/440082
Reviewed-by: Mark Zhou <markzipan@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>

pkg/dev_compiler/lib/src/kernel/compiler_new.dart

@@ -6551,6 +6551,19 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     // Otherwise generate this as a normal typed method call.
     var jsName = _emitMemberName(name, member: target);
     var invocation = js.call('#.#(#)', [jsReceiver, jsName, jsArguments]);
+    if (_shouldRewriteInvocationWithHotReloadChecks(target)) {
+      var checkedInvocation = _rewriteInvocationWithHotReloadChecks(
+        jsReceiver,
+        jsName,
+        target,
+        node.arguments,
+        node.getStaticType(_staticTypeContext),
+        _nodeStart(node),
+      );
+      // As an optimization, avoid extra checks when the invocation code was
+      // compiled in the same generation that it is running.
+      return _emitHotReloadSafeInvocation(invocation, checkedInvocation);
+    }
     return _isNullCheckableJsInterop(node.interfaceTarget)
         ? _wrapWithJsInteropNullCheck(invocation)
         : invocation;
@@ -7636,6 +7649,8 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
       ..sourceInformation = _nodeStart(node);
     if (_shouldRewriteInvocationWithHotReloadChecks(target)) {
       var checkedCall = _rewriteInvocationWithHotReloadChecks(
+        fn.receiver,
+        fn.selector,
         target,
         node.arguments,
         node.getStaticType(_staticTypeContext),
@@ -7678,6 +7693,10 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
   /// time to include checks to preserve soundness in the presence of hot
   /// reloads at runtime.
   ///
+  /// The compiled JavaScript [receiver] and [selector] should be passed so that
+  /// they can be reused for the validated invocation after the checks have
+  /// passed.
+  ///
   /// The checks are similar to the those performed when making a dynamic call.
   /// The [arguments] are checked for the correct shape and runtime types.
   /// Additionally after the invocation, the returned value is checked against
@@ -7689,7 +7708,9 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
   /// The resulting expression for the validated call site will receive the
   /// [originalCallSiteSourceLocation].
   js_ast.Expression _rewriteInvocationWithHotReloadChecks(
-    Member target,
+    js_ast.Expression receiver,
+    js_ast.Expression selector,
+    Procedure target,
     Arguments arguments,
     DartType expectedReturnType,
     SourceLocation? originalCallSiteSourceLocation,
@@ -7738,8 +7759,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     // still valid.
     var checkResult = _emitScopedId('\$result');
     _letVariables!.add(checkResult);
-    var jsTarget = _emitStaticTarget(target);
-    var jsTypeArguments = [
+    var typeArguments = [
       // TODO(nshahan): Remove this check if we stop rewriting calls to SDK
       // functions.
       if (_reifyGenericFunction(target))
@@ -7748,9 +7768,9 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     var correctnessCheck = _runtimeCall(
       'hotReloadCorrectnessChecks(#, #, #, #, #)',
       [
-        jsTarget.receiver,
-        jsTarget.selector,
-        js_ast.ArrayInitializer(jsTypeArguments),
+        receiver,
+        selector,
+        js_ast.ArrayInitializer(typeArguments),
         js_ast.ArrayInitializer(hoistedPositionalVariables),
         hoistedNamedVariables.isEmpty
             ? js_ast.LiteralNull()
@@ -7766,14 +7786,18 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
         : js_ast.Binary(',', letAssignments, checkAssignment);
     // Create a new invocation of the original target but passing all the
     // arguments via their let variables.
-    var validatedCallSite = js_ast.Call(jsTarget, [
-      ...jsTypeArguments,
-      ...hoistedPositionalVariables,
-      if (hoistedNamedVariables.isNotEmpty)
-        js_ast.ObjectInitializer([
-          for (var e in hoistedNamedVariables.entries)
-            js_ast.Property(js.string(e.key), e.value),
-        ]),
+    var validatedCallSite = js.call('#.#(#)', [
+      receiver,
+      selector,
+      [
+        ...typeArguments,
+        ...hoistedPositionalVariables,
+        if (hoistedNamedVariables.isNotEmpty)
+          js_ast.ObjectInitializer([
+            for (var e in hoistedNamedVariables.entries)
+              js_ast.Property(js.string(e.key), e.value),
+          ]),
+      ],
     ])..sourceInformation = originalCallSiteSourceLocation;
     // Cast the result of the checked call or the value returned from a
     // `NoSuchMethod` invocation.

sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/operations.dart

@@ -744,13 +744,15 @@ Object? hotReloadCorrectnessChecks(
       ),
     );
   }
-  var functionType = JS<Object?>(
-    '',
-    '#[#][#]',
-    receiver,
-    name,
-    JS_GET_NAME(JsGetName.SIGNATURE_NAME),
-  );
+  var functionType = _jsInstanceOf(receiver, Object)
+      ? getMethodType(receiver, name)
+      : JS<Object?>(
+          '',
+          '#[#][#]',
+          receiver,
+          name,
+          JS_GET_NAME(JsGetName.SIGNATURE_NAME),
+        );
   if (functionType == null) {
     // Allow JavaScript interop calls without checking arguments.
     // TODO(nshahan): Potentially we should be checking arguments to static
@@ -1546,7 +1548,7 @@ bool isStateBearingSymbol(property) => JS<bool>(
 /// copies the members of [classDeclaration] and its prototype's properties to
 /// the existing class. Existing members not prefixed by a special identifier
 /// are replaced (see [isStateBearingSymbol]).
-declareClass(library, classIdentifier, classDeclaration) {
+declareClass(Object library, Object classIdentifier, Object classDeclaration) {
   var originalClass = JS<Object>('!', '#.#', library, classIdentifier);
   if (JS<bool>('!', '# === void 0', originalClass)) {
     JS('', '#.# = #', library, classIdentifier, classDeclaration);
@@ -1559,6 +1561,8 @@ declareClass(library, classIdentifier, classDeclaration) {
       !isStateBearingSymbol(property),
       originalClassProto,
     );
+    // Reconcile instance members.
+    deleteClassMembers(originalClassProto, newClassProto);
     copyProperties(originalClassProto, newClassProto, copyWhen: copyWhenProto);
     var copyWhen = (property) => JS<bool>(
       '!',
@@ -1566,6 +1570,7 @@ declareClass(library, classIdentifier, classDeclaration) {
       !isStateBearingSymbol(property),
       originalClass,
     );
+    // Reconcile static members.
     deleteClassMembers(originalClass, classDeclaration);
     copyProperties(originalClass, classDeclaration, copyWhen: copyWhen);
   }
@@ -1579,10 +1584,12 @@ declareClass(library, classIdentifier, classDeclaration) {
 ///
 /// Called from generated code.
 void deleteClassMembers(Object oldClass, Object newClass) {
-  for (var name in getOwnNamesAndSymbols(oldClass)) {
-    if (JS<Object?>('', '#.#', newClass, name) == null &&
-        !isStateBearingSymbol(name)) {
-      JS('', 'delete #.#', oldClass, name);
+  var oldClassNamesAndSymbols = getOwnNamesAndSymbols(oldClass);
+  var newClassNamesAndSymbols = getOwnNamesAndSymbols(newClass);
+  for (var property in oldClassNamesAndSymbols) {
+    if (JS<bool>('', '!#.includes(#)', newClassNamesAndSymbols, property) &&
+        !isStateBearingSymbol(property)) {
+      JS('', 'delete #.#', oldClass, property);
     }
   }
 }

tests/hot_reload/call_deleted_instance_method_ddc/config.json

@@ -0,0 +1,5 @@
+{
+  "exclude": [
+    "vm"
+  ]
+}

tests/hot_reload/call_deleted_instance_method_ddc/main.0.dart

@@ -0,0 +1,30 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'package:reload_test/reload_test_utils.dart';
+
+var retained;
+C? c;
+
+class C {
+  String deleted() {
+    return 'hello';
+  }
+}
+
+helper() {
+  c = C();
+  retained = () => c!.deleted();
+  return retained();
+}
+
+Future<void> main() async {
+  helper();
+  await hotReload();
+  Expect.throws<NoSuchMethodError>(
+    helper,
+    (error) => '$error'.contains('deleted'),
+  );
+}

tests/hot_reload/call_deleted_instance_method_ddc/main.1.dart

@@ -0,0 +1,44 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'package:reload_test/reload_test_utils.dart';
+
+var retained;
+C? c;
+
+class C {}
+
+helper() {
+  return retained();
+}
+
+Future<void> main() async {
+  helper();
+  await hotReload();
+  Expect.throws<NoSuchMethodError>(
+    helper,
+    (error) => '$error'.contains('deleted'),
+  );
+}
+
+/** DIFF **/
+/*
+ var retained;
+ C? c;
+ 
+-class C {
+-  String deleted() {
+-    return 'hello';
+-  }
+-}
++class C {}
+ 
+ helper() {
+-  c = C();
+-  retained = () => c!.deleted();
+   return retained();
+ }
+ 
+*/

tests/hot_reload/call_deleted_instance_method_vm/config.json

@@ -0,0 +1,6 @@
+{
+  "exclude": [
+    "chrome",
+    "d8"
+  ]
+}

tests/hot_reload/call_deleted_instance_method_vm/main.0.dart

@@ -0,0 +1,30 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'package:reload_test/reload_test_utils.dart';
+
+var retained;
+C? c;
+
+class C {
+  String deleted() {
+    return 'hello';
+  }
+}
+
+helper() {
+  c = C();
+  retained = () => c!.deleted();
+  return retained();
+}
+
+Future<void> main() async {
+  helper();
+  await hotReload();
+  Expect.throws(
+    helper,
+    (error) => '$error'.contains('Lookup failed: deleted in @methods in C'),
+  );
+}

tests/hot_reload/call_deleted_instance_method_vm/main.1.dart

@@ -0,0 +1,44 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'package:reload_test/reload_test_utils.dart';
+
+var retained;
+C? c;
+
+class C {}
+
+helper() {
+  return retained();
+}
+
+Future<void> main() async {
+  helper();
+  await hotReload();
+  Expect.throws(
+    helper,
+    (error) => '$error'.contains('Lookup failed: deleted in @methods in C'),
+  );
+}
+
+/** DIFF **/
+/*
+ var retained;
+ C? c;
+ 
+-class C {
+-  String deleted() {
+-    return 'hello';
+-  }
+-}
++class C {}
+ 
+ helper() {
+-  c = C();
+-  retained = () => c!.deleted();
+   return retained();
+ }
+ 
+*/

tests/hot_reload/call_instance_method_parameters_change_ddc/config.json

@@ -0,0 +1,5 @@
+{
+  "exclude": [
+    "vm"
+  ]
+}

tests/hot_reload/call_instance_method_parameters_change_ddc/main.0.dart

@@ -0,0 +1,30 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'package:reload_test/reload_test_utils.dart';
+
+var retained;
+C? c;
+
+class C {
+  int parametersChange(int i) {
+    return i + 10;
+  }
+}
+
+helper() {
+  c = C();
+  retained = () => c!.parametersChange(32);
+  return retained!();
+}
+
+Future<void> main() async {
+  Expect.equals(42, helper());
+  await hotReload();
+  Expect.throws<TypeError>(
+    helper,
+    (error) => '$error'.contains("'int' is not a subtype of type 'String'"),
+  );
+}

tests/hot_reload/call_instance_method_parameters_change_ddc/main.1.dart

@@ -0,0 +1,48 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'package:reload_test/reload_test_utils.dart';
+
+var retained;
+C? c;
+
+class C {
+  int parametersChange(String s) {
+    return s.length;
+  }
+}
+
+helper() {
+  return retained!();
+}
+
+Future<void> main() async {
+  Expect.equals(42, helper());
+  await hotReload();
+  Expect.throws<TypeError>(
+    helper,
+    (error) => '$error'.contains("'int' is not a subtype of type 'String'"),
+  );
+}
+
+/** DIFF **/
+/*
+ C? c;
+ 
+ class C {
+-  int parametersChange(int i) {
+-    return i + 10;
++  int parametersChange(String s) {
++    return s.length;
+   }
+ }
+ 
+ helper() {
+-  c = C();
+-  retained = () => c!.parametersChange(32);
+   return retained!();
+ }
+ 
+*/

tests/hot_reload/call_instance_method_parameters_change_vm/config.json

@@ -0,0 +1,6 @@
+{
+  "exclude": [
+    "chrome",
+    "d8"
+  ]
+}

tests/hot_reload/call_instance_method_parameters_change_vm/main.0.dart

@@ -0,0 +1,31 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'package:reload_test/reload_test_utils.dart';
+
+var retained;
+C? c;
+
+class C {
+  int parametersChange(int i) {
+    return i + 10;
+  }
+}
+
+helper() {
+  c = C();
+  retained = () => c!.parametersChange(32);
+  return retained!();
+}
+
+Future<void> main() async {
+  Expect.equals(42, helper());
+  await hotReload();
+  Expect.throws<NoSuchMethodError>(
+    helper,
+    (error) =>
+        '$error'.contains("Class 'int' has no instance getter 'length'."),
+  );
+}

tests/hot_reload/call_instance_method_parameters_change_vm/main.1.dart

@@ -0,0 +1,49 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'package:reload_test/reload_test_utils.dart';
+
+var retained;
+C? c;
+
+class C {
+  int parametersChange(String s) {
+    return s.length;
+  }
+}
+
+helper() {
+  return retained!();
+}
+
+Future<void> main() async {
+  Expect.equals(42, helper());
+  await hotReload();
+  Expect.throws<NoSuchMethodError>(
+    helper,
+    (error) =>
+        '$error'.contains("Class 'int' has no instance getter 'length'."),
+  );
+}
+
+/** DIFF **/
+/*
+ C? c;
+ 
+ class C {
+-  int parametersChange(int i) {
+-    return i + 10;
++  int parametersChange(String s) {
++    return s.length;
+   }
+ }
+ 
+ helper() {
+-  c = C();
+-  retained = () => c!.parametersChange(32);
+   return retained!();
+ }
+ 
+*/

tests/hot_reload/call_instance_method_return_change_ddc/config.json

@@ -0,0 +1,5 @@
+{
+  "exclude": [
+    "vm"
+  ]
+}

tests/hot_reload/call_instance_method_return_change_ddc/main.0.dart

@@ -0,0 +1,30 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'package:reload_test/reload_test_utils.dart';
+
+int Function()? retained;
+C? c;
+
+class C {
+  String returnChange() {
+    return 'hello';
+  }
+}
+
+helper() {
+  c = C();
+  retained = () => c!.returnChange().length;
+  return retained!();
+}
+
+Future<void> main() async {
+  helper();
+  await hotReload();
+  Expect.throws<TypeError>(
+    helper,
+    (error) => '$error'.contains("'double' is not a subtype of type 'String'"),
+  );
+}

tests/hot_reload/call_instance_method_return_change_ddc/main.1.dart

@@ -0,0 +1,48 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'package:reload_test/reload_test_utils.dart';
+
+int Function()? retained;
+C? c;
+
+class C {
+  double returnChange() {
+    return 3.14;
+  }
+}
+
+helper() {
+  return retained!();
+}
+
+Future<void> main() async {
+  helper();
+  await hotReload();
+  Expect.throws<TypeError>(
+    helper,
+    (error) => '$error'.contains("'double' is not a subtype of type 'String'"),
+  );
+}
+
+/** DIFF **/
+/*
+ C? c;
+ 
+ class C {
+-  String returnChange() {
+-    return 'hello';
++  double returnChange() {
++    return 3.14;
+   }
+ }
+ 
+ helper() {
+-  c = C();
+-  retained = () => c!.returnChange().length;
+   return retained!();
+ }
+ 
+*/

tests/hot_reload/call_instance_method_return_change_vm/config.json

@@ -0,0 +1,6 @@
+{
+  "exclude": [
+    "chrome",
+    "d8"
+  ]
+}

tests/hot_reload/call_instance_method_return_change_vm/main.0.dart

@@ -0,0 +1,30 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'package:reload_test/reload_test_utils.dart';
+
+int Function()? retained;
+C? c;
+
+class C {
+  String returnChange() {
+    return 'hello';
+  }
+}
+
+helper() {
+  c = C();
+  retained = () => c!.returnChange().length;
+  return retained!();
+}
+
+Future<void> main() async {
+  helper();
+  await hotReload();
+  Expect.throws<NoSuchMethodError>(
+    helper,
+    (error) => '$error'.contains("'length'"),
+  );
+}

tests/hot_reload/call_instance_method_return_change_vm/main.1.dart

@@ -0,0 +1,48 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'package:reload_test/reload_test_utils.dart';
+
+int Function()? retained;
+C? c;
+
+class C {
+  double returnChange() {
+    return 3.14;
+  }
+}
+
+helper() {
+  return retained!();
+}
+
+Future<void> main() async {
+  helper();
+  await hotReload();
+  Expect.throws<NoSuchMethodError>(
+    helper,
+    (error) => '$error'.contains("'length'"),
+  );
+}
+
+/** DIFF **/
+/*
+ C? c;
+ 
+ class C {
+-  String returnChange() {
+-    return 'hello';
++  double returnChange() {
++    return 3.14;
+   }
+ }
+ 
+ helper() {
+-  c = C();
+-  retained = () => c!.returnChange().length;
+   return retained!();
+ }
+ 
+*/

tests/hot_reload/change_instance_format5_ddc/config.json

@@ -0,0 +1,5 @@
+{
+  "exclude": [
+    "vm"
+  ]
+}

tests/hot_reload/change_instance_format5_ddc/main.0.dart

@@ -0,0 +1,33 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'package:reload_test/reload_test_utils.dart';
+
+// Adapted from:
+// https://github.com/dart-lang/sdk/blob/1a486499bf73ee5b007abbe522b94869a1f36d02/runtime/vm/isolate_reload_test.cc#L3985
+
+// Tests reload succeeds when instance format changes.
+// Change: Bar {a, b}, Foo : Bar {c:42} -> Bar {c:42}, Foo : Bar {}
+// Validate: c keeps the value in the retained Foo object.
+
+class Bar {
+  var a;
+  var b;
+}
+
+class Foo extends Bar {
+  var c;
+}
+
+var f;
+
+Future<void> main() async {
+  f = Foo();
+  f.c = 42;
+  Expect.equals(42, f.c);
+  await hotReload();
+
+  Expect.equals(null, f.c);
+}

tests/hot_reload/change_instance_format5_ddc/main.1.dart

@@ -0,0 +1,50 @@
+// Copyright (c) 2025, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'package:expect/expect.dart';
+import 'package:reload_test/reload_test_utils.dart';
+
+// Adapted from:
+// https://github.com/dart-lang/sdk/blob/1a486499bf73ee5b007abbe522b94869a1f36d02/runtime/vm/isolate_reload_test.cc#L3985
+
+// Tests reload succeeds when instance format changes.
+// Change: Bar {a, b}, Foo : Bar {c:42} -> Bar {c:42}, Foo : Bar {}
+// Validate: c keeps the value in the retained Foo object.
+
+class Bar {
+  var c;
+}
+
+class Foo extends Bar {}
+
+var f;
+
+Future<void> main() async {
+  f = Foo();
+  f.c = 42;
+  Expect.equals(42, f.c);
+  await hotReload();
+
+  Expect.equals(null, f.c);
+}
+
+/** DIFF **/
+/*
+ // Validate: c keeps the value in the retained Foo object.
+ 
+ class Bar {
+-  var a;
+-  var b;
+-}
+-
+-class Foo extends Bar {
+   var c;
+ }
+ 
++class Foo extends Bar {}
++
+ var f;
+ 
+ Future<void> main() async {
+*/

tests/hot_reload/change_instance_format5_vm/config.json

@@ -0,0 +1,6 @@
+{
+  "exclude": [
+    "d8",
+    "chrome"
+  ]
+}