[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 <jensj@google.com>
Reviewed-by: Srujan Gaddam <srujzs@google.com>
Reviewed-by: Mark Zhou <markzipan@google.com>
Reviewed-by: Nate Biggs <natebiggs@google.com>
Reviewed-by: Samuel Rawlins <srawlins@google.com>
Commit-Queue: Nicholas Shahan <nshahan@google.com>
diff --git a/pkg/dev_compiler/lib/src/kernel/hot_reload_delta_inspector.dart b/pkg/dev_compiler/lib/src/kernel/hot_reload_delta_inspector.dart
new file mode 100644
index 0000000..6720ed7
--- /dev/null
+++ b/pkg/dev_compiler/lib/src/kernel/hot_reload_delta_inspector.dart
@@ -0,0 +1,55 @@
+// Copyright (c) 2024, 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:kernel/ast.dart';
+import 'package:kernel/library_index.dart';
+
+/// Inspects a delta [Component] and compares against the last known accepted
+/// version.
+class HotReloadDeltaInspector {
+ /// A partial index for the last accepted generation [Component].
+ ///
+ /// In practice this is likely a partial index of the last known accepted
+ /// generation that only contains the libraries present in the delta.
+ late LibraryIndex _partialLastAcceptedLibraryIndex;
+
+ /// Rejection errors discovered while comparing a delta with the previous
+ /// generation.
+ final _rejectionMessages = <String>[];
+
+ /// Returns all hot reload rejection errors discovered while comparing [delta]
+ /// against the [lastAccepted] version.
+ // TODO(nshahan): Annotate delta component with information for DDC.
+ List<String> compareGenerations(Component lastAccepted, Component delta) {
+ _partialLastAcceptedLibraryIndex = LibraryIndex(lastAccepted,
+ [for (var library in delta.libraries) '${library.fileUri}']);
+ _rejectionMessages.clear();
+
+ for (var library in delta.libraries) {
+ for (var deltaClass in library.classes) {
+ final acceptedClass = _partialLastAcceptedLibraryIndex.tryGetClass(
+ '${library.importUri}', deltaClass.name);
+ if (acceptedClass == null) {
+ // No previous version of the class to compare with.
+ continue;
+ }
+ _checkConstClassConsistency(acceptedClass, deltaClass);
+ }
+ }
+ return _rejectionMessages;
+ }
+
+ /// Records a rejection error when [acceptedClass] is const but [deltaClass]
+ /// is non-const.
+ ///
+ /// [acceptedClass] and [deltaClass] must represent the same class in the
+ /// last known accepted and delta components respectively.
+ void _checkConstClassConsistency(Class acceptedClass, Class deltaClass) {
+ if (acceptedClass.hasConstConstructor && !deltaClass.hasConstConstructor) {
+ _rejectionMessages.add('Const class cannot become non-const: '
+ "Library:'${deltaClass.enclosingLibrary.importUri}' "
+ 'Class: ${deltaClass.name}');
+ }
+ }
+}
diff --git a/pkg/dev_compiler/test/hot_reload_delta_inspector_test.dart b/pkg/dev_compiler/test/hot_reload_delta_inspector_test.dart
new file mode 100644
index 0000000..718d31d
--- /dev/null
+++ b/pkg/dev_compiler/test/hot_reload_delta_inspector_test.dart
@@ -0,0 +1,179 @@
+// Copyright (c) 2024, 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:dev_compiler/src/kernel/hot_reload_delta_inspector.dart';
+import 'package:kernel/ast.dart';
+import 'package:test/test.dart';
+
+import 'memory_compiler.dart';
+
+Future<void> main() async {
+ group('const classes', () {
+ final deltaInspector = HotReloadDeltaInspector();
+ test('rejection when removing only const constructor', () async {
+ final initialSource = '''
+ var globalVariable;
+
+ class A {
+ final String s;
+ const A(this.s);
+ }
+
+ main() {
+ globalVariable = const A('hello');
+ print(globalVariable.s);
+ }
+ ''';
+ final deltaSource = '''
+ var globalVariable;
+
+ class A {
+ final String s;
+ A(this.s);
+ }
+
+ main() {
+ print('hello world');
+ }
+ ''';
+ final (:initial, :delta) =
+ await compileComponents(initialSource, deltaSource);
+ expect(
+ deltaInspector.compareGenerations(initial, delta),
+ unorderedEquals([
+ 'Const class cannot become non-const: '
+ "Library:'memory:///main.dart' "
+ 'Class: A'
+ ]));
+ });
+ test('multiple rejections when removing only const constructors', () async {
+ final initialSource = '''
+ var globalA, globalB, globalC, globalD;
+
+ class A {
+ final String s;
+ const A(this.s);
+ }
+
+ class B {
+ final String s;
+ const B(this.s);
+ }
+
+ class C {
+ final String s;
+ C(this.s);
+ }
+
+ class D {
+ final String s;
+ const D(this.s);
+ }
+
+ main() {
+ globalA = const A('hello');
+ globalB = const B('world');
+ globalC = C('hello');
+ globalD = const D('world');
+ print(globalA.s);
+ print(globalB.s);
+ print(globalC.s);
+ print(globalD.s);
+ }
+ ''';
+ final deltaSource = '''
+ var globalA, globalB, globalC, globalD;
+
+ class A {
+ final String s;
+ A(this.s);
+ }
+
+ class B {
+ final String s;
+ const B(this.s);
+ }
+
+ class C {
+ final String s;
+ C(this.s);
+ }
+
+ class D {
+ final String s;
+ D(this.s);
+ }
+
+ main() {
+ print('hello world');
+ }
+ ''';
+ final (:initial, :delta) =
+ await compileComponents(initialSource, deltaSource);
+ expect(
+ deltaInspector.compareGenerations(initial, delta),
+ unorderedEquals([
+ 'Const class cannot become non-const: '
+ "Library:'memory:///main.dart' "
+ 'Class: A',
+ 'Const class cannot become non-const: '
+ "Library:'memory:///main.dart' "
+ 'Class: D'
+ ]));
+ });
+
+ test('no error when removing const constructor while adding another',
+ () async {
+ final initialSource = '''
+ var globalVariable;
+
+ class A {
+ final String s;
+ const A(this.s);
+ }
+
+ main() {
+ globalVariable = const A('hello');
+ print(globalVariable.s);
+ }
+ ''';
+ final deltaSource = '''
+ var globalVariable;
+
+ class A {
+ final String s;
+ A(this.s);
+ const A.named(this.s);
+ }
+
+ main() {
+ print('hello world');
+ }
+ ''';
+ final (:initial, :delta) =
+ await compileComponents(initialSource, deltaSource);
+ expect(deltaInspector.compareGenerations(initial, delta), isEmpty);
+ });
+ });
+}
+
+/// Test only helper compiles [initialSource] and [deltaSource] and returns two
+/// kernel components.
+Future<({Component initial, Component delta})> compileComponents(
+ String initialSource, String deltaSource) async {
+ final fileName = 'main.dart';
+ final fileUri = Uri(scheme: 'memory', host: '', path: fileName);
+ final memoryFileMap = {fileName: initialSource};
+ final initialResult = await componentFromMemory(memoryFileMap, fileUri);
+ expect(initialResult.errors, isEmpty,
+ reason: 'Initial source produced compile time errors.');
+ memoryFileMap[fileName] = deltaSource;
+ final deltaResult = await componentFromMemory(memoryFileMap, fileUri);
+ expect(deltaResult.errors, isEmpty,
+ reason: 'Delta source produced compile time errors.');
+ return (
+ initial: initialResult.ddcResult.component,
+ delta: deltaResult.ddcResult.component
+ );
+}
diff --git a/pkg/dev_compiler/test/memory_compiler.dart b/pkg/dev_compiler/test/memory_compiler.dart
index ff7a177..20475cc 100644
--- a/pkg/dev_compiler/test/memory_compiler.dart
+++ b/pkg/dev_compiler/test/memory_compiler.dart
@@ -22,16 +22,28 @@
this.ddcResult, this.compiler, this.program, this.errors);
}
+/// Result of compiling using [componentFromMemory].
+///
+/// This is meant for use in tests and performs the front end compilation to
+/// components without calling DDC.
+class MemoryComponentResult {
+ final fe.DdcResult ddcResult;
+ final List<fe.DiagnosticMessage> errors;
+
+ MemoryComponentResult(this.ddcResult, this.errors);
+}
+
/// Uri used as the base uri for files provided in memory through the
/// [MemoryFileSystem].
Uri memoryDirectory = Uri.parse('memory://');
-/// Compiles [entryPoint] using the [memoryFiles] as sources.
+/// Compiles [entryPoint] to a kernel `Component` using the [memoryFiles] as
+/// sources.
///
/// [memoryFiles] maps relative paths to their source text. [entryPoint] must
-/// be absolute, using [memoryDirectory] as base uri to refer to a file from
+/// be absolute, using [memoryDirectory] as a base uri to refer to a file from
/// [memoryFiles].
-Future<MemoryCompilerResult> compileFromMemory(
+Future<MemoryComponentResult> componentFromMemory(
Map<String, String> memoryFiles, Uri entryPoint,
{Map<fe.ExperimentalFlag, bool>? explicitExperimentalFlags}) async {
var errors = <fe.DiagnosticMessage>[];
@@ -48,7 +60,6 @@
.entityForUri(memoryDirectory.resolve(entry.key))
.writeAsStringSync(entry.value);
}
- var options = Options(moduleName: 'test');
var compilerState = fe.initializeCompiler(
null,
false,
@@ -68,6 +79,20 @@
if (result == null) {
throw 'Memory compilation failed';
}
+ return MemoryComponentResult(result, errors);
+}
+
+/// Compiles [entryPoint] to JavaScript using the [memoryFiles] as sources.
+///
+/// [memoryFiles] maps relative paths to their source text. [entryPoint] must
+/// be absolute, using [memoryDirectory] as a base uri to refer to a file from
+/// [memoryFiles].
+Future<MemoryCompilerResult> compileFromMemory(
+ Map<String, String> memoryFiles, Uri entryPoint,
+ {Map<fe.ExperimentalFlag, bool>? explicitExperimentalFlags}) async {
+ var MemoryComponentResult(ddcResult: result, :errors) =
+ await componentFromMemory(memoryFiles, entryPoint);
+ var options = Options(moduleName: 'test');
var compiler =
// TODO(nshahan): Do we need to support [importToSummary] and
// [summaryToModule].
diff --git a/pkg/front_end/test/spell_checking_list_common.txt b/pkg/front_end/test/spell_checking_list_common.txt
index 863a5b0..fe92305 100644
--- a/pkg/front_end/test/spell_checking_list_common.txt
+++ b/pkg/front_end/test/spell_checking_list_common.txt
@@ -1360,6 +1360,7 @@
generated
generates
generating
+generations
generative
generator
generators
@@ -1471,6 +1472,7 @@
horribly
host
hostnames
+hot
how
however
http
@@ -2575,6 +2577,7 @@
reinsert
reissue
rejected
+rejections
rejects
relate
related
diff --git a/pkg/frontend_server/lib/frontend_server.dart b/pkg/frontend_server/lib/frontend_server.dart
index e80472c..441d6ca 100644
--- a/pkg/frontend_server/lib/frontend_server.dart
+++ b/pkg/frontend_server/lib/frontend_server.dart
@@ -1048,6 +1048,7 @@
_options['filesystem-scheme'], _options['dartdevc-module-format'],
fullComponent: false);
} catch (e) {
+ _outputStream.writeln('$e');
errors.add(e.toString());
}
} else {
diff --git a/pkg/frontend_server/lib/src/javascript_bundle.dart b/pkg/frontend_server/lib/src/javascript_bundle.dart
index 5ca3d6b..9920adf 100644
--- a/pkg/frontend_server/lib/src/javascript_bundle.dart
+++ b/pkg/frontend_server/lib/src/javascript_bundle.dart
@@ -10,6 +10,7 @@
import 'package:dev_compiler/dev_compiler.dart';
import 'package:dev_compiler/src/command/command.dart';
+import 'package:dev_compiler/src/kernel/hot_reload_delta_inspector.dart';
import 'package:dev_compiler/src/js_ast/nodes.dart';
import 'package:front_end/src/api_unstable/vm.dart' show FileSystem;
import 'package:kernel/ast.dart';
@@ -55,6 +56,7 @@
final _summaryToLibraryBundleName = new Map<Component, String>.identity();
final Map<Uri, String> _summaryToLibraryBundleJSPath = <Uri, String>{};
final String _fileSystemScheme;
+ final HotReloadDeltaInspector _deltaInspector = new HotReloadDeltaInspector();
late Component _lastFullComponent;
late Component _currentComponent;
@@ -83,6 +85,13 @@
Component lastFullComponent,
Uri mainUri,
PackageConfig packageConfig) async {
+ if (canaryFeatures && _moduleFormat == ModuleFormat.ddc) {
+ // Find any potential hot reload rejections before updating the strongly
+ // connected component graph.
+ final List<String> errors = _deltaInspector.compareGenerations(
+ lastFullComponent, partialComponent);
+ if (errors.isNotEmpty) throw new Exception(errors.join('/n'));
+ }
_currentComponent = partialComponent;
_updateFullComponent(lastFullComponent, partialComponent);
_strongComponents = new StrongComponents(
