Skip to content

Commit f4c87f2

Browse files
nshahanCommit Queue
authored andcommitted
[ddc] Add hot reload checks to instance getter accesses
Checks the compile and current hot reload generations. If they are not equal the value is checked to have a getter of the expected name and if so it gets called. If not, `.noSuchMethod` is invoked instead. Issue: #60100 Change-Id: Ib80582b45f87f2ab41b943e0f263167b38601e0a Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429066 Reviewed-by: Srujan Gaddam <[email protected]> Reviewed-by: Nate Biggs <[email protected]> Commit-Queue: Nicholas Shahan <[email protected]>
1 parent ef300f5 commit f4c87f2

File tree

18 files changed

+528
-8
lines changed

18 files changed

+528
-8
lines changed

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

Lines changed: 28 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5454,9 +5454,13 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
54545454
js_ast.Expression visitDynamicGet(DynamicGet node) {
54555455
var jsReceiver = _visitExpression(node.receiver);
54565456
var jsMemberName = _emitMemberName(node.name.text);
5457-
return _runtimeCall('dload$_replSuffix(#, #)', [jsReceiver, jsMemberName]);
5457+
return _emitDynamicGet(jsReceiver, jsMemberName);
54585458
}
54595459

5460+
js_ast.Expression _emitDynamicGet(
5461+
js_ast.Expression receiver, js_ast.Expression memberName) =>
5462+
_runtimeCall('dload$_replSuffix(#, #)', [receiver, memberName]);
5463+
54605464
@override
54615465
js_ast.Expression visitInstanceGet(InstanceGet node) {
54625466
// TODO(nshahan): Marking an end span for property accessors would improve
@@ -5485,6 +5489,17 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
54855489
var jsMemberName =
54865490
_emitMemberName(memberName, member: node.interfaceTarget);
54875491
var instanceGet = js_ast.PropertyAccess(jsReceiver, jsMemberName);
5492+
if (_shouldRewriteInvocationWithHotReloadChecks(node.interfaceTarget)) {
5493+
instanceGet.sourceInformation = _nodeStart(node);
5494+
// Since there are no arguments (unlike methods) the dynamic get path can
5495+
// be reused for the hot reload checks on a getter.
5496+
var checkedGet = _emitCast(
5497+
_emitDynamicGet(
5498+
_visitExpression(receiver), _emitMemberName(memberName)),
5499+
node.resultType)
5500+
..sourceInformation = _nodeStart(node);
5501+
return _emitHotReloadSafeInvocation(instanceGet, checkedGet);
5502+
}
54885503
return _isNullCheckableJsInterop(node.interfaceTarget)
54895504
? _wrapWithJsInteropNullCheck(instanceGet)
54905505
: instanceGet;
@@ -6802,20 +6817,14 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
68026817
var staticCall = js_ast.Call(fn, args)
68036818
..sourceInformation = _nodeStart(node);
68046819
if (_shouldRewriteInvocationWithHotReloadChecks(target)) {
6805-
var generationCheck = js.call('# === #', [
6806-
js.number(_hotReloadGeneration),
6807-
_runtimeCall('global.dartDevEmbedder.hotReloadGeneration')
6808-
]);
68096820
var checkedCall = _rewriteInvocationWithHotReloadChecks(
68106821
target,
68116822
node.arguments,
68126823
node.getStaticType(_staticTypeContext),
68136824
_nodeStart(node));
68146825
// As an optimization, avoid extra checks when the invocation code was
68156826
// compiled in the same generation that it is running.
6816-
return js_ast.InvocationWithHotReloadChecks(
6817-
generationCheck, staticCall, checkedCall)
6818-
..sourceInformation = continueSourceMap;
6827+
return _emitHotReloadSafeInvocation(staticCall, checkedCall);
68196828
}
68206829
return _isNullCheckableJsInterop(target)
68216830
? _wrapWithJsInteropNullCheck(staticCall)
@@ -6831,6 +6840,17 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
68316840
!usesJSInterop(target) &&
68326841
target != _assertInteropMethod;
68336842

6843+
js_ast.Expression _emitHotReloadSafeInvocation(
6844+
js_ast.Expression uncheckedCall, js_ast.Expression checkedCall) {
6845+
var generationCheck = js.call('# === #', [
6846+
js.number(_hotReloadGeneration),
6847+
_runtimeCall('global.dartDevEmbedder.hotReloadGeneration')
6848+
]);
6849+
return js_ast.InvocationWithHotReloadChecks(
6850+
generationCheck, uncheckedCall, checkedCall)
6851+
..sourceInformation = continueSourceMap;
6852+
}
6853+
68346854
/// Rewrites an invocation of [target] that is statically sound at compile
68356855
/// time to include checks to preserve soundness in the presence of hot
68366856
/// reloads at runtime.

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1576,11 +1576,27 @@ declareClass(library, classIdentifier, classDeclaration) {
15761576
!isStateBearingSymbol(property),
15771577
originalClass,
15781578
);
1579+
deleteClassMembers(originalClass, classDeclaration);
15791580
copyProperties(originalClass, classDeclaration, copyWhen: copyWhen);
15801581
}
15811582
return JS<Object>('!', '#.#', library, classIdentifier);
15821583
}
15831584

