[cfe] Remove member/constructor iterators from NameSpace

This remove the iterators from the NameSpace interface and instead
collects the list of builders belonging to a library or declaration
directly inside these builders.

Adds ComputedNameSpace for import, export and prefix name spaces. This
allows for replacing a member and has a filteredIterator function. These name spaces are computed from import/exports and therefore need to be iterated through directly and replace builder when two builders collide.

Change-Id: Iec974656c5331498aa1cfca7973300ac6e044994
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428820
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Chloe Stefantsova <cstefantsova@google.com>

Author: johnniwinther

pkg/front_end/lib/src/base/import.dart

@@ -90,8 +90,8 @@ class Import {
             importOffset: importOffset, prefixOffset: prefixOffset);
       };
     }
-    Iterator<NamedBuilder> iterator = importedLibraryBuilder!.exportNameSpace
-        .filteredIterator(includeDuplicates: false);
+    Iterator<NamedBuilder> iterator =
+        importedLibraryBuilder!.exportNameSpace.filteredIterator();
     while (iterator.moveNext()) {
       NamedBuilder member = iterator.current;
       String name = member.name;

pkg/front_end/lib/src/base/name_space.dart

@@ -33,55 +33,42 @@ abstract class NameSpace {
   LookupResult? lookupLocalMember(String name);
 
   void forEachLocalExtension(void Function(ExtensionBuilder member) f);
+}
 
-  /// Returns an iterator of all members and setters mapped in this name space,
-  /// including duplicate members mapped to the same name.
-  Iterator<NamedBuilder> get unfilteredIterator;
+abstract class MutableNameSpace implements NameSpace {
+  void addLocalMember(String name, NamedBuilder member, {required bool setter});
 
+  /// Adds [builder] to the extensions in this name space.
+  void addExtension(ExtensionBuilder builder);
+}
+
+abstract class ComputedNameSpace implements NameSpace {
   /// Returns a filtered iterator of members and setters mapped in this name
   /// space.
   ///
   /// Only members of type [T] are included. If [parent] is provided, on members
-  /// declared in [parent] are included. If [includeDuplicates] is `true`, all
-  /// duplicates of the same name are included, otherwise, only the first
-  /// declared member is included.
-  Iterator<T> filteredIterator<T extends NamedBuilder>(
-      {required bool includeDuplicates});
+  /// declared in [parent] are included. Duplicates are not included.
+  Iterator<T> filteredIterator<T extends NamedBuilder>();
 }
 
-abstract class MutableNameSpace implements NameSpace {
-  factory MutableNameSpace() = NameSpaceImpl._;
-
-  void addLocalMember(String name, NamedBuilder member,
-      {required bool setter, bool allowReplace = false});
+abstract class ComputedMutableNameSpace
+    implements MutableNameSpace, ComputedNameSpace {
+  factory ComputedMutableNameSpace() = NameSpaceImpl._;
 
-  /// Adds [builder] to the extensions in this name space.
-  void addExtension(ExtensionBuilder builder);
+  void replaceLocalMember(String name, NamedBuilder member,
+      {required bool setter});
 }
 
 abstract class DeclarationNameSpace implements NameSpace {
   MemberBuilder? lookupConstructor(String name);
-
-  /// Returns an iterator of all constructors mapped in this scope,
-  /// including duplicate constructors mapped to the same name.
-  Iterator<MemberBuilder> get unfilteredConstructorIterator;
-
-  /// Returns a filtered iterator of constructors mapped in this scope.
-  ///
-  /// Only members of type [T] are included. If [parent] is provided, on members
-  /// declared in [parent] are included. If [includeDuplicates] is `true`, all
-  /// duplicates of the same name are included, otherwise, only the first
-  /// declared member is included.
-  Iterator<T> filteredConstructorIterator<T extends MemberBuilder>(
-      {required bool includeDuplicates});
 }
 
 abstract class MutableDeclarationNameSpace
     implements DeclarationNameSpace, MutableNameSpace {
   void addConstructor(String name, MemberBuilder builder);
 }
 
-base class NameSpaceImpl implements NameSpace, MutableNameSpace {
+base class NameSpaceImpl implements NameSpace, ComputedMutableNameSpace {
   Map<String, NamedBuilder>? _getables;
   Map<String, NamedBuilder>? _setables;
   Set<ExtensionBuilder>? _extensions;
@@ -96,7 +83,12 @@ base class NameSpaceImpl implements NameSpace, MutableNameSpace {
 
   @override
   void addLocalMember(String name, NamedBuilder member,
-      {required bool setter, bool allowReplace = false}) {
+      {required bool setter}) {
+    _addLocalMember(name, member, setter: setter, allowReplace: false);
+  }
+
+  void _addLocalMember(String name, NamedBuilder member,
+      {required bool setter, required bool allowReplace}) {
     if (setter) {
       assert(
           _setables == null ||
@@ -118,16 +110,27 @@ base class NameSpaceImpl implements NameSpace, MutableNameSpace {
     }
   }
 
+  @override
+  void replaceLocalMember(String name, NamedBuilder member,
+      {required bool setter}) {
+    _addLocalMember(name, member, setter: setter, allowReplace: true);
+  }
+
   @override
   void addExtension(ExtensionBuilder builder) {
     (_extensions ??= {}).add(builder);
   }
 
   @override
-  Iterator<T> filteredIterator<T extends NamedBuilder>(
-      {required bool includeDuplicates}) {
-    return new FilteredIterator<T>(unfilteredIterator,
-        includeDuplicates: includeDuplicates);
+  Iterator<T> filteredIterator<T extends NamedBuilder>() {
+    return new FilteredIterator<T>(
+        new ScopeIterator(
+            _getables?.values.iterator,
+            _setables?.values.iterator,
+            _extensions
+                // Coverage-ignore(suite): Not run.
+                ?.iterator),
+        includeDuplicates: false);
   }
 
   @override
@@ -153,12 +156,6 @@ base class NameSpaceImpl implements NameSpace, MutableNameSpace {
   LookupResult? lookupLocalMember(String name) {
     return LookupResult.createResult(_getables?[name], _setables?[name]);
   }
-
-  @override
-  Iterator<NamedBuilder> get unfilteredIterator => new ScopeIterator(
-      _getables?.values.iterator,
-      _setables?.values.iterator,
-      _extensions?.iterator);
 }
 
 final class SourceLibraryNameSpace extends NameSpaceImpl {
@@ -267,19 +264,6 @@ abstract base class DeclarationNameSpaceBase extends NameSpaceImpl
 
   @override
   MemberBuilder? lookupConstructor(String name) => _constructors?[name];
-
-  /// Returns an iterator of all constructors mapped in this scope,
-  /// including duplicate constructors mapped to the same name.
-  @override
-  Iterator<MemberBuilder> get unfilteredConstructorIterator =>
-      new ConstructorNameSpaceIterator(_constructors?.values.iterator);
-
-  @override
-  Iterator<T> filteredConstructorIterator<T extends MemberBuilder>(
-      {required bool includeDuplicates}) {
-    return new FilteredIterator<T>(unfilteredConstructorIterator,
-        includeDuplicates: includeDuplicates);
-  }
 }
 
 final class SourceDeclarationNameSpace extends DeclarationNameSpaceBase {

pkg/front_end/lib/src/base/scope.dart

@@ -9,7 +9,6 @@ import 'package:kernel/type_environment.dart';
 import '../builder/builder.dart';
 import '../builder/declaration_builders.dart';
 import '../builder/library_builder.dart';
-import '../builder/member_builder.dart';
 import '../builder/metadata_builder.dart';
 import '../builder/prefix_builder.dart';
 import '../kernel/hierarchy/class_member.dart' show ClassMember;
@@ -275,7 +274,7 @@ class CompilationUnitScope extends BaseNameSpaceLookupScope {
 /// The scope containing the prefixes imported into a compilation unit.
 class CompilationUnitPrefixScope extends BaseNameSpaceLookupScope {
   @override
-  final NameSpace _nameSpace;
+  final ComputedNameSpace _nameSpace;
 
   @override
   final LookupScope? _parent;
@@ -292,8 +291,7 @@ class CompilationUnitPrefixScope extends BaseNameSpaceLookupScope {
   void forEachExtension(void Function(ExtensionBuilder) f) {
     if (_extensions == null) {
       Set<ExtensionBuilder> extensions = _extensions = {};
-      Iterator<PrefixBuilder> iterator =
-          _nameSpace.filteredIterator(includeDuplicates: false);
+      Iterator<PrefixBuilder> iterator = _nameSpace.filteredIterator();
       while (iterator.moveNext()) {
         iterator.current.forEachExtension((e) {
           extensions.add(e);
@@ -573,6 +571,7 @@ class ScopeIterator implements Iterator<NamedBuilder> {
   bool moveNext() {
     NamedBuilder? next = _current?.next;
     if (next != null) {
+      // Coverage-ignore-block(suite): Not run.
       _current = next;
       return true;
     }
@@ -591,6 +590,7 @@ class ScopeIterator implements Iterator<NamedBuilder> {
       setters = null;
     }
     if (extensions != null) {
+      // Coverage-ignore-block(suite): Not run.
       while (extensions!.moveNext()) {
         Builder extension = extensions!.current;
         // Named extensions have already been included throw [local] so we skip
@@ -614,39 +614,6 @@ class ScopeIterator implements Iterator<NamedBuilder> {
   }
 }
 
-/// Iterator over builders mapped in a [ConstructorNameSpace], including
-/// duplicates for each directly mapped builder.
-class ConstructorNameSpaceIterator implements Iterator<MemberBuilder> {
-  Iterator<MemberBuilder>? _local;
-
-  MemberBuilder? _current;
-
-  ConstructorNameSpaceIterator(this._local);
-
-  @override
-  bool moveNext() {
-    MemberBuilder? next = _current?.next as MemberBuilder?;
-    if (next != null) {
-      _current = next;
-      return true;
-    }
-    if (_local != null) {
-      if (_local!.moveNext()) {
-        _current = _local!.current;
-        return true;
-      }
-      _local = null;
-    }
-    return false;
-  }
-
-  @override
-  MemberBuilder get current {
-    return _current ?? // Coverage-ignore(suite): Not run.
-        (throw new StateError('No element'));
-  }
-}
-
 /// Filtered builder [Iterator].
 class FilteredIterator<T extends NamedBuilder> implements Iterator<T> {
   final Iterator<NamedBuilder> _iterator;

pkg/front_end/lib/src/builder/class_builder.dart

@@ -6,49 +6,7 @@ part of 'declaration_builders.dart';
 
 const Uri? noUri = null;
 
-abstract class ClassMemberAccess {
-  /// [Iterator] for all constructors declared in this class or any of its
-  /// augmentations.
-  ///
-  /// Duplicates and augmenting constructor are _not_ included.
-  ///
-  /// For instance:
-  ///
-  ///     class Class {
-  ///       Class(); // declared, so it is included
-  ///       Class.named(); // declared, so it is included
-  ///       Class.named(); // duplicate, so it is *not* included
-  ///     }
-  ///
-  ///     augment class Class {
-  ///       augment Class(); // augmenting, so it is *not* included
-  ///       Class.extra(); // declared, so it is included
-  ///     }
-  ///
-  Iterator<T> fullConstructorIterator<T extends MemberBuilder>();
-
-  /// [Iterator] for all members declared in this class or any of its
-  /// augmentations.
-  ///
-  /// Duplicates and augmenting members are _not_ included.
-  ///
-  /// For instance:
-  ///
-  ///     class Class {
-  ///       method() {} // Declared, so it is included.
-  ///       method2() {} // Declared, so it is included.
-  ///       method2() {} // Duplicate, so it is *not* included.
-  ///     }
-  ///
-  ///     augment class Class {
-  ///       augment method() {} // Augmenting, so it is *not* included.
-  ///       extra() {} // Declared, so it is included.
-  ///     }
-  ///
-  Iterator<T> fullMemberIterator<T extends NamedBuilder>();
-}
-
-abstract class ClassBuilder implements DeclarationBuilder, ClassMemberAccess {
+abstract class ClassBuilder implements DeclarationBuilder {
   /// The type in the `extends` clause of a class declaration.
   ///
   /// Currently this also holds the synthesized super class for a mixin

pkg/front_end/lib/src/builder/declaration_builder.dart

@@ -43,6 +43,26 @@ abstract class IDeclarationBuilder implements ITypeDeclarationBuilder {
 
   List<DartType> buildAliasedTypeArguments(LibraryBuilder library,
       List<TypeBuilder>? arguments, ClassHierarchyBase? hierarchy);
+
+  /// Returns an iterator of all members declared in this declaration, including
+  /// duplicate declarations.
+  Iterator<MemberBuilder> get unfilteredMembersIterator;
+
+  /// [Iterator] for all members declared in this declaration of type [T].
+  ///
+  /// If [includeDuplicates] is `true`, duplicate declarations are included.
+  Iterator<T> filteredMembersIterator<T extends MemberBuilder>(
+      {required bool includeDuplicates});
+
+  /// Returns an iterator of all constructors declared in this declaration,
+  /// including duplicate declarations.
+  Iterator<MemberBuilder> get unfilteredConstructorsIterator;
+
+  /// [Iterator] for all constructors declared in this declaration of type [T].
+  ///
+  /// If [includeDuplicates] is `true`, duplicate declarations are included.
+  Iterator<T> filteredConstructorsIterator<T extends MemberBuilder>(
+      {required bool includeDuplicates});
 }
 
 abstract class DeclarationBuilderImpl extends TypeDeclarationBuilderImpl

pkg/front_end/lib/src/builder/extension_type_declaration_builder.dart

@@ -4,8 +4,7 @@
 
 part of 'declaration_builders.dart';
 
-abstract class ExtensionTypeDeclarationBuilder
-    implements DeclarationBuilder, ClassMemberAccess {
+abstract class ExtensionTypeDeclarationBuilder implements DeclarationBuilder {
   /// The type of the underlying representation.
   DartType get declaredRepresentationType;
 

pkg/front_end/lib/src/builder/library_builder.dart

@@ -319,7 +319,7 @@ abstract class SourceCompilationUnit
 abstract class LibraryBuilder implements Builder, ProblemReporting {
   NameSpace get libraryNameSpace;
 
-  NameSpace get exportNameSpace;
+  ComputedNameSpace get exportNameSpace;
 
   List<Export> get exporters;
 

pkg/front_end/lib/src/builder/prefix_builder.dart

@@ -22,7 +22,8 @@ class PrefixBuilder extends NamedBuilderImpl implements LookupResult {
   @override
   final String name;
 
-  final MutableNameSpace _prefixNameSpace = new MutableNameSpace();
+  final ComputedMutableNameSpace _prefixNameSpace =
+      new ComputedMutableNameSpace();
 
   late final LookupScope _prefixScope =
       new NameSpaceLookupScope(_prefixNameSpace, ScopeKind.library);
@@ -85,8 +86,7 @@ class PrefixBuilder extends NamedBuilderImpl implements LookupResult {
       NamedBuilder result = computeAmbiguousDeclarationForImport(
           parent, name, existing, member,
           uriOffset: new UriOffset(fileUri, prefixOffset));
-      _prefixNameSpace.addLocalMember(name, result,
-          setter: isSetter, allowReplace: true);
+      _prefixNameSpace.replaceLocalMember(name, result, setter: isSetter);
     } else {
       _prefixNameSpace.addLocalMember(name, member, setter: isSetter);
     }