Skip to content

Commit 2fff7d8

Browse files
jensjohaCommit Queue
authored andcommitted
[CFE/VM] Make IncrementalCompilerResult classHierarchy, coreTypes non-nullable; always use latest
This also fixes a potential leak on missing .accept call where using the non-latest coreTypes in the VM causes us to hold on to old libraries. Tested: CI Change-Id: I3a4a25dab83de0d5113b9f08ec745ac181c24b9b Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/411580 Commit-Queue: Jens Johansen <[email protected]> Reviewed-by: Johnni Winther <[email protected]> Reviewed-by: Alexander Markov <[email protected]> Reviewed-by: Nicholas Shahan <[email protected]>
1 parent 9c3e1a1 commit 2fff7d8

File tree

9 files changed

+28
-19
lines changed

9 files changed

+28
-19
lines changed

pkg/dev_compiler/lib/src/command/command.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ Future<CompilerResult> _compile(List<String> args,
349349
incrementalCompilerResult.component,
350350
cachedSdkInput?.component,
351351
doneAdditionalDills,
352-
incrementalCompilerResult.classHierarchy!,
352+
incrementalCompilerResult.classHierarchy,
353353
incrementalCompilerResult.neededDillLibraries);
354354
}
355355
compilerState.options.onDiagnostic = null; // See http://dartbug.com/36983.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ class ExpressionCompilerWorker {
461461
if (errors.isNotEmpty) return null;
462462

463463
var coreTypes = incrementalCompilerResult.coreTypes;
464-
var hierarchy = incrementalCompilerResult.classHierarchy!;
464+
var hierarchy = incrementalCompilerResult.classHierarchy;
465465