1585+
/// Deletes the members from [oldClass] that are not present in [newClass].
1586+
///
1587+
/// Existing members not prefixed by a special identifier are replaced
1588+
/// (see [isStateBearingSymbol]).
1589+
///
1590+
/// Called from generated code.
1591+
void deleteClassMembers(Object oldClass, Object newClass) {
1592+
for (var name in getOwnNamesAndSymbols(oldClass)) {
1593+
if (JS<Object?>('', '#.#', newClass, name) == null &&
1594+
!isStateBearingSymbol(name)) {
1595+
JS('', 'delete #.#', oldClass, name);
1596+
}
1597+
}
1598+
}
1599+
15841600
/// Declares properties in [propertiesObject] on [topLevelContainer].
15851601
///
15861602
/// [topLevelContainer] must already exist.
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"exclude": [
3+
"vm"
4+
]
5+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:expect/expect.dart';
6+
import 'package:reload_test/reload_test_utils.dart';
7+
8+
var retained;
9+
C? c;
10+
11+
class C {
12+
String get deleted {
13+
return 'hello';
14+
}
15+
}
16+
17+
helper() {
18+
c = C();
19+
retained = () => c!.deleted;
20+
return retained();
21+
}
22+
23+
Future<void> main() async {
24+
helper();
25+
await hotReload();
26+
Expect.throws<NoSuchMethodError>(
27+
helper,
28+
(error) => '$error'.contains('deleted'),
29+
);
30+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:expect/expect.dart';
6+
import 'package:reload_test/reload_test_utils.dart';
7+
8+
var retained;
9+
C? c;
10+
11+
class C {}
12+
13+
helper() {
14+
return retained();
15+
}
16+
17+
Future<void> main() async {
18+
helper();
19+
await hotReload();
20+
Expect.throws<NoSuchMethodError>(
21+
helper,
22+
(error) => '$error'.contains('deleted'),
23+
);
24+
}
25+
26+
/** DIFF **/
27+
/*
28+
var retained;
29+
C? c;
30+
31+
-class C {
32+
- String get deleted {
33+
- return 'hello';
34+
- }
35+
-}
36+
+class C {}
37+
38+
helper() {
39+
- c = C();
40+
- retained = () => c!.deleted;
41+
return retained();
42+
}
43+
44+
*/
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"exclude": [
3+
"chrome",
4+
"d8"
5+
]
6+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:expect/expect.dart';
6+
import 'package:reload_test/reload_test_utils.dart';
7+
8+
var retained;
9+
C? c;
10+
11+
class C {
12+
String get deleted {
13+
return 'hello';
14+
}
15+
}
16+
17+
helper() {
18+
c = C();
19+
retained = () => c!.deleted;
20+
return retained();
21+
}
22+
23+
Future<void> main() async {
24+
helper();
25+
await hotReload();
26+
Expect.throws(
27+
helper,
28+
(error) => '$error'.contains('Lookup failed: deleted in @getters in C'),
29+
);
30+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:expect/expect.dart';
6+
import 'package:reload_test/reload_test_utils.dart';
7+
8+
var retained;
9+
C? c;
10+
11+
class C {}
12+
13+
helper() {
14+
return retained();
15+
}
16+
17+
Future<void> main() async {
18+
helper();
19+
await hotReload();
20+
Expect.throws(
21+
helper,
22+
(error) => '$error'.contains('Lookup failed: deleted in @getters in C'),
23+
);
24+
}
25+
26+
/** DIFF **/
27+
/*
28+
var retained;
29+
C? c;
30+
31+
-class C {
32+
- String get deleted {
33+
- return 'hello';
34+
- }
35+
-}
36+
+class C {}
37+
38+
helper() {
39+
- c = C();
40+
- retained = () => c!.deleted;
41+
return retained();
42+
}
43+
44+
*/
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"exclude": [
3+
"vm"
4+
]
5+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
import 'package:expect/expect.dart';
6+
import 'package:reload_test/reload_test_utils.dart';
7+
8+
int Function()? retained;
9+
C? c;
10+
11+
class C {
12+
String get returnChange {
13+
return 'hello';
14+
}
15+
}
16+
17+
helper() {
18+
c = C();
19+
retained = () => c!.returnChange.length;
20+
return retained!();
21+
}
22+
23+
Future<void> main() async {
24+
helper();
25+
await hotReload();
26+
Expect.throws<TypeError>(
27+
helper,
28+
(error) => '$error'.contains("'double' is not a subtype of type 'String'"),
29+
);
30+
}

0 commit comments

Comments
 (0)