Skip to content

Commit

Permalink
[cfe] Remove CompilationUnit.localMembersNameIterator
Browse files Browse the repository at this point in the history 
The incremental compiler is changed to use the library builder instead of the compilation units to patch up export scopes, leaving
[localMembersNameIterator] unused on [CompilationUnit].

Change-Id: Ic1527d7e2dc62fb66c2db35a85be6d53bf2c5d09
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/383620
Reviewed-by: Jens Johansen <jensj@google.com>
Commit-Queue: Johnni Winther <johnniwinther@google.com>
  • Loading branch information
@johnniwinther
johnniwinther authored and Commit Queue committed Sep 5, 2024
1 parent 454d4a0 commit 573c3a0
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 64 deletions.
85 changes: 34 additions & 51 deletions pkg/front_end/lib/src/base/incremental_compiler.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
?.enterPhase(BenchmarkPhases.incremental_setupInLoop);
currentKernelTarget = _setupNewKernelTarget(c, uriTranslator, hierarchy,
reusedLibraries, experimentalInvalidation, entryPoints);
Map<DillLibraryBuilder, List<CompilationUnit>>? rebuildBodiesMap =
Map<DillLibraryBuilder, CompilationUnit>? rebuildBodiesMap =
_experimentalInvalidationCreateRebuildBodiesBuilders(
currentKernelTarget, experimentalInvalidation, uriTranslator);
entryPoints = currentKernelTarget.setEntryPoints(entryPoints);
Expand Down Expand Up @@ -592,7 +592,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
bool changed = false;
Set<Library> newDillLibraryBuilders = new Set<Library>();
_userBuilders ??= {};
Map<LibraryBuilder, List<CompilationUnit>>? convertedLibraries;
Map<LibraryBuilder, CompilationUnit>? convertedLibraries;
for (SourceLibraryBuilder builder
in nextGoodKernelTarget.loader.sourceLibraryBuilders) {
if (cleanedUpBuilders.contains(builder)) {
Expand All @@ -605,8 +605,8 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
newDillLibraryBuilders.add(builder.library);
changed = true;
if (experimentalInvalidation != null) {
convertedLibraries ??= new Map<LibraryBuilder, List<CompilationUnit>>();
convertedLibraries[builder] = [dillBuilder.mainCompilationUnit];
convertedLibraries ??= new Map<LibraryBuilder, CompilationUnit>();
convertedLibraries[builder] = dillBuilder.mainCompilationUnit;
}
}
nextGoodKernelTarget.loader.clearSourceLibraryBuilders();
Expand Down Expand Up @@ -847,39 +847,39 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
/// Fill in the replacement maps that describe the replacements that need to
/// happen because of experimental invalidation.
void _experimentalInvalidationFillReplacementMaps(
Map<LibraryBuilder, List<CompilationUnit>> rebuildBodiesMap,
Map<LibraryBuilder, CompilationUnit> rebuildBodiesMap,
Map<LibraryBuilder, Map<String, Builder>> replacementMap,
Map<LibraryBuilder, Map<String, Builder>> replacementSettersMap) {
for (MapEntry<LibraryBuilder, List<CompilationUnit>> entry
for (MapEntry<LibraryBuilder, CompilationUnit> entry
in rebuildBodiesMap.entries) {
Map<String, Builder> childReplacementMap = {};
Map<String, Builder> childReplacementSettersMap = {};
List<CompilationUnit> compilationUnits = rebuildBodiesMap[entry.key]!;
CompilationUnit mainCompilationUnit = rebuildBodiesMap[entry.key]!;
replacementMap[entry.key] = childReplacementMap;
replacementSettersMap[entry.key] = childReplacementSettersMap;
for (CompilationUnit compilationUnit in compilationUnits) {
NameIterator iterator = compilationUnit.localMembersNameIterator;
while (iterator.moveNext()) {
Builder childBuilder = iterator.current;
if (childBuilder is SourceExtensionBuilder &&
childBuilder.isUnnamedExtension) {
continue;
}
String name = iterator.name;
Map<String, Builder> map;
if (childBuilder.isSetter) {
map = childReplacementSettersMap;
} else {
map = childReplacementMap;
}
assert(
!map.containsKey(name),
// Coverage-ignore(suite): Not run.
"Unexpected double-entry for $name in "
"${compilationUnit.importUri} (org from ${entry.key.importUri}): "
"$childBuilder and ${map[name]}");
map[name] = childBuilder;
NameIterator iterator =
mainCompilationUnit.libraryBuilder.localMembersNameIterator;
while (iterator.moveNext()) {
Builder childBuilder = iterator.current;
if (childBuilder is SourceExtensionBuilder &&
childBuilder.isUnnamedExtension) {
continue;
}
String name = iterator.name;
Map<String, Builder> map;
if (childBuilder.isSetter) {
map = childReplacementSettersMap;
} else {
map = childReplacementMap;
}
assert(
!map.containsKey(name),
// Coverage-ignore(suite): Not run.
"Unexpected double-entry for $name in "
"${mainCompilationUnit.importUri} "
"(org from ${entry.key.importUri}): "
"$childBuilder and ${map[name]}");
map[name] = childBuilder;
}
}
}
Expand All @@ -888,41 +888,24 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
/// be rebuild special, namely they have to be
/// [currentKernelTarget.loader.read] with references from the original
/// [Library] for things to work.
Map<DillLibraryBuilder, List<CompilationUnit>>
Map<DillLibraryBuilder, CompilationUnit>
_experimentalInvalidationCreateRebuildBodiesBuilders(
IncrementalKernelTarget currentKernelTarget,
ExperimentalInvalidation? experimentalInvalidation,
UriTranslator uriTranslator) {
// Any builder(s) in [rebuildBodies] should be semi-reused: Create source
// compilation units based on the underlying libraries.
// Maps from old library builder to list of new compilation unit(s).
Map<DillLibraryBuilder, List<CompilationUnit>> rebuildBodiesMap =
new Map<DillLibraryBuilder, List<CompilationUnit>>.identity();
Map<DillLibraryBuilder, CompilationUnit> rebuildBodiesMap =
new Map<DillLibraryBuilder, CompilationUnit>.identity();
if (experimentalInvalidation != null) {
for (DillLibraryBuilder library
in experimentalInvalidation.rebuildBodies) {
CompilationUnit newMainCompilationUnit = currentKernelTarget.loader
.readAsEntryPoint(library.importUri,
fileUri: library.fileUri,
referencesFromIndex: new IndexedLibrary(library.library));
List<CompilationUnit> compilationUnits = [newMainCompilationUnit];
rebuildBodiesMap[library] = compilationUnits;
for (LibraryPart part in library.library.parts) {
// We need to pass the reference to make any class, procedure etc
// overwrite correctly, but the library itself should not be
// over written as the library for parts are temporary "fake"
// libraries.
Uri partUri = getPartUri(library.importUri, part);
Uri? fileUri =
uriTranslator.getPartFileUri(library.library.fileUri, part);
CompilationUnit newPartCompilationUnit = currentKernelTarget.loader
.read(partUri, -1,
accessor: newMainCompilationUnit,
fileUri: fileUri,
referencesFromIndex: new IndexedLibrary(library.library),
referenceIsPartOwner: true);
compilationUnits.add(newPartCompilationUnit);
}
rebuildBodiesMap[library] = newMainCompilationUnit;
}
}
return rebuildBodiesMap;
Expand All @@ -933,7 +916,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
/// didn't do anything special.
void _experimentalInvalidationPatchUpScopes(
ExperimentalInvalidation? experimentalInvalidation,
Map<DillLibraryBuilder, List<CompilationUnit>> rebuildBodiesMap) {
Map<DillLibraryBuilder, CompilationUnit> rebuildBodiesMap) {
if (experimentalInvalidation != null) {
// Maps from old library builder to map of new content.
Map<LibraryBuilder, Map<String, Builder>> replacementMap = {};
Expand Down
7 changes: 0 additions & 7 deletions pkg/front_end/lib/src/builder/library_builder.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,6 @@ sealed class CompilationUnit {
void addExporter(CompilationUnit exporter,
List<CombinatorBuilder>? combinators, int charOffset);

/// Returns an iterator of all members (typedefs, classes and members)
/// declared in this library, including duplicate declarations.
///
/// Compared to [localMembersIterator] this also gives access to the name
/// that the builders are mapped to.
NameIterator<Builder> get localMembersNameIterator;

/// Add a problem with a severity determined by the severity of the message.
///
/// If [fileUri] is null, it defaults to `this.fileUri`.
Expand Down
4 changes: 0 additions & 4 deletions pkg/front_end/lib/src/dill/dill_library_builder.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,10 +88,6 @@ class DillCompilationUnitImpl extends DillCompilationUnit {
@override
Loader get loader => _dillLibraryBuilder.loader;

@override
NameIterator<Builder> get localMembersNameIterator =>
_dillLibraryBuilder.localMembersNameIterator;

@override
Null get partOfLibrary => _dillLibraryBuilder.partOfLibrary;

Expand Down
4 changes: 3 additions & 1 deletion pkg/front_end/lib/src/source/source_builder_factory.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,9 @@ class BuilderFactoryImpl implements BuilderFactory, BuilderFactoryResult {
originImportUri: _compilationUnit.originImportUri,
fileUri: newFileUri,
accessor: _compilationUnit,
isPatch: _compilationUnit.isAugmenting);
isPatch: _compilationUnit.isAugmenting,
referencesFromIndex: indexedLibrary,
referenceIsPartOwner: indexedLibrary != null);
_parts.add(new Part(charOffset, compilationUnit));

// TODO(ahe): [metadata] should be stored, evaluated, and added to [part].
Expand Down
1 change: 0 additions & 1 deletion pkg/front_end/lib/src/source/source_compilation_unit.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,6 @@ class SourceCompilationUnitImpl
@override
bool get isSynthetic => accessProblem != null;

@override
NameIterator<Builder> get localMembersNameIterator =>
_sourceLibraryBuilder.localMembersNameIterator;

Expand Down

0 comments on commit 573c3a0

Please sign in to comment.