466466
var kernel2jsCompiler = ProgramCompiler(
467467
finalComponent,

pkg/dev_compiler/test/expression_compiler/test_compiler.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ class TestExpressionCompiler {
6060
// Initialize DDC.
6161
var moduleName = p.basenameWithoutExtension(output.toFilePath());
6262

63-
var classHierarchy = compilerResult.classHierarchy!;
63+
var classHierarchy = compilerResult.classHierarchy;
6464
var compilerOptions = Options(
6565
replCompile: true,
6666
moduleName: moduleName,

pkg/dev_compiler/test/module_symbols/module_symbols_test_shared.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ class TestCompiler {
3333

3434
// Initialize DDC.
3535
var moduleName = 'foo.dart';
36-
var classHierarchy = compilerResult.classHierarchy!;
36+
var classHierarchy = compilerResult.classHierarchy;
3737
var compilerOptions = Options(
3838
replCompile: true,
3939
moduleName: moduleName,

pkg/front_end/lib/src/api_prototype/incremental_kernel_generator.dart

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,10 +146,12 @@ bool isLegalIdentifier(String identifier) {
146146

147147
class IncrementalCompilerResult {
148148
final Component component;
149-
final ClassHierarchy? classHierarchy;
150-
final CoreTypes? coreTypes;
149+
final ClassHierarchy classHierarchy;
150+
final CoreTypes coreTypes;
151151
final Set<Library>? neededDillLibraries;
152152

153153
IncrementalCompilerResult(this.component,
154-
{this.classHierarchy, this.coreTypes, this.neededDillLibraries});
154+
{required this.classHierarchy,
155+
required this.coreTypes,
156+
this.neededDillLibraries});
155157
}

pkg/front_end/test/ast_nodes_has_to_string_test.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ Future<void> main(List<String> args) async {
2929
/*Uri initializeFrom*/ null, /*bool outlineOnly*/ true);
3030
IncrementalCompilerResult compilerResult = await compiler.computeDelta();
3131
c = compilerResult.component;
32-
classHierarchy = compilerResult.classHierarchy!;
32+
classHierarchy = compilerResult.classHierarchy;
3333
List<Library> libraries = c.libraries
3434
.where((Library lib) =>
3535
(lib.importUri.toString() == "package:kernel/ast.dart"))

pkg/front_end/test/expression_suite.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ class CompileExpression extends Step<List<TestCase>, List<TestCase>, Context> {
417417
IncrementalCompilerResult compilerResult,
418418
Context context) async {
419419
Map<String, DartType>? definitions = createDefinitionsWithTypes(
420-
compilerResult.classHierarchy?.knownLibraries,
420+
compilerResult.classHierarchy.knownLibraries,
421421
test.definitionTypes,
422422
test.definitions);
423423

@@ -428,7 +428,7 @@ class CompileExpression extends Step<List<TestCase>, List<TestCase>, Context> {
428428
}
429429
}
430430
List<TypeParameter>? typeParams = createTypeParametersWithBounds(
431-
compilerResult.classHierarchy?.knownLibraries,
431+
compilerResult.classHierarchy.knownLibraries,
432432
test.typeBounds,
433433
test.typeDefaults,
434434
test.typeDefinitions);
@@ -471,7 +471,7 @@ class CompileExpression extends Step<List<TestCase>, List<TestCase>, Context> {
471471
IncrementalCompiler compilerNoNNBD,
472472
IncrementalCompilerResult compilerResult,
473473
Context context) async {
474-
for (Library lib in compilerResult.classHierarchy!.knownLibraries) {
474+
for (Library lib in compilerResult.classHierarchy.knownLibraries) {
475475
if (!context.fuzzedLibraries.add(lib.importUri)) continue;
476476

477477
for (Member m in lib.members) {

pkg/frontend_server/test/frontend_server_test.dart

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ import 'package:frontend_server/frontend_server.dart';
1717
import 'package:frontend_server/starter.dart';
1818
import 'package:kernel/ast.dart' show Component, Library;
1919
import 'package:kernel/binary/ast_to_binary.dart';
20+
import 'package:kernel/class_hierarchy.dart';
21+
import 'package:kernel/core_types.dart';
2022
import 'package:kernel/kernel.dart' show loadComponentFromBinary;
2123
import 'package:kernel/target/targets.dart';
2224
import 'package:kernel/verifier.dart' show VerificationStage, verifyComponent;
@@ -111,8 +113,12 @@ class _MockedIncrementalCompiler implements IncrementalCompiler {
111113

112114
@override
113115
Future<IncrementalCompilerResult> compile({List<Uri>? entryPoints}) async {
116+
Component component = new Component();
117+
CoreTypes coreTypes = new CoreTypes(component);
118+
ClassHierarchy classHierarchy = new ClassHierarchy(component, coreTypes);
114119
return new Future<IncrementalCompilerResult>.value(
115-
new IncrementalCompilerResult(new Component()));
120+
new IncrementalCompilerResult(component,
121+
coreTypes: coreTypes, classHierarchy: classHierarchy));
116122
}
117123

118124
@override

pkg/vm/lib/incremental_compiler.dart

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,20 +97,19 @@ class IncrementalCompiler {
9797
}
9898

9999
IncrementalCompilerResult _combinePendingDeltas(bool includePlatform) {
100+
assert(_pendingDeltas.isNotEmpty);
100101
Procedure? mainMethod;
101102
NonNullableByDefaultCompiledMode compilationMode =
102103
NonNullableByDefaultCompiledMode.Invalid;
103104
Map<Uri, Library> combined = <Uri, Library>{};
104105
Map<Uri, Source> uriToSource = new Map<Uri, Source>();
105-
ClassHierarchy? classHierarchy;
106-
CoreTypes? coreTypes;
106+
ClassHierarchy classHierarchy = _pendingDeltas.last.classHierarchy;
107+
CoreTypes coreTypes = _pendingDeltas.last.coreTypes;
107108
for (IncrementalCompilerResult deltaResult in _pendingDeltas) {
108109
Component delta = deltaResult.component;
109110
if (delta.mainMethod != null) {
110111
mainMethod = delta.mainMethod;
111112
}
112-
classHierarchy ??= deltaResult.classHierarchy;
113-
coreTypes ??= deltaResult.coreTypes;
114113
compilationMode = delta.mode;
115114
uriToSource.addAll(delta.uriToSource);
116115
for (Library library in delta.libraries) {
@@ -149,6 +148,7 @@ class IncrementalCompiler {
149148
"compilation only; cannot accept",
150149
);
151150
}
151+
if (_pendingDeltas.isEmpty) return;
152152
Map<Uri, Library> combined = <Uri, Library>{};
153153
Map<Uri, Source> uriToSource = <Uri, Source>{};
154154

@@ -249,10 +249,11 @@ class IncrementalCompiler {
249249
String? scriptUri,
250250
bool isStatic,
251251
) {
252-
ClassHierarchy? classHierarchy =
252+
assert(_lastKnownGood != null || _pendingDeltas.isNotEmpty);
253+
ClassHierarchy classHierarchy =
253254
(_lastKnownGood ?? _combinePendingDeltas(false)).classHierarchy;
254255
Map<String, DartType>? completeDefinitions = createDefinitionsWithTypes(
255-
classHierarchy?.knownLibraries,
256+
classHierarchy.knownLibraries,
256257
definitionTypes,
257258
definitions,
258259
);
@@ -269,7 +270,7 @@ class IncrementalCompiler {
269270
}
270271

271272
List<TypeParameter>? typeParameters = createTypeParametersWithBounds(
272-
classHierarchy?.knownLibraries,
273+
classHierarchy.knownLibraries,
273274
typeBounds,
274275
typeDefaults,
275276
typeDefinitions,

0 commit comments

Comments
 (0)