[front_end] Add option for expanded invalidation
In this mode when an edit only touches the body of a mixin (not the public API) libraries that apply the mixin will also be invalidated. Add mixin invalidation test. Issue: https://github.com/dart-lang/sdk/issues/61864 Change-Id: Id4dde67364dcef958ae1e0e0bbf124cec389f5f0 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/459343 Commit-Queue: Nicholas Shahan <nshahan@google.com> Reviewed-by: Jens Johansen <jensj@google.com>
This commit is contained in:
committed by
Commit Queue
parent
37574dfdf0
commit
c2c5357a25
@@ -328,6 +328,15 @@ class DevCompilerTarget extends Target {
|
||||
@override
|
||||
DartLibrarySupport get dartLibrarySupport =>
|
||||
const DevCompilerDartLibrarySupport();
|
||||
|
||||
// For correctness the DDC runtime needs to reevaluate libraries that contain
|
||||
// mixin applications when the mixin was edited. If the edit was only within
|
||||
// the body of a mixin member the experimental invalidation logic would only
|
||||
// invalidate that library. This enables a search for applications of the
|
||||
// mixin in other libraries adds them to the invalidated set.
|
||||
@override
|
||||
bool get incrementalCompilerIncludeMixinApplicationInvalidatedLibraries =>
|
||||
true;
|
||||
}
|
||||
|
||||
class DevCompilerDartLibrarySupport extends CustomizedDartLibrarySupport {
|
||||
|
||||
@@ -332,6 +332,10 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
|
||||
reusedResult,
|
||||
c,
|
||||
uriTranslator,
|
||||
context
|
||||
.options
|
||||
.target
|
||||
.incrementalCompilerIncludeMixinApplicationInvalidatedLibraries,
|
||||
);
|
||||
recorderForTesting?.recordRebuildBodiesCount(
|
||||
experimentalInvalidation?.missingSources.length ?? 0,
|
||||
@@ -528,6 +532,14 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
|
||||
c,
|
||||
cleanedUpBuilders: cleanedUpBuilders,
|
||||
);
|
||||
if (experimentalInvalidation != null &&
|
||||
experimentalInvalidation.invalidatedMixinApplicationLibraries !=
|
||||
null) {
|
||||
outputLibraries = {
|
||||
...outputLibraries,
|
||||
...experimentalInvalidation.invalidatedMixinApplicationLibraries!,
|
||||
}.toList();
|
||||
}
|
||||
List<String> problemsAsJson = _componentProblems.reissueProblems(
|
||||
context,
|
||||
currentKernelTarget,
|
||||
@@ -1118,6 +1130,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
|
||||
ReusageResult reusedResult,
|
||||
CompilerContext c,
|
||||
UriTranslator uriTranslator,
|
||||
bool collectMixinsToo,
|
||||
) async {
|
||||
Set<DillLibraryBuilder>? rebuildBodies;
|
||||
Set<DillLibraryBuilder> originalNotReusedLibraries;
|
||||
@@ -1232,6 +1245,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
|
||||
// procedures, if the changed file is used as a mixin anywhere else
|
||||
// we can't only recompile the changed file.
|
||||
// TODO(jensj): Check for mixins in a smarter and faster way.
|
||||
Set<Library>? invalidatedMixinApplicationLibraries;
|
||||
if (!skipExperimentalInvalidationChecksForTesting) {
|
||||
for (LibraryBuilder builder in reusedResult.notReusedLibraries) {
|
||||
if (missingSources!.contains(builder.fileUri)) {
|
||||
@@ -1256,6 +1270,13 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
if (collectMixinsToo &&
|
||||
c.mixedInClass != null &&
|
||||
missingSources.contains(c.mixedInClass!.fileUri)) {
|
||||
(invalidatedMixinApplicationLibraries ??= {}).add(
|
||||
c.enclosingLibrary,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1324,6 +1345,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
|
||||
rebuildBodies,
|
||||
originalNotReusedLibraries,
|
||||
missingSources,
|
||||
invalidatedMixinApplicationLibraries,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -2727,11 +2749,13 @@ class ExperimentalInvalidation {
|
||||
final Set<DillLibraryBuilder> rebuildBodies;
|
||||
final Set<DillLibraryBuilder> originalNotReusedLibraries;
|
||||
final Set<Uri> missingSources;
|
||||
final Set<Library>? invalidatedMixinApplicationLibraries;
|
||||
|
||||
ExperimentalInvalidation(
|
||||
this.rebuildBodies,
|
||||
this.originalNotReusedLibraries,
|
||||
this.missingSources,
|
||||
this.invalidatedMixinApplicationLibraries,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,3 @@
|
||||
import 'main_lib1.dart';
|
||||
|
||||
C c = new C();
|
||||
@@ -0,0 +1,44 @@
|
||||
library;
|
||||
import self as self;
|
||||
import "main_lib1.dart" as mai;
|
||||
|
||||
import "org-dartlang-testcase:///main_lib1.dart";
|
||||
|
||||
static field mai::C c = new mai::C::•();
|
||||
|
||||
library;
|
||||
import self as mai;
|
||||
import "dart:core" as core;
|
||||
import "main_lib2.dart" as mai2;
|
||||
|
||||
import "org-dartlang-testcase:///main_lib2.dart";
|
||||
|
||||
class C extends mai::_C&Object&M {
|
||||
synthetic constructor •() → mai::C
|
||||
: super mai::_C&Object&M::•()
|
||||
;
|
||||
static synthetic method _#new#tearOff() → mai::C
|
||||
return new mai::C::•();
|
||||
}
|
||||
abstract class _C&Object&M = core::Object with mai2::M /*isAnonymousMixin,hasConstConstructor*/ {
|
||||
const synthetic constructor •() → mai::_C&Object&M
|
||||
: super core::Object::•()
|
||||
;
|
||||
synthetic mixin-super-stub method m1() → core::String
|
||||
return super.{mai2::M::m1}();
|
||||
synthetic mixin-super-stub method m2() → core::String
|
||||
return super.{mai2::M::m2}();
|
||||
}
|
||||
|
||||
library;
|
||||
import self as mai2;
|
||||
import "dart:core" as core;
|
||||
|
||||
abstract class M extends core::Object /*isMixinDeclaration*/ {
|
||||
method m1() → core::String {
|
||||
return "hello";
|
||||
}
|
||||
method m2() → core::String {
|
||||
return "hello";
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,44 @@
|
||||
library;
|
||||
import self as self;
|
||||
import "main_lib1.dart" as mai;
|
||||
|
||||
import "org-dartlang-testcase:///main_lib1.dart";
|
||||
|
||||
static field mai::C c = new mai::C::•();
|
||||
|
||||
library;
|
||||
import self as mai;
|
||||
import "dart:core" as core;
|
||||
import "main_lib2.dart" as mai2;
|
||||
|
||||
import "org-dartlang-testcase:///main_lib2.dart";
|
||||
|
||||
class C extends mai::_C&Object&M {
|
||||
synthetic constructor •() → mai::C
|
||||
: super mai::_C&Object&M::•()
|
||||
;
|
||||
static synthetic method _#new#tearOff() → mai::C
|
||||
return new mai::C::•();
|
||||
}
|
||||
abstract class _C&Object&M = core::Object with mai2::M /*isAnonymousMixin,hasConstConstructor*/ {
|
||||
const synthetic constructor •() → mai::_C&Object&M
|
||||
: super core::Object::•()
|
||||
;
|
||||
synthetic mixin-super-stub method m1() → core::String
|
||||
return super.{mai2::M::m1}();
|
||||
synthetic mixin-super-stub method m2() → core::String
|
||||
return super.{mai2::M::m2}();
|
||||
}
|
||||
|
||||
library;
|
||||
import self as mai2;
|
||||
import "dart:core" as core;
|
||||
|
||||
abstract class M extends core::Object /*isMixinDeclaration*/ {
|
||||
method m1() → core::String {
|
||||
return "hello";
|
||||
}
|
||||
method m2() → core::String {
|
||||
return "hello";
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,41 @@
|
||||
library;
|
||||
import self as self;
|
||||
import "main_lib1.dart" as mai;
|
||||
|
||||
import "org-dartlang-testcase:///main_lib1.dart";
|
||||
|
||||
static field mai::C c;
|
||||
|
||||
library;
|
||||
import self as mai;
|
||||
import "dart:core" as core;
|
||||
import "main_lib2.dart" as mai2;
|
||||
|
||||
import "org-dartlang-testcase:///main_lib2.dart";
|
||||
|
||||
class C extends mai::_C&Object&M {
|
||||
synthetic constructor •() → mai::C
|
||||
;
|
||||
static synthetic method _#new#tearOff() → mai::C
|
||||
return new mai::C::•();
|
||||
}
|
||||
abstract class _C&Object&M = core::Object with mai2::M /*isAnonymousMixin,hasConstConstructor*/ {
|
||||
const synthetic constructor •() → mai::_C&Object&M
|
||||
: super core::Object::•()
|
||||
;
|
||||
synthetic mixin-super-stub method m1() → core::String
|
||||
return super.{mai2::M::m1}();
|
||||
synthetic mixin-super-stub method m2() → core::String
|
||||
return super.{mai2::M::m2}();
|
||||
}
|
||||
|
||||
library;
|
||||
import self as mai2;
|
||||
import "dart:core" as core;
|
||||
|
||||
abstract class M extends core::Object /*isMixinDeclaration*/ {
|
||||
method m1() → core::String
|
||||
;
|
||||
method m2() → core::String
|
||||
;
|
||||
}
|
||||
@@ -0,0 +1,44 @@
|
||||
library;
|
||||
import self as self;
|
||||
import "main_lib1.dart" as mai;
|
||||
|
||||
import "org-dartlang-testcase:///main_lib1.dart";
|
||||
|
||||
static field mai::C c = new mai::C::•();
|
||||
|
||||
library;
|
||||
import self as mai;
|
||||
import "dart:core" as core;
|
||||
import "main_lib2.dart" as mai2;
|
||||
|
||||
import "org-dartlang-testcase:///main_lib2.dart";
|
||||
|
||||
class C extends mai::_C&Object&M {
|
||||
synthetic constructor •() → mai::C
|
||||
: super mai::_C&Object&M::•()
|
||||
;
|
||||
static synthetic method _#new#tearOff() → mai::C
|
||||
return new mai::C::•();
|
||||
}
|
||||
abstract class _C&Object&M = core::Object with mai2::M /*isAnonymousMixin,hasConstConstructor*/ {
|
||||
const synthetic constructor •() → mai::_C&Object&M
|
||||
: super core::Object::•()
|
||||
;
|
||||
synthetic mixin-super-stub method m1() → core::String
|
||||
return super.{mai2::M::m1}();
|
||||
synthetic mixin-super-stub method m2() → core::String
|
||||
return super.{mai2::M::m2}();
|
||||
}
|
||||
|
||||
library;
|
||||
import self as mai2;
|
||||
import "dart:core" as core;
|
||||
|
||||
abstract class M extends core::Object /*isMixinDeclaration*/ {
|
||||
method m1() → core::String {
|
||||
return "hello";
|
||||
}
|
||||
method m2() → core::String {
|
||||
return "hello";
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,3 @@
|
||||
import 'main_lib1.dart';
|
||||
|
||||
C c = new C();
|
||||
+3
@@ -0,0 +1,3 @@
|
||||
import 'main_lib1.dart';
|
||||
|
||||
C c = new C();
|
||||
@@ -0,0 +1,3 @@
|
||||
import 'main_lib2.dart';
|
||||
|
||||
class C with M {}
|
||||
@@ -0,0 +1,9 @@
|
||||
mixin M {
|
||||
String m1() {
|
||||
return 'hello';
|
||||
}
|
||||
|
||||
String m2() {
|
||||
return 'hello';
|
||||
}
|
||||
}
|
||||
@@ -2783,6 +2783,181 @@ e() {
|
||||
});
|
||||
});
|
||||
|
||||
group('changes to mixin body invalidate libraries that apply the mixin',
|
||||
() {
|
||||
Future<void> runTests(
|
||||
{required String moduleFormat, bool canary = false}) async {
|
||||
new File('${tempDir.path}/main.dart')
|
||||
..createSync()
|
||||
..writeAsStringSync("import 'class2.dart';"
|
||||
"import 'helper.dart';"
|
||||
"final h = new Helper();"
|
||||
"final c = new C2();"
|
||||
"main() {"
|
||||
" print(c.fn1());"
|
||||
" print(c.gn1());"
|
||||
" print(c.fn2());"
|
||||
" print(c.gn2());"
|
||||
" print(h.x());"
|
||||
"}");
|
||||
new File('${tempDir.path}/helper.dart')
|
||||
..createSync()
|
||||
..writeAsStringSync(
|
||||
"class Helper { String x() { return 'Helper'; } }");
|
||||
new File('${tempDir.path}/class2.dart')
|
||||
..createSync()
|
||||
..writeAsStringSync("import 'mixin2.dart';\n"
|
||||
"import 'class1.dart';\n"
|
||||
"class C2 extends C1 with M2 {}\n");
|
||||
new File('${tempDir.path}/class1.dart')
|
||||
..createSync()
|
||||
..writeAsStringSync("import 'mixin1.dart';\n"
|
||||
"class C1 with M1 {}\n");
|
||||
new File('${tempDir.path}/mixin1.dart')
|
||||
..createSync()
|
||||
..writeAsStringSync("mixin M1 {\n"
|
||||
" String fn1() { return 'hello'; }\n"
|
||||
" String gn1() => 'hello';\n"
|
||||
"}\n");
|
||||
new File('${tempDir.path}/mixin2.dart')
|
||||
..createSync()
|
||||
..writeAsStringSync("mixin M2 {\n"
|
||||
" String fn2() { return 'hello'; }\n"
|
||||
" String gn2() => 'hello';\n"
|
||||
"}\n");
|
||||
File packageConfig =
|
||||
new File('${tempDir.path}/.dart_tool/package_config.json')
|
||||
..createSync(recursive: true)
|
||||
..writeAsStringSync('{\n'
|
||||
' "configVersion": 2,\n'
|
||||
' "packages": [\n'
|
||||
' {\n'
|
||||
' "name": "hello",\n'
|
||||
' "rootUri": "../",\n'
|
||||
' "packageUri": "./"\n'
|
||||
' }\n'
|
||||
' ]\n'
|
||||
'}\n');
|
||||
final String entrypoint = 'package:hello/main.dart';
|
||||
final File dillFile = new File('${tempDir.path}/out.dill');
|
||||
final List<String> args = <String>[
|
||||
'--sdk-root=${sdkRoot.toFilePath()}',
|
||||
'--incremental',
|
||||
'--platform=${ddcPlatformKernel.path}',
|
||||
'--output-dill=${dillFile.path}',
|
||||
'--target=dartdevc',
|
||||
'--dartdevc-module-format=$moduleFormat',
|
||||
if (canary) '--dartdevc-canary',
|
||||
'--packages=${packageConfig.path}',
|
||||
];
|
||||
final FrontendServer frontendServer = new FrontendServer();
|
||||
final Future<int> result = frontendServer.open(args);
|
||||
frontendServer.compile(entrypoint);
|
||||
int count = 0;
|
||||
final Completer<bool> expectationCompleter = new Completer<bool>();
|
||||
frontendServer.listen((Result compiledResult) {
|
||||
if (count == 0) {
|
||||
compiledResult.expectNoErrors(filename: dillFile.path);
|
||||
// Should find all files in the output for the initial compile.
|
||||
final File manifestFile = new File('${dillFile.path}.json');
|
||||
expect(manifestFile.existsSync(), true);
|
||||
frontendServer.accept();
|
||||
count++;
|
||||
final Map<String, dynamic> manifest =
|
||||
json.decode(utf8.decode(manifestFile.readAsBytesSync()));
|
||||
expect(
|
||||
manifest.keys,
|
||||
unorderedEquals([
|
||||
'/packages/hello/main.dart.lib.js',
|
||||
'/packages/hello/helper.dart.lib.js',
|
||||
'/packages/hello/class1.dart.lib.js',
|
||||
'/packages/hello/mixin1.dart.lib.js',
|
||||
'/packages/hello/class2.dart.lib.js',
|
||||
'/packages/hello/mixin2.dart.lib.js',
|
||||
]));
|
||||
// Modify the body expression of an arrow function in the M1 mixin.
|
||||
final File mixinFile = new File('${tempDir.path}/mixin1.dart')
|
||||
..createSync()
|
||||
..writeAsStringSync("mixin M1 {\n"
|
||||
" String fn1() { return 'hello'; }\n"
|
||||
" String gn1() => 'goodbye';\n"
|
||||
"}\n");
|
||||
|
||||
frontendServer.recompile(mixinFile.uri, entryPoint: entrypoint);
|
||||
} else if (count == 1) {
|
||||
final File dillIncFile =
|
||||
new File('${dillFile.path}.incremental.dill');
|
||||
compiledResult.expectNoErrors(filename: dillIncFile.path);
|
||||
frontendServer.accept();
|
||||
count++;
|
||||
// Find four output files have been invalidated corresponding to the
|
||||
// library that contains the modified mixin and the reverse imports
|
||||
// back to the main entrypoint.
|
||||
//
|
||||
// This is a side effect of the arrow function body expressions
|
||||
// being present in the incremental compilers "textual outline" so
|
||||
// changes to the body will invalidate the "public" API. This is
|
||||
// more than DDC needs for correctness.
|
||||
final File manifestFile = new File('${dillIncFile.path}.json');
|
||||
final Map<String, dynamic> manifest =
|
||||
json.decode(utf8.decode(manifestFile.readAsBytesSync()));
|
||||
expect(
|
||||
manifest.keys,
|
||||
unorderedEquals([
|
||||
'/packages/hello/main.dart.lib.js',
|
||||
'/packages/hello/class2.dart.lib.js',
|
||||
'/packages/hello/class1.dart.lib.js',
|
||||
'/packages/hello/mixin1.dart.lib.js'
|
||||
]));
|
||||
// Modify an expression in the block body of a method in the mixin
|
||||
// M1.
|
||||
final File mixinFile = new File('${tempDir.path}/mixin1.dart')
|
||||
..createSync()
|
||||
..writeAsStringSync("mixin M1 {\n"
|
||||
" String fn1() { return 'goodbye'; }\n"
|
||||
" String gn1() => 'goodbye';\n"
|
||||
"}\n");
|
||||
|
||||
frontendServer.recompile(mixinFile.uri, entryPoint: entrypoint);
|
||||
} else if (count == 2) {
|
||||
final File dillIncFile =
|
||||
new File('${dillFile.path}.incremental.dill');
|
||||
compiledResult.expectNoErrors(filename: dillIncFile.path);
|
||||
frontendServer.accept();
|
||||
count++;
|
||||
// Find two output files have been invalidated corresponding to the
|
||||
// library that contains the modified mixin and the library contains
|
||||
// an application of the modified mixin.
|
||||
//
|
||||
// DDC needs to reevaluate the library with the mixin application to
|
||||
// ensure the mixin method body is applied.
|
||||
final File manifestFile = new File('${dillIncFile.path}.json');
|
||||
final Map<String, dynamic> manifest =
|
||||
json.decode(utf8.decode(manifestFile.readAsBytesSync()));
|
||||
expect(
|
||||
manifest.keys,
|
||||
unorderedEquals([
|
||||
'/packages/hello/class1.dart.lib.js',
|
||||
'/packages/hello/mixin1.dart.lib.js'
|
||||
]));
|
||||
frontendServer.quit();
|
||||
expectationCompleter.complete(true);
|
||||
}
|
||||
});
|
||||
expect(await result, 0);
|
||||
await expectationCompleter.future;
|
||||
frontendServer.close();
|
||||
}
|
||||
|
||||
test('AMD module format', () async {
|
||||
await runTests(moduleFormat: 'amd');
|
||||
});
|
||||
|
||||
test('DDC module format and canary', () async {
|
||||
await runTests(moduleFormat: 'ddc', canary: true);
|
||||
});
|
||||
});
|
||||
|
||||
group('compile expression to JavaScript', () {
|
||||
Future<void> runTests(
|
||||
{required String moduleFormat, bool canary = false}) async {
|
||||
|
||||
@@ -555,6 +555,14 @@ abstract class Target {
|
||||
|
||||
/// Should this target-specific pragma be recognized by annotation parsers?
|
||||
bool isSupportedPragma(String pragmaName) => false;
|
||||
|
||||
/// When `true` the incremental compiler will always include libraries that
|
||||
/// apply invalidated mixins in the output of a recompile.
|
||||
///
|
||||
/// They will be included even when only the mixin was edited, and even if the
|
||||
/// invalidation was only within the body of the mixin member.
|
||||
bool get incrementalCompilerIncludeMixinApplicationInvalidatedLibraries =>
|
||||
false;
|
||||
}
|
||||
|
||||
class NoneConstantsBackend extends ConstantsBackend {
|
||||
|
||||
@@ -0,0 +1,7 @@
|
||||
// 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 'mixin.dart';
|
||||
|
||||
class C with M {}
|
||||
@@ -0,0 +1,17 @@
|
||||
// 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';
|
||||
|
||||
import 'class.dart';
|
||||
import 'mixin.dart';
|
||||
|
||||
var c = C();
|
||||
|
||||
Future<void> main() async {
|
||||
Expect.equals('hello', c.fn());
|
||||
await hotReload();
|
||||
Expect.equals('goodbye', c.fn());
|
||||
}
|
||||
@@ -0,0 +1,9 @@
|
||||
// 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.
|
||||
|
||||
mixin M {
|
||||
String fn() {
|
||||
return 'hello';
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,20 @@
|
||||
// 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.
|
||||
|
||||
mixin M {
|
||||
String fn() {
|
||||
return 'goodbye';
|
||||
}
|
||||
}
|
||||
|
||||
/** DIFF **/
|
||||
/*
|
||||
|
||||
mixin M {
|
||||
String fn() {
|
||||
- return 'hello';
|
||||
+ return 'goodbye';
|
||||
}
|
||||
}
|
||||
*/
|
||||
Reference in New Issue
Block a user