Skip to content

Commit 476d5cc

Browse files
authored
Refactor PackageGraph._tagReexportsFor and document (#3772)
Just some cleanup, possibly a performance boost: * Remove `lastExportedElement`; unused. * Do not add `null` values to the cycle check set. * Track the cycle check set as a local variable, rather than a field. * Document several related APIs. * Rename from 'reexports' to 'exports'; I think this is just simpler.
1 parent 24658cc commit 476d5cc

File tree

1 file changed

+31
-17
lines changed

1 file changed

+31
-17
lines changed

lib/src/model/package_graph.dart

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ class PackageGraph with CommentReferable, Nameable {
361361
@override
362362
PackageGraph get packageGraph => this;
363363

364-
/// Map of package name to Package.
364+
/// Map of package name to [Package].
365365
///
366366
/// This mapping must be complete before [initializePackageGraph] is called.
367367
final Map<String, Package> packageMap = {};
@@ -497,38 +497,48 @@ class PackageGraph with CommentReferable, Nameable {
497497
Iterable<Package> get _documentedPackages =>
498498
packages.where((p) => p.documentedWhere != DocumentLocation.missing);
499499

500-
/// A mapping of all the [Library]s that export a given [LibraryElement].
500+
/// A mapping from a [LibraryElement] to all of the [Library]s that export it.
501501
Map<LibraryElement, Set<Library>> _libraryExports = {};
502502

503-
/// Prevent cycles from breaking our stack.
504-
Set<(Library, LibraryElement?)> _reexportsTagged = {};
505-
506-
void _tagReexportsFor(
507-
final Library topLevelLibrary, final LibraryElement? libraryElement,
508-
[LibraryExportElement? lastExportedElement]) {
509-
var key = (topLevelLibrary, libraryElement);
510-
if (_reexportsTagged.contains(key)) {
503+
/// Marks [publicLibrary] as a library that exports [libraryElement] and all
504+
/// libraries that [libraryElement] transitively exports.
505+
///
506+
/// [alreadyTagged] is used internall to prevent visiting in cycles.
507+
void _tagExportsFor(
508+
final Library publicLibrary,
509+
final LibraryElement libraryElement, {
510+
Set<(Library, LibraryElement)>? alreadyTagged,
511+
}) {
512+
alreadyTagged ??= {};
513+
var key = (publicLibrary, libraryElement);
514+
if (alreadyTagged.contains(key)) {
511515
return;
512516
}
513-
_reexportsTagged.add(key);
514-
if (libraryElement == null) return;
515-
_libraryExports.putIfAbsent(libraryElement, () => {}).add(topLevelLibrary);
517+
alreadyTagged.add(key);
518+
// Mark that `publicLibrary` exports `libraryElement`.
519+
_libraryExports.putIfAbsent(libraryElement, () => {}).add(publicLibrary);
516520
for (var exportedElement in libraryElement.libraryExports) {
517-
_tagReexportsFor(
518-
topLevelLibrary, exportedElement.exportedLibrary, exportedElement);
521+
var exportedLibrary = exportedElement.exportedLibrary;
522+
if (exportedLibrary != null) {
523+
// Follow the exports down; as `publicLibrary` exports `libraryElement`,
524+
// it also exports each `exportedLibrary`.
525+
_tagExportsFor(publicLibrary, exportedLibrary,
526+
alreadyTagged: alreadyTagged);
527+
}
519528
}
520529
}
521530

522531
int _lastSizeOfAllLibraries = 0;
523532

533+
/// A mapping from a [LibraryElement] to all of the [Library]s that export it,
534+
/// which is created if it is not yet populated.
524535
Map<LibraryElement, Set<Library>> get libraryExports {
525536
// Table must be reset if we're still in the middle of adding libraries.
526537
if (allLibraries.keys.length != _lastSizeOfAllLibraries) {
527538
_lastSizeOfAllLibraries = allLibraries.keys.length;
528539
_libraryExports = {};
529-
_reexportsTagged = {};
530540
for (var library in publicLibraries) {
531-
_tagReexportsFor(library, library.element);
541+
_tagExportsFor(library, library.element);
532542
}
533543
}
534544
return _libraryExports;
@@ -621,11 +631,15 @@ class PackageGraph with CommentReferable, Nameable {
621631
}
622632

623633
@visibleForTesting
634+
635+
/// The set of all libraries (public and implementation) found across all
636+
/// [packages].
624637
late final List<Library> libraries =
625638
packages.expand((p) => p.libraries).toList(growable: false)..sort();
626639

627640
int get libraryCount => libraries.length;
628641

642+
/// The set of public libraries found across all [packages].
629643
late final Set<Library> publicLibraries = () {
630644
assert(allLibrariesAdded);
631645
return libraries.wherePublic.toSet();

0 commit comments

Comments
 (0)