[ddc] Add validation to instance setter calls

Ensures soundness for instance setter calls after a hot reload.

Issue: #60100
Issue: #60112
Change-Id: Ie6736d002752076e5526bd37ae9f103040ed8cdd
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/437124
Reviewed-by: Mark Zhou <[email protected]>
Commit-Queue: Nicholas Shahan <[email protected]>

Author: nshahan

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

@@ -6171,6 +6171,8 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
     );
     var instanceGet = js_ast.PropertyAccess(jsReceiver, jsMemberName);
     if (_shouldRewriteInvocationWithHotReloadChecks(node.interfaceTarget)) {
+      // TODO(nshahan): Add a runtime helper to perform the checks only so the
+      // call site can be monomorphic.
       instanceGet.sourceInformation = _nodeStart(node);
       // Since there are no arguments (unlike methods) the dynamic get path can
       // be reused for the hot reload checks on a getter.
@@ -6245,23 +6247,43 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
       member.name.text == 'call' && isNonStaticJsInterop(member);
 
   @override
-  js_ast.Expression visitDynamicSet(DynamicSet node) {
-    return _runtimeCall('dput$_replSuffix(#, #, #)', [
-      _visitExpression(node.receiver),
-      _emitMemberName(node.name.text),
-      _visitExpression(node.value),
-    ]);
-  }
+  js_ast.Expression visitDynamicSet(DynamicSet node) => _emitDynamicSet(
+    _visitExpression(node.receiver),
+    _emitMemberName(node.name.text),
+    _visitExpression(node.value),
+  );
+
+  js_ast.Expression _emitDynamicSet(
+    js_ast.Expression receiver,
+    js_ast.Expression memberName,
+    js_ast.Expression value,
+  ) => _runtimeCall('dput$_replSuffix(#, #, #)', [receiver, memberName, value]);
 
   @override
   js_ast.Expression visitInstanceSet(InstanceSet node) {
     var target = node.interfaceTarget;
-    var value = isJsMember(target) ? _assertInterop(node.value) : node.value;
-    return js.call('#.# = #', [
-      _visitExpression(node.receiver),
-      _emitMemberName(node.name.text, member: target),
-      _visitExpression(value),
-    ]);
+    var receiver = _visitExpression(node.receiver);
+    var memberName = _emitMemberName(node.name.text, member: target);
+    var value = _visitExpression(
+      isJsMember(target) ? _assertInterop(node.value) : node.value,
+    );
+    var uncheckedSet = js.call('#.# = #', [receiver, memberName, value]);
+    if (_shouldRewriteInvocationWithHotReloadChecks(target)) {
+      // TODO(nshahan): Add a runtime helper to perform the checks only so the
+      // call site can be monomorphic.
+      var checkedSet = _emitDynamicSet(receiver, memberName, value);
+      // Dart setters can contain a return statement but that value doesn't get
+      // returned anywhere. Instead, the result of an assignment expression is
+      // the RHS. DDC relies on the same behavior in the JavaScript
+      // representation to carry the value through as the result of the
+      // assignment expression. It is then sufficient to check the type of the
+      // argument as it flows into the setter. If it is still valid, then it
+      // should also be valid to flow through to the context where the
+      // assignment appears. No extra cast on the assignment expression is
+      // needed.
+      return _emitHotReloadSafeInvocation(uncheckedSet, checkedSet);
+    }
+    return uncheckedSet;
   }
 
   /// True when the result of evaluating [e] is not known to have the Object

tests/hot_reload/call_deleted_instance_setter_ddc/config.json

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

tests/hot_reload/call_deleted_instance_setter_ddc/main.0.dart

@@ -0,0 +1,28 @@
+// 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 {
+  set deleted(String s) {}
+}
+
+helper() {
+  c = C();
+  retained = () => c!.deleted = 'hello';
+  return retained();
+}
+
+Future<void> main() async {
+  helper();
+  await hotReload();
+  Expect.throws<NoSuchMethodError>(
+    helper,
+    (error) => '$error'.contains('deleted'),
+  );
+}

tests/hot_reload/call_deleted_instance_setter_ddc/main.1.dart

@@ -0,0 +1,42 @@
+// 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 {
+-  set deleted(String s) {}
+-}
++class C {}
+ 
+ helper() {
+-  c = C();
+-  retained = () => c!.deleted = 'hello';
+   return retained();
+ }
+ 
+*/

tests/hot_reload/call_deleted_instance_setter_vm/config.json

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

tests/hot_reload/call_deleted_instance_setter_vm/main.0.dart

@@ -0,0 +1,28 @@
+// 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 {
+  set deleted(String s) {}
+}
+
+helper() {
+  c = C();
+  retained = () => c!.deleted = 'hello';
+  return retained();
+}
+
+Future<void> main() async {
+  helper();
+  await hotReload();
+  Expect.throws(
+    helper,
+    (error) => '$error'.contains('Lookup failed: deleted in @setters in C'),
+  );
+}

tests/hot_reload/call_deleted_instance_setter_vm/main.1.dart

@@ -0,0 +1,42 @@
+// 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 @setters in C'),
+  );
+}
+
+/** DIFF **/
+/*
+ var retained;
+ C? c;
+ 
+-class C {
+-  set deleted(String s) {}
+-}
++class C {}
+ 
+ helper() {
+-  c = C();
+-  retained = () => c!.deleted = 'hello';
+   return retained();
+ }
+ 
+*/

tests/hot_reload/call_instance_getter_return_change_ddc/main.1.dart

@@ -21,7 +21,10 @@ helper() {
 Future<void> main() async {
   helper();
   await hotReload();
-  Expect.throws<TypeError>(helper);
+  Expect.throws<TypeError>(
+    helper,
+    (error) => '$error'.contains("'double' is not a subtype of type 'String'"),
+  );
 }
 
 /** DIFF **/
@@ -42,13 +45,4 @@ Future<void> main() async {
    return retained!();
  }
  
- Future<void> main() async {
-   helper();
-   await hotReload();
--  Expect.throws<TypeError>(
--    helper,
--    (error) => '$error'.contains("'double' is not a subtype of type 'String'"),
--  );
-+  Expect.throws<TypeError>(helper);
- }
 */

tests/hot_reload/call_instance_setter_type_change_ddc/config.json

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

tests/hot_reload/call_instance_setter_type_change_ddc/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 _i = 0;
+  set typeChange(int i) {
+    _i = i;
+  }
+}
+
+helper() {
+  c = C();
+  retained = () => c!.typeChange = 99;
+  return retained!();
+}
+
+Future<void> main() async {
+  helper();
+  await hotReload();
+  Expect.throws<TypeError>(
+    helper,
+    (error) => '$error'.contains("'int' is not a subtype of type 'String'"),
+  );
+}