Skip to content

Commit 7d7d9ab

Browse files
MarkzipanCommit Queue
Markzipan
authored and
Commit Queue
committed
[ddc] Extending static fields to perform type checks on first access.
Type changes across hot reloads need to be performed even if the underlying value has already been initialized. To avoid extraneous type checks, getters replace themselves with a direct access on their value store on first access. Also updates our builder to support get/set as property descriptor functions. Adds some extra conditions that will be cleaned up when pragma support is added (#57049). Change-Id: I44f4a9e1a6fd8a49aca5e629fb21348170a9d80f Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/393041 Reviewed-by: Nicholas Shahan <[email protected]> Commit-Queue: Mark Zhou <[email protected]>
1 parent 107995f commit 7d7d9ab

File tree

6 files changed

+212
-8
lines changed

6 files changed

+212
-8
lines changed

‎pkg/dev_compiler/lib/src/js_ast/builder.dart‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1708,8 +1708,8 @@ class MiniJsParser {
17081708
if (isGetter || isSetter) {
17091709
var token = lastToken;
17101710
getToken();
1711-
if (lastCategory == COLON) {
1712-
// That wasn't a accessor but the 'get' or 'set' property: retropedal.
1711+
if (lastCategory == COLON || lastCategory == LPAREN) {
1712+
// That wasn't a accessor but the 'get' or 'set' property/function.
17131713
isGetter = isSetter = false;
17141714
name = LiteralString('"$token"');
17151715
}

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

Lines changed: 126 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2747,6 +2747,130 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
27472747
/// sentinel value if [field] is final to detect multiple initializations.
27482748
js_ast.Fun _emitLazyInitializingFunction(js_ast.Expression valueCache,
27492749
js_ast.Expression initializer, Field field) {
2750+
// We avoid emitting casts for top level fields in the legacy SDK since
2751+
// some are used for legacy type checks and must be initialized to avoid
2752+
// infinite loops.
2753+
var initialFieldValueExpression =
2754+
!_options.soundNullSafety && _isSdkInternalRuntime(_currentLibrary!)
2755+
? valueCache
2756+
: _emitCast(valueCache, field.type);
2757+
2758+
// Lazy static fields require an additional type check around their value
2759+
// cache if their type is updated after hot reload. To avoid a type check
2760+
// on every access, the generated getter overrides itself with a direct
2761+
// access on its underlying value cache on first access.
2762+
// TODO(markzipan): The performance ramifications of a lookup vs
2763+
// self-rewriting "smart" getter are unknown. We should revisit this if
2764+
// property accesses become a bottleneck.
2765+
if (field.isStatic) {
2766+
var getterName = memberNames[field]!;
2767+
// Final fields are generated with additional logic to detect
2768+
// initialization cycles via a special sentinel.
2769+
if (field.isFinal) {
2770+
var finalLateInitDetectorSentinel = _getSymbol(
2771+
_emitPrivateNameSymbol(field.enclosingLibrary, '_#initializing'));
2772+
// Emits code like:
2773+
//
2774+
// if ([valueCache] === _#initializing)
2775+
// dart.throwLateInitializationError(field);
2776+
// if ([valueCache] === void 0) {
2777+
// [valueCache] = _#initializing;
2778+
// try {
2779+
// [valueCache] = initializer;
2780+
// } catch (e) {
2781+
// // Reset the sentinel on error so it can be reinitialized.
2782+
// if ([valueCache] === _#initializing) {
2783+
// [valueCache] = void 0;
2784+
// }
2785+
// throw e;
2786+
// }
2787+
// }
2788+
// _typeCheck([valueCache]);
2789+
// Object.defineProperty(this, field, {
2790+
// get() {
2791+
// return [valueCache];
2792+
// }
2793+
// });
2794+
// return this.field;
2795+
return js.fun(r'''
2796+
function() {
2797+
if (# === #) #;
2798+
if (# === void 0) {
2799+
# = #;
2800+
try {
2801+
# = #;
2802+
} catch (e) {
2803+
if (# === #) {
2804+
# = void 0;
2805+
}
2806+
throw e;
2807+
}
2808+
}
2809+
#;
2810+
Object.defineProperty(this, #, {
2811+
get() {
2812+
return #;
2813+
}
2814+
});
2815+
return this.#;
2816+
}
2817+
''', [
2818+
valueCache,
2819+
finalLateInitDetectorSentinel,
2820+
_runtimeCall(
2821+
'throwLateInitializationError(#)',
2822+
[js.string(field.name.text)],
2823+
),
2824+
valueCache,
2825+
valueCache,
2826+
finalLateInitDetectorSentinel,
2827+
valueCache,
2828+
initializer,
2829+
valueCache,
2830+
finalLateInitDetectorSentinel,
2831+
valueCache,
2832+
initialFieldValueExpression,
2833+
js.string(getterName),
2834+
valueCache,
2835+
getterName,
2836+
]);
2837+
} else {
2838+
// Emits code like:
2839+
//
2840+
// if ([valueCache] === void 0) {
2841+
// [valueCache] = initializer;
2842+
// }
2843+
// _typeCheck([valueCache]);
2844+
// Object.defineProperty(this, field, {
2845+
// get() {
2846+
// return [valueCache];
2847+
// }
2848+
// });
2849+
// return this.field;
2850+
return js.fun(r'''
2851+
function() {
2852+
if (# === void 0) {
2853+
# = #;
2854+
}
2855+
#;
2856+
Object.defineProperty(this, #, {
2857+
get() {
2858+
return #;
2859+
}
2860+
});
2861+
return this.#;
2862+
}
2863+
''', [
2864+
valueCache,
2865+
valueCache,
2866+
initializer,
2867+
initialFieldValueExpression,
2868+
js.string(getterName),
2869+
valueCache,
2870+
getterName,
2871+
]);
2872+
}
2873+
}
27502874
// Final fields are generated with additional logic to detect
27512875
// initialization cycles via a special sentinel.
27522876
if (field.isFinal) {
@@ -2800,7 +2924,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
28002924
valueCache,
28012925
finalLateInitDetectorSentinel,
28022926
valueCache,
2803-
valueCache,
2927+
initialFieldValueExpression,
28042928
]);
28052929
} else {
28062930
return js.fun(r'''
@@ -2814,7 +2938,7 @@ class LibraryCompiler extends ComputeOnceConstantVisitor<js_ast.Expression>
28142938
valueCache,
28152939
valueCache,
28162940
initializer,
2817-
valueCache,
2941+
initialFieldValueExpression,
28182942
]);
28192943
}
28202944
}

‎sdk/lib/_internal/js_dev_runtime/private/ddc_runtime/records.dart‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@ final class Shape {
1010
final int positionals;
1111

1212
/// The names of the named elements in the record in alphabetical order.
13-
final List<String>? named;
13+
///
14+
/// This is a JS Array of JS String literals. We avoid typing this explicitly
15+
/// so it can remain unboxed, as it's only used for our internal
16+
/// representation.
17+
final List<dynamic /* String */ >? named;
1418

1519
Shape(this.positionals, this.named);
1620

@@ -149,7 +153,7 @@ final _records = JS('!', 'new Map()');
149153
Shape registerShape(
150154
@notNull String shapeKey,
151155
@notNull int positionals,
152-
List<String>? named,
156+
List<dynamic /* String */ >? named,
153157
) {
154158
var cached = JS<Shape?>('', '#.get(#)', shapes, shapeKey);
155159
if (cached != null) {
@@ -169,7 +173,7 @@ Shape registerShape(
169173
Object registerRecord(
170174
@notNull String shapeKey,
171175
@notNull int positionals,
172-
List<String>? named,
176+
List<dynamic /* String */ >? named,
173177
) {
174178
var cached = JS('', '#.get(#)', _records, shapeKey);
175179
if (cached != null) {
@@ -245,7 +249,7 @@ Object registerRecord(
245249
Object recordLiteral(
246250
@notNull String shapeKey,
247251
@notNull int positionals,
248-
List<String>? named,
252+
List<dynamic /* String */ >? named,
249253
@notNull List values,
250254
) {
251255
var shape = registerShape(shapeKey, positionals, named);

‎tests/hot_reload/existing_field_changes_type_no_initializer/config.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
{
2+
"exclude": []
3+
}

‎tests/hot_reload/existing_field_changes_type_no_initializer/main.0.dart‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright (c) 2024, 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+
class Foo {
9+
int x;
10+
Foo(this.x);
11+
}
12+
13+
late Foo foo;
14+
15+
helper() {
16+
foo = Foo(42);
17+
}
18+
19+
Future<void> main() async {
20+
helper();
21+
Expect.type<int>(foo.x);
22+
Expect.equals(42, foo.x);
23+
24+
await hotReload();
25+
26+
Expect.throws<TypeError>(() => foo.x);
27+
}

‎tests/hot_reload/existing_field_changes_type_no_initializer/main.1.dart‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// Copyright (c) 2024, 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+
class Foo {
9+
String x;
10+
Foo(this.x);
11+
}
12+
13+
late Foo foo;
14+
15+
helper() {}
16+
17+
Future<void> main() async {
18+
helper();
19+
Expect.type<int>(foo.x);
20+
Expect.equals(42, foo.x);
21+
22+
await hotReload();
23+
24+
Expect.throws<TypeError>(() => foo.x);
25+
}
26+
/** DIFF **/
27+
/*
28+
@@ -6,15 +6,13 @@
29+
import 'package:reload_test/reload_test_utils.dart';
30+
31+
class Foo {
32+
- int x;
33+
+ String x;
34+
Foo(this.x);
35+
}
36+
37+
late Foo foo;
38+
39+
-helper() {
40+
- foo = Foo(42);
41+
-}
42+
+helper() {}
43+
44+
Future<void> main() async {
45+
helper();
46+
*/

0 commit comments

Comments
 (0)