Skip to content

Commit f91050c

Browse files
nshahanCommit Queue
nshahan
authored and
Commit Queue
committed
[ddc] Add visitor for hot reload deltas
Initially this visitor will reject reloads (by throwing an Exception). Future changes will add the ability to record data about the delta to guide the JavaScript compilation decisions. - Rejects deltas that delete all const constructors from a class (making it non-const). - Adds ability to test the visitor directly by compiling components from source via an in-memory compiler. - Updates DDC frontend_server compiles to write errors to the output stream. Change-Id: Ib318f42d28367416983266a214f97821a38a2913 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/401600 Reviewed-by: Jens Johansen <[email protected]> Reviewed-by: Srujan Gaddam <[email protected]> Reviewed-by: Mark Zhou <[email protected]> Reviewed-by: Nate Biggs <[email protected]> Reviewed-by: Samuel Rawlins <[email protected]> Commit-Queue: Nicholas Shahan <[email protected]>
1 parent ef771d4 commit f91050c

File tree

6 files changed

+276
-4
lines changed

6 files changed

+276
-4
lines changed

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

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
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:kernel/ast.dart';
6+
import 'package:kernel/library_index.dart';
7+
8+
/// Inspects a delta [Component] and compares against the last known accepted
9+
/// version.
10+
class HotReloadDeltaInspector {
11+
/// A partial index for the last accepted generation [Component].
12+
///
13+
/// In practice this is likely a partial index of the last known accepted
14+
/// generation that only contains the libraries present in the delta.
15+
late LibraryIndex _partialLastAcceptedLibraryIndex;
16+
17+
/// Rejection errors discovered while comparing a delta with the previous
18+
/// generation.
19+
final _rejectionMessages = <String>[];
20+
21+
/// Returns all hot reload rejection errors discovered while comparing [delta]
22+
/// against the [lastAccepted] version.
23+
// TODO(nshahan): Annotate delta component with information for DDC.
24+
List<String> compareGenerations(Component lastAccepted, Component delta) {
25+
_partialLastAcceptedLibraryIndex = LibraryIndex(lastAccepted,
26+
[for (var library in delta.libraries) '${library.fileUri}']);
27+
_rejectionMessages.clear();
28+
29+
for (var library in delta.libraries) {
30+
for (var deltaClass in library.classes) {
31+
final acceptedClass = _partialLastAcceptedLibraryIndex.tryGetClass(
32+
'${library.importUri}', deltaClass.name);
33+
if (acceptedClass == null) {
34+
// No previous version of the class to compare with.
35+
continue;
36+
}
37+
_checkConstClassConsistency(acceptedClass, deltaClass);
38+
}
39+
}
40+
return _rejectionMessages;
41+
}
42+
43+
/// Records a rejection error when [acceptedClass] is const but [deltaClass]
44+
/// is non-const.
45+
///
46+
/// [acceptedClass] and [deltaClass] must represent the same class in the
47+
/// last known accepted and delta components respectively.
48+
void _checkConstClassConsistency(Class acceptedClass, Class deltaClass) {
49+
if (acceptedClass.hasConstConstructor && !deltaClass.hasConstConstructor) {
50+
_rejectionMessages.add('Const class cannot become non-const: '
51+
"Library:'${deltaClass.enclosingLibrary.importUri}' "
52+
'Class: ${deltaClass.name}');
53+
}
54+
}
55+
}

