dart-lang/sdk
1
0
Fork 0
Code Issues Pull Requests Actions 3 Packages Projects Releases Wiki Activity

[cfe] Cleanup DillLibrary/ExportNameSpace implementation

Browse Source 
This simplifies the computation of DillExportNameSpace and removes the
need for LazyNameSpace.

Change-Id: I6f29e9a6def47ed8751437b57c6ad7d29fcf494a
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/429940
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>
This commit is contained in:
Johnni Winther
2025-05-21 07:03:53 -07:00
committed by Commit Queue
parent 8ac2279b07
commit dceefd02ca
6 changed files with 102 additions and 191 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/front_end/lib/src/base/incremental_compiler.dart
+14 -60
View File
@@ -66,13 +66,12 @@ import '../api_prototype/incremental_kernel_generator.dart'
import '../api_prototype/lowering_predicates.dart'
show isExtensionThisName, syntheticThisName;
import '../api_prototype/memory_file_system.dart' show MemoryFileSystem;
import '../builder/builder.dart' show Builder, NamedBuilder;
import '../builder/builder.dart' show Builder;
import '../builder/declaration_builders.dart'
show ClassBuilder, ExtensionBuilder, ExtensionTypeDeclarationBuilder;
import '../builder/library_builder.dart'
show CompilationUnit, LibraryBuilder, SourceCompilationUnit;
import '../builder/member_builder.dart' show MemberBuilder;
import '../builder/property_builder.dart';
import '../codes/cfe_codes.dart';
import '../dill/dill_class_builder.dart' show DillClassBuilder;
import '../dill/dill_library_builder.dart' show DillLibraryBuilder;
@@ -82,7 +81,6 @@ import '../kernel/benchmarker.dart' show BenchmarkPhases, Benchmarker;
import '../kernel/hierarchy/hierarchy_builder.dart' show ClassHierarchyBuilder;
import '../kernel/internal_ast.dart' show VariableDeclarationImpl;
import '../kernel/kernel_target.dart' show BuildResult, KernelTarget;
import '../source/source_extension_builder.dart';
import '../source/source_library_builder.dart'
show
ImplicitLanguageVersion,
@@ -592,23 +590,16 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
/// dill library builders might have links (via export scopes) to the
/// source builders. Patch that up.
// Maps from old library builder to map of new content.
Map<LibraryBuilder, Map<String, NamedBuilder>>? replacementMap = {};
// Map from old library builder to name space of new content.
Map<LibraryBuilder, NameSpace>? replacementNameSpaceMap = {};
// Maps from old library builder to map of new content.
Map<LibraryBuilder, Map<String, NamedBuilder>>? replacementSettersMap =
{};
Map<LibraryBuilder, NameSpace> replacementLookupMap = {};
_experimentalInvalidationFillReplacementMaps(convertedLibraries!,
replacementMap, replacementSettersMap, replacementLookupMap);
_experimentalInvalidationFillReplacementMaps(
convertedLibraries!, replacementNameSpaceMap);
for (DillLibraryBuilder builder
in experimentalInvalidation.originalNotReusedLibraries) {
if (builder.isBuilt) {
builder.patchUpExportScope(
replacementMap, replacementSettersMap, replacementLookupMap);
builder.patchUpExportScope(replacementNameSpaceMap);
// Clear cached calculations that points (potential) to now replaced
// things.
@@ -619,8 +610,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
}
}
}
replacementMap = null;
replacementSettersMap = null;
replacementNameSpaceMap = null;
}
}
nextGoodKernelTarget.loader.buildersCreatedWithReferences.clear();
@@ -770,42 +760,12 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
/// happen because of experimental invalidation.
void _experimentalInvalidationFillReplacementMaps(
Map<LibraryBuilder, CompilationUnit> rebuildBodiesMap,
Map<LibraryBuilder, Map<String, NamedBuilder>> replacementMap,
Map<LibraryBuilder, Map<String, NamedBuilder>> replacementSettersMap,
Map<LibraryBuilder, NameSpace> replacementLookupMap) {
Map<LibraryBuilder, NameSpace> replacementNameSpaceMap) {
for (MapEntry<LibraryBuilder, CompilationUnit> entry
in rebuildBodiesMap.entries) {
Map<String, NamedBuilder> childReplacementMap = {};
Map<String, NamedBuilder> childReplacementSettersMap = {};
CompilationUnit mainCompilationUnit = rebuildBodiesMap[entry.key]!;
replacementMap[entry.key] = childReplacementMap;
replacementSettersMap[entry.key] = childReplacementSettersMap;
replacementLookupMap[entry.key] =
replacementNameSpaceMap[entry.key] =
mainCompilationUnit.libraryBuilder.libraryNameSpace;
Iterator<NamedBuilder> iterator =
mainCompilationUnit.libraryBuilder.unfilteredMembersIterator;
while (iterator.moveNext()) {
NamedBuilder childBuilder = iterator.current;
if (childBuilder is SourceExtensionBuilder &&
childBuilder.isUnnamedExtension) {
continue;
}
String name = childBuilder.name;
Map<String, Builder> map;
if (isMappedAsSetter(childBuilder)) {
map = childReplacementSettersMap;
} else {
map = childReplacementMap;
}
assert(
!map.containsKey(name),
"Unexpected double-entry for $name in "
"${mainCompilationUnit.importUri} "
"(org from ${entry.key.importUri}): "
"$childBuilder and ${map[name]}");
map[name] = childBuilder;
}
}
}
@@ -843,23 +803,17 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
ExperimentalInvalidation? experimentalInvalidation,
Map<DillLibraryBuilder, CompilationUnit> rebuildBodiesMap) {
if (experimentalInvalidation != null) {
// Maps from old library builder to map of new content.
Map<LibraryBuilder, Map<String, NamedBuilder>> replacementMap = {};
// Map from old library builder to name space of new content.
Map<LibraryBuilder, NameSpace> replacementNameSpaceMap = {};
// Maps from old library builder to map of new content.
Map<LibraryBuilder, Map<String, NamedBuilder>> replacementSettersMap = {};
Map<LibraryBuilder, NameSpace> replacementLookupMap = {};
_experimentalInvalidationFillReplacementMaps(rebuildBodiesMap,
replacementMap, replacementSettersMap, replacementLookupMap);
_experimentalInvalidationFillReplacementMaps(
rebuildBodiesMap, replacementNameSpaceMap);
for (DillLibraryBuilder builder
in experimentalInvalidation.originalNotReusedLibraries) {
// There's only something to patch up if it was build already.
if (builder.isBuilt) {
builder.patchUpExportScope(
replacementMap, replacementSettersMap, replacementLookupMap);
builder.patchUpExportScope(replacementNameSpaceMap);
}
}
}
pkg/front_end/lib/src/base/name_space.dart
+70 -102
View File
@@ -7,7 +7,6 @@ import '../builder/declaration_builders.dart';
import '../builder/library_builder.dart';
import '../builder/member_builder.dart';
import '../builder/property_builder.dart';
import '../dill/dill_library_builder.dart';
import 'lookup_result.dart';
import 'scope.dart';
import 'uris.dart';
@@ -375,53 +374,17 @@ final class DillDeclarationNameSpace extends DeclarationNameSpaceBase {
DillDeclarationNameSpace() : super._();
}
abstract base class LazyNameSpace extends ComputedMutableNameSpaceImpl {
LazyNameSpace() : super._();
/// Override this method to lazily populate the scope before access.
void ensureNameSpace();
@override
Map<String, LookupResult>? get _content {
ensureNameSpace();
return super._content;
}
@override
Set<ExtensionBuilder>? get _extensions {
ensureNameSpace();
return super._extensions;
}
final class DillLibraryNameSpace extends ComputedMutableNameSpaceImpl {
DillLibraryNameSpace() : super._();
}
final class DillLibraryNameSpace extends LazyNameSpace {
final DillLibraryBuilder _libraryBuilder;
DillLibraryNameSpace(this._libraryBuilder);
@override
void ensureNameSpace() {
_libraryBuilder.ensureLoaded();
}
}
final class DillExportNameSpace extends LazyNameSpace {
final DillLibraryBuilder _libraryBuilder;
DillExportNameSpace(this._libraryBuilder);
@override
void ensureNameSpace() {
_libraryBuilder.ensureLoaded();
}
final class DillExportNameSpace extends ComputedMutableNameSpaceImpl {
DillExportNameSpace() : super._();
/// Patch up the scope, using the two replacement maps to replace builders in
/// scope. The replacement maps from old LibraryBuilder to map, mapping
/// from name to new (replacement) builder.
void patchUpScope(
Map<LibraryBuilder, Map<String, NamedBuilder>> replacementMap,
Map<LibraryBuilder, Map<String, NamedBuilder>> replacementMapSetters,
Map<LibraryBuilder, NameSpace> replacementLookupMap) {
void patchUpScope(Map<LibraryBuilder, NameSpace> replacementNameSpaceMap) {
// In the following we refer to non-setters as 'getters' for brevity.
//
// We have to replace all getters and setters in [_locals] and [_setters]
@@ -435,70 +398,71 @@ final class DillExportNameSpace extends LazyNameSpace {
// For this reason we start by collecting the names of all getters/setters
// that need (some) replacement. Afterwards we go through these names
// handling both getters and setters at the same time.
{
Set<String> replacedNames = {};
_content?.forEach((String name, LookupResult result) {
if (replacementMap.containsKey(result.getable?.parent)) {
replacedNames.add(name);
}
if (replacementMapSetters.containsKey(result.setable?.parent)) {
replacedNames.add(name);
}
});
if (replacedNames.isNotEmpty) {
for (String name in replacedNames) {
// We start be collecting the relation between an existing getter/setter
// and the getter/setter that will replace it. This information is used
// below to handle all the different cases that can occur.
LookupResult existingResult = _content![name]!;
NamedBuilder? existingGetter = existingResult.getable;
NamedBuilder? existingSetter = existingResult.setable;
LookupResult? replacementResult;
if (existingGetter != null && existingSetter != null) {
if (existingGetter == existingSetter) {
replacementResult = replacementLookupMap[existingGetter.parent]!
.lookupLocalMember(name);
} else {
NamedBuilder? replacementGetter =
replacementLookupMap[existingGetter.parent]
?.lookupLocalMember(name)
?.getable;
NamedBuilder? replacementSetter =
replacementLookupMap[existingSetter.parent]
?.lookupLocalMember(name)
?.setable;
replacementResult = LookupResult.createResult(
replacementGetter ?? existingGetter,
replacementSetter ?? existingSetter);
}
} else if (existingGetter != null) {
replacementResult = LookupResult.createResult(
replacementLookupMap[existingGetter.parent]
?.lookupLocalMember(name)
?.getable,
null);
} else if (existingSetter != null) {
replacementResult = LookupResult.createResult(
null,
replacementLookupMap[existingSetter.parent]
?.lookupLocalMember(name)
?.setable);
}
if (replacementResult != null) {
(_content ??= // Coverage-ignore(suite): Not run.
{})[name] = replacementResult;
Set<String> replacedNames = {};
_content?.forEach((String name, LookupResult result) {
if (replacementNameSpaceMap.containsKey(result.getable?.parent)) {
replacedNames.add(name);
}
if (replacementNameSpaceMap.containsKey(result.setable?.parent)) {
replacedNames.add(name);
}
});
if (replacedNames.isNotEmpty) {
for (String name in replacedNames) {
// We start be collecting the relation between an existing getter/setter
// and the getter/setter that will replace it. This information is used
// below to handle all the different cases that can occur.
LookupResult existingResult = _content![name]!;
NamedBuilder? existingGetter = existingResult.getable;
NamedBuilder? existingSetter = existingResult.setable;
LookupResult? replacementResult;
if (existingGetter != null && existingSetter != null) {
if (existingGetter == existingSetter) {
replacementResult = replacementNameSpaceMap[existingGetter.parent]!
.lookupLocalMember(name);
} else {
// Coverage-ignore-block(suite): Not run.
_content?.remove(name);
NamedBuilder? replacementGetter =
replacementNameSpaceMap[existingGetter.parent]
?.lookupLocalMember(name)
?.getable;
NamedBuilder? replacementSetter =
replacementNameSpaceMap[existingSetter.parent]
?.lookupLocalMember(name)
?.setable;
replacementResult = LookupResult.createResult(
replacementGetter ?? existingGetter,
replacementSetter ?? existingSetter);
}
} else if (existingGetter != null) {
replacementResult = LookupResult.createResult(
replacementNameSpaceMap[existingGetter.parent]
?.lookupLocalMember(name)
?.getable,
null);
} else if (existingSetter != null) {
replacementResult = LookupResult.createResult(
null,
replacementNameSpaceMap[existingSetter.parent]
?.lookupLocalMember(name)
?.setable);
}
if (replacementResult != null) {
(_content ??= // Coverage-ignore(suite): Not run.
{})[name] = replacementResult;
} else {
// Coverage-ignore-block(suite): Not run.
_content?.remove(name);
}
}
}
if (_extensions != null) {
// Coverage-ignore-block(suite): Not run.
bool needsPatching = false;
for (ExtensionBuilder extensionBuilder in _extensions!) {
if (replacementMap.containsKey(extensionBuilder.libraryBuilder)) {
if (replacementNameSpaceMap
.containsKey(extensionBuilder.libraryBuilder)) {
needsPatching = true;
break;
}
@@ -507,12 +471,16 @@ final class DillExportNameSpace extends LazyNameSpace {
Set<ExtensionBuilder> extensionsReplacement =
new Set<ExtensionBuilder>();
for (ExtensionBuilder extensionBuilder in _extensions!) {
if (replacementMap.containsKey(extensionBuilder.libraryBuilder)) {
assert(replacementMap[extensionBuilder.libraryBuilder]![
extensionBuilder.name] !=
if (replacementNameSpaceMap
.containsKey(extensionBuilder.libraryBuilder)) {
assert(replacementNameSpaceMap[extensionBuilder.libraryBuilder]!
.lookupLocalMember(extensionBuilder.name)!
.getable !=
null);
extensionsReplacement.add(replacementMap[extensionBuilder
.libraryBuilder]![extensionBuilder.name] as ExtensionBuilder);
extensionsReplacement.add(
replacementNameSpaceMap[extensionBuilder.libraryBuilder]!
.lookupLocalMember(extensionBuilder.name)!
.getable as ExtensionBuilder);
break;
} else {
extensionsReplacement.add(extensionBuilder);
pkg/front_end/lib/src/builder/library_builder.dart
-5
View File
@@ -355,11 +355,6 @@ abstract class LibraryBuilder implements Builder, ProblemReporting {
/// used in conditional imports and `bool.fromEnvironment` constants.
bool get isUnsupported;
/// Returns an iterator of all members (typedefs, classes and members)
/// declared in this library, including duplicate declarations.
// TODO(johnniwinther): Should the only exist on [SourceLibraryBuilder]?
Iterator<NamedBuilder> get unfilteredMembersIterator;
/// [Iterator] for all declarations declared in this library of type [T].
///
/// If [includeDuplicates] is `true`, duplicate declarations are included.
pkg/front_end/lib/src/dill/dill_library_builder.dart
+13 -20
View File
@@ -97,9 +97,9 @@ class DillCompilationUnitImpl extends DillCompilationUnit {
}
class DillLibraryBuilder extends LibraryBuilderImpl {
late final DillLibraryNameSpace _nameSpace;
final DillLibraryNameSpace _nameSpace = new DillLibraryNameSpace();
late final DillExportNameSpace _exportNameSpace;
final DillExportNameSpace _exportNameSpace = new DillExportNameSpace();
@override
final Library library;
@@ -129,16 +129,18 @@ class DillLibraryBuilder extends LibraryBuilderImpl {
final List<NamedBuilder> _memberBuilders = [];
DillLibraryBuilder(this.library, this.loader) : super(library.fileUri) {
_nameSpace = new DillLibraryNameSpace(this);
_exportNameSpace = new DillExportNameSpace(this);
DillLibraryBuilder(this.library, this.loader) : super(library.fileUri);
@override
NameSpace get libraryNameSpace {
ensureLoaded();
return _nameSpace;
}
@override
NameSpace get libraryNameSpace => _nameSpace;
@override
ComputedNameSpace get exportNameSpace => _exportNameSpace;
ComputedNameSpace get exportNameSpace {
ensureLoaded();
return _exportNameSpace;
}
@override
List<Export> get exporters => mainCompilationUnit.exporters;
@@ -481,12 +483,6 @@ class DillLibraryBuilder extends LibraryBuilderImpl {
}
}
@override
Iterator<NamedBuilder> get unfilteredMembersIterator {
ensureLoaded();
return _memberBuilders.iterator;
}
@override
Iterator<T> filteredMembersIterator<T extends NamedBuilder>(
{required bool includeDuplicates}) {
@@ -502,10 +498,7 @@ class DillLibraryBuilder extends LibraryBuilderImpl {
/// builders in the export scope. The replacement maps from old LibraryBuilder
/// to map, mapping from name to new (replacement) builder.
void patchUpExportScope(
Map<LibraryBuilder, Map<String, NamedBuilder>> replacementMap,
Map<LibraryBuilder, Map<String, NamedBuilder>> replacementMapSetters,
Map<LibraryBuilder, NameSpace> replacementLookupMap) {
_exportNameSpace.patchUpScope(
replacementMap, replacementMapSetters, replacementLookupMap);
Map<LibraryBuilder, NameSpace> replacementNameSpaceMap) {
_exportNameSpace.patchUpScope(replacementNameSpaceMap);
}
}
pkg/front_end/lib/src/source/source_library_builder.dart
+2 -1
View File
@@ -473,7 +473,8 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
new FilteredIterator<T>(_memberBuilders.iterator,
includeDuplicates: includeDuplicates);
@override
/// Returns an iterator of all members (typedefs, classes and members)
/// declared in this library, including duplicate declarations.
Iterator<NamedBuilder> get unfilteredMembersIterator {
return _memberBuilders.iterator;
}
pkg/front_end/test/coverage_suite_expected.dart
+3 -3
View File
@@ -150,7 +150,7 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
),
// 100.0%.
"package:front_end/src/base/incremental_compiler.dart": (
hitCount: 834,
hitCount: 813,
missCount: 0,
),
// 100.0%.
@@ -195,7 +195,7 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
),
// 100.0%.
"package:front_end/src/base/name_space.dart": (
hitCount: 175,
hitCount: 163,
missCount: 0,
),
// 100.0%.
@@ -455,7 +455,7 @@ const Map<String, ({int hitCount, int missCount})> _expect = {
),
// 100.0%.
"package:front_end/src/dill/dill_library_builder.dart": (
hitCount: 353,
hitCount: 347,
missCount: 0,
),
// 100.0%.