‎pkg/dev_compiler/test/hot_reload_delta_inspector_test.dart‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
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:dev_compiler/src/kernel/hot_reload_delta_inspector.dart';
6+
import 'package:kernel/ast.dart';
7+
import 'package:test/test.dart';
8+
9+
import 'memory_compiler.dart';
10+
11+
Future<void> main() async {
12+
group('const classes', () {
13+
final deltaInspector = HotReloadDeltaInspector();
14+
test('rejection when removing only const constructor', () async {
15+
final initialSource = '''
16+
var globalVariable;
17+
18+
class A {
19+
final String s;
20+
const A(this.s);
21+
}
22+
23+
main() {
24+
globalVariable = const A('hello');
25+
print(globalVariable.s);
26+
}
27+
''';
28+
final deltaSource = '''
29+
var globalVariable;
30+
31+
class A {
32+
final String s;
33+
A(this.s);
34+
}
35+
36+
main() {
37+
print('hello world');
38+
}
39+
''';
40+
final (:initial, :delta) =
41+
await compileComponents(initialSource, deltaSource);
42+
expect(
43+
deltaInspector.compareGenerations(initial, delta),
44+
unorderedEquals([
45+
'Const class cannot become non-const: '
46+
"Library:'memory:///main.dart' "
47+
'Class: A'
48+
]));
49+
});
50+
test('multiple rejections when removing only const constructors', () async {
51+
final initialSource = '''
52+
var globalA, globalB, globalC, globalD;
53+
54+
class A {
55+
final String s;
56+
const A(this.s);
57+
}
58+
59+
class B {
60+
final String s;
61+
const B(this.s);
62+
}
63+
64+
class C {
65+
final String s;
66+
C(this.s);
67+
}
68+
69+
class D {
70+
final String s;
71+
const D(this.s);
72+
}
73+
74+
main() {
75+
globalA = const A('hello');
76+
globalB = const B('world');
77+
globalC = C('hello');
78+
globalD = const D('world');
79+
print(globalA.s);
80+
print(globalB.s);
81+
print(globalC.s);
82+
print(globalD.s);
83+
}
84+
''';
85+
final deltaSource = '''
86+
var globalA, globalB, globalC, globalD;
87+
88+
class A {
89+
final String s;
90+
A(this.s);
91+
}
92+
93+
class B {
94+
final String s;
95+
const B(this.s);
96+
}
97+
98+
class C {
99+
final String s;
100+
C(this.s);
101+
}
102+
103+
class D {
104+
final String s;
105+
D(this.s);
106+
}
107+
108+
main() {
109+
print('hello world');
110+
}
111+
''';
112+
final (:initial, :delta) =
113+
await compileComponents(initialSource, deltaSource);
114+
expect(
115+
deltaInspector.compareGenerations(initial, delta),
116+
unorderedEquals([
117+
'Const class cannot become non-const: '
118+
"Library:'memory:///main.dart' "
119+
'Class: A',
120+
'Const class cannot become non-const: '
121+
"Library:'memory:///main.dart' "
122+
'Class: D'
123+
]));
124+
});
125+
126+
test('no error when removing const constructor while adding another',
127+
() async {
128+
final initialSource = '''
129+
var globalVariable;
130+
131+
class A {
132+
final String s;
133+
const A(this.s);
134+
}
135+
136+
main() {
137+
globalVariable = const A('hello');
138+
print(globalVariable.s);
139+
}
140+
''';
141+
final deltaSource = '''
142+
var globalVariable;
143+
144+
class A {
145+
final String s;
146+
A(this.s);
147+
const A.named(this.s);
148+
}
149+
150+
main() {
151+
print('hello world');
152+
}
153+
''';
154+
final (:initial, :delta) =
155+
await compileComponents(initialSource, deltaSource);
156+
expect(deltaInspector.compareGenerations(initial, delta), isEmpty);
157+
});
158+
});
159+
}
160+
161+
/// Test only helper compiles [initialSource] and [deltaSource] and returns two
162+
/// kernel components.
163+
Future<({Component initial, Component delta})> compileComponents(
164+
String initialSource, String deltaSource) async {
165+
final fileName = 'main.dart';
166+
final fileUri = Uri(scheme: 'memory', host: '', path: fileName);
167+
final memoryFileMap = {fileName: initialSource};
168+
final initialResult = await componentFromMemory(memoryFileMap, fileUri);
169+
expect(initialResult.errors, isEmpty,
170+
reason: 'Initial source produced compile time errors.');
171+
memoryFileMap[fileName] = deltaSource;
172+
final deltaResult = await componentFromMemory(memoryFileMap, fileUri);
173+
expect(deltaResult.errors, isEmpty,
174+
reason: 'Delta source produced compile time errors.');
175+
return (
176+
initial: initialResult.ddcResult.component,
177+
delta: deltaResult.ddcResult.component
178+
);
179+
}

‎pkg/dev_compiler/test/memory_compiler.dart‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,28 @@ class MemoryCompilerResult {
2222
this.ddcResult, this.compiler, this.program, this.errors);
2323
}
2424

25+
/// Result of compiling using [componentFromMemory].
26+
///
27+
/// This is meant for use in tests and performs the front end compilation to
28+
/// components without calling DDC.
29+
class MemoryComponentResult {
30+
final fe.DdcResult ddcResult;
31+
final List<fe.DiagnosticMessage> errors;
32+
33+
MemoryComponentResult(this.ddcResult, this.errors);
34+
}
35+
2536
/// Uri used as the base uri for files provided in memory through the
2637
/// [MemoryFileSystem].
2738
Uri memoryDirectory = Uri.parse('memory://');
2839

29-
/// Compiles [entryPoint] using the [memoryFiles] as sources.
40+
/// Compiles [entryPoint] to a kernel `Component` using the [memoryFiles] as
41+
/// sources.
3042
///
3143
/// [memoryFiles] maps relative paths to their source text. [entryPoint] must
32-
/// be absolute, using [memoryDirectory] as base uri to refer to a file from
44+
/// be absolute, using [memoryDirectory] as a base uri to refer to a file from
3345
/// [memoryFiles].
34-
Future<MemoryCompilerResult> compileFromMemory(
46+
Future<MemoryComponentResult> componentFromMemory(
3547
Map<String, String> memoryFiles, Uri entryPoint,
3648
{Map<fe.ExperimentalFlag, bool>? explicitExperimentalFlags}) async {
3749
var errors = <fe.DiagnosticMessage>[];
@@ -48,7 +60,6 @@ Future<MemoryCompilerResult> compileFromMemory(
4860
.entityForUri(memoryDirectory.resolve(entry.key))
4961
.writeAsStringSync(entry.value);
5062
}
51-
var options = Options(moduleName: 'test');
5263
var compilerState = fe.initializeCompiler(
5364
null,
5465
false,
@@ -68,6 +79,20 @@ Future<MemoryCompilerResult> compileFromMemory(
6879
if (result == null) {
6980
throw 'Memory compilation failed';
7081
}
82+
return MemoryComponentResult(result, errors);
83+
}
84+
85+
/// Compiles [entryPoint] to JavaScript using the [memoryFiles] as sources.
86+
///
87+
/// [memoryFiles] maps relative paths to their source text. [entryPoint] must
88+
/// be absolute, using [memoryDirectory] as a base uri to refer to a file from
89+
/// [memoryFiles].
90+
Future<MemoryCompilerResult> compileFromMemory(
91+
Map<String, String> memoryFiles, Uri entryPoint,
92+
{Map<fe.ExperimentalFlag, bool>? explicitExperimentalFlags}) async {
93+
var MemoryComponentResult(ddcResult: result, :errors) =
94+
await componentFromMemory(memoryFiles, entryPoint);
95+
var options = Options(moduleName: 'test');
7196
var compiler =
7297
// TODO(nshahan): Do we need to support [importToSummary] and
7398
// [summaryToModule].

‎pkg/front_end/test/spell_checking_list_common.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1360,6 +1360,7 @@ generate
13601360
generated
13611361
generates
13621362
generating
1363+
generations
13631364
generative
13641365
generator
13651366
generators
@@ -1471,6 +1472,7 @@ horns
14711472
horribly
14721473
host
14731474
hostnames
1475+
hot
14741476
how
14751477
however
14761478
http
@@ -2575,6 +2577,7 @@ regular
25752577
reinsert
25762578
reissue
25772579
rejected
2580+
rejections
25782581
rejects
25792582
relate
25802583
related

‎pkg/frontend_server/lib/frontend_server.dart‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1048,6 +1048,7 @@ class FrontendCompiler implements CompilerInterface {
10481048
_options['filesystem-scheme'], _options['dartdevc-module-format'],
10491049
fullComponent: false);
10501050
} catch (e) {
1051+
_outputStream.writeln('$e');
10511052
errors.add(e.toString());
10521053
}
10531054
} else {

‎pkg/frontend_server/lib/src/javascript_bundle.dart‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import 'dart:typed_data';
1010

1111
import 'package:dev_compiler/dev_compiler.dart';
1212
import 'package:dev_compiler/src/command/command.dart';
13+
import 'package:dev_compiler/src/kernel/hot_reload_delta_inspector.dart';
1314
import 'package:dev_compiler/src/js_ast/nodes.dart';
1415
import 'package:front_end/src/api_unstable/vm.dart' show FileSystem;
1516
import 'package:kernel/ast.dart';
@@ -55,6 +56,7 @@ class IncrementalJavaScriptBundler {
5556
final _summaryToLibraryBundleName = new Map<Component, String>.identity();
5657
final Map<Uri, String> _summaryToLibraryBundleJSPath = <Uri, String>{};
5758
final String _fileSystemScheme;
59+
final HotReloadDeltaInspector _deltaInspector = new HotReloadDeltaInspector();
5860

5961
late Component _lastFullComponent;
6062
late Component _currentComponent;
@@ -83,6 +85,13 @@ class IncrementalJavaScriptBundler {
8385
Component lastFullComponent,
8486
Uri mainUri,
8587
PackageConfig packageConfig) async {
88+
if (canaryFeatures && _moduleFormat == ModuleFormat.ddc) {
89+
// Find any potential hot reload rejections before updating the strongly
90+
// connected component graph.
91+
final List<String> errors = _deltaInspector.compareGenerations(
92+
lastFullComponent, partialComponent);
93+
if (errors.isNotEmpty) throw new Exception(errors.join('/n'));
94+
}
8695
_currentComponent = partialComponent;
8796
_updateFullComponent(lastFullComponent, partialComponent);
8897
_strongComponents = new StrongComponents(

0 commit comments

Comments
 (0)