[cfe] Cleanup NameSpace

This remove part of the NameSpace to avoid an overreliance on its implementation. This is done in preparation for hold the name space content in a single map instead of a separate map for getables and setables.

The NamedBuilder interfaces is added to use for Builders that can be mapped in name spaces. This avoids the need for the NameIterator and all the associated methods.

Change-Id: Ia547bdc8ddcb83f47473b51a2059428b352f8916
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/428001
Commit-Queue: Johnni Winther <[email protected]>
Reviewed-by: Chloe Stefantsova <[email protected]>

Author: johnniwinther

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

@@ -31,7 +31,7 @@ class Export {
   /// This set in [SourceLibraryBuilder._addDependencies].
   late final LibraryDependency libraryDependency;
 
-  bool addToExportScope(String name, Builder member) {
+  bool addToExportScope(String name, NamedBuilder member) {
     if (combinators != null) {
       for (CombinatorBuilder combinator in combinators!) {
         if (combinator.isShow && !combinator.names.contains(name)) return false;

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

@@ -6,7 +6,6 @@ import 'package:kernel/ast.dart' show LibraryDependency;
 
 import '../builder/builder.dart';
 import '../builder/library_builder.dart';
-import '../builder/name_iterator.dart';
 import '../builder/prefix_builder.dart';
 import '../source/source_library_builder.dart';
 import 'combinator.dart' show CombinatorBuilder;
@@ -69,11 +68,11 @@ class Import {
   void finalizeImports(SourceCompilationUnit importer) {
     if (nativeImportPath != null) return;
 
-    void Function(String, Builder) add;
+    void Function(String, NamedBuilder) add;
 
     PrefixFragment? prefixFragment = this.prefixFragment;
     if (prefixFragment == null) {
-      add = (String name, Builder builder) {
+      add = (String name, NamedBuilder builder) {
         importer.addImportedBuilderToScope(
             name: name, builder: builder, charOffset: importOffset);
       };
@@ -86,16 +85,16 @@ class Import {
             charOffset: prefixOffset);
       }
 
-      add = (String name, Builder member) {
+      add = (String name, NamedBuilder member) {
         prefixFragment.builder.addToPrefixScope(name, member,
             importOffset: importOffset, prefixOffset: prefixOffset);
       };
     }
-    NameIterator<Builder> iterator = importedLibraryBuilder!.exportNameSpace
-        .filteredNameIterator(includeDuplicates: false);
+    Iterator<NamedBuilder> iterator = importedLibraryBuilder!.exportNameSpace
+        .filteredIterator(includeDuplicates: false);
     while (iterator.moveNext()) {
-      String name = iterator.name;
-      Builder member = iterator.current;
+      NamedBuilder member = iterator.current;
+      String name = member.name;
       bool include = true;
       if (combinators != null) {
         for (CombinatorBuilder combinator in combinators!) {

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

@@ -8,6 +8,7 @@ import 'dart:typed_data';
 
 import 'package:_fe_analyzer_shared/src/scanner/abstract_scanner.dart'
     show ScannerConfiguration;
+import 'package:front_end/src/base/name_space.dart';
 import 'package:kernel/binary/ast_from_binary.dart'
     show
         BinaryBuilderWithMetadata,
@@ -65,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;
+import '../builder/builder.dart' show Builder, NamedBuilder;
 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/name_iterator.dart' show NameIterator;
 import '../builder/property_builder.dart';
 import '../codes/cfe_codes.dart';
 import '../dill/dill_class_builder.dart' show DillClassBuilder;
@@ -99,10 +99,9 @@ import 'compiler_context.dart' show CompilerContext;
 import 'hybrid_file_system.dart' show HybridFileSystem;
 import 'incremental_serializer.dart' show IncrementalSerializer;
 import 'library_graph.dart' show LibraryGraph;
-import 'lookup_result.dart';
 import 'ticker.dart' show Ticker;
 import 'uri_translator.dart' show UriTranslator;
-import 'uris.dart' show dartCore, getPartUri;
+import 'uris.dart' show getPartUri;
 
 final Uri dartFfiUri = Uri.parse("dart:ffi");
 
@@ -594,10 +593,11 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
         /// source builders. Patch that up.
 
         // Maps from old library builder to map of new content.
-        Map<LibraryBuilder, Map<String, Builder>>? replacementMap = {};
+        Map<LibraryBuilder, Map<String, NamedBuilder>>? replacementMap = {};
 
         // Maps from old library builder to map of new content.
-        Map<LibraryBuilder, Map<String, Builder>>? replacementSettersMap = {};
+        Map<LibraryBuilder, Map<String, NamedBuilder>>? replacementSettersMap =
+            {};
 
         _experimentalInvalidationFillReplacementMaps(
             convertedLibraries!, replacementMap, replacementSettersMap);
@@ -609,10 +609,10 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
 
             // Clear cached calculations that points (potential) to now replaced
             // things.
-            for (Builder builder in builder.libraryNameSpace.localMembers) {
-              if (builder is DillClassBuilder) {
-                builder.clearCachedValues();
-              }
+            Iterator<DillClassBuilder> iterator = builder.libraryNameSpace
+                .filteredIterator(includeDuplicates: true);
+            while (iterator.moveNext()) {
+              iterator.current.clearCachedValues();
             }
           }
         }
@@ -652,62 +652,10 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
   // Coverage-ignore(suite): Not run.
   String? _hasEquivalentScopes(SourceLibraryBuilder sourceLibraryBuilder,
       DillLibraryBuilder dillLibraryBuilder) {
-    bool isEquivalent = true;
-    StringBuffer sb = new StringBuffer();
-    sb.writeln('Mismatch on ${sourceLibraryBuilder.importUri}:');
-    sourceLibraryBuilder.exportNameSpace
-        .forEachLocalMember((String name, Builder sourceBuilder) {
-      Builder? dillBuilder =
-          dillLibraryBuilder.exportNameSpace.lookupLocalMember(name)?.getable;
-      if (dillBuilder == null) {
-        if ((name == 'dynamic' || name == 'Never') &&
-            sourceLibraryBuilder.importUri == dartCore) {
-          // The source library builder for dart:core has synthetically
-          // injected builders for `dynamic` and `Never` which do not have
-          // corresponding classes in the AST.
-          return;
-        }
-        sb.writeln('No dill builder for ${name}: $sourceBuilder');
-        isEquivalent = false;
-      }
-    });
-    dillLibraryBuilder.exportNameSpace
-        .forEachLocalMember((String name, Builder dillBuilder) {
-      Builder? sourceBuilder =
-          sourceLibraryBuilder.exportNameSpace.lookupLocalMember(name)?.getable;
-      if (sourceBuilder == null) {
-        sb.writeln('No source builder for ${name}: $dillBuilder');
-        isEquivalent = false;
-      }
-    });
-    sourceLibraryBuilder.exportNameSpace
-        .forEachLocalSetter((String name, Builder sourceBuilder) {
-      Builder? dillBuilder =
-          dillLibraryBuilder.exportNameSpace.lookupLocalMember(name)?.setable;
-      if (dillBuilder == null) {
-        sb.writeln('No dill builder for ${name}=: $sourceBuilder');
-        isEquivalent = false;
-      }
-    });
-    dillLibraryBuilder.exportNameSpace
-        .forEachLocalSetter((String name, Builder dillBuilder) {
-      LookupResult? result =
-          sourceLibraryBuilder.exportNameSpace.lookupLocalMember(name);
-      Builder? sourceBuilder = result?.setable;
-      if (sourceBuilder == null) {
-        sourceBuilder = result?.getable;
-        if (sourceBuilder is PropertyBuilder && sourceBuilder.hasSetter) {
-          // Assignable fields can be lowered into a getter and setter.
-          return;
-        }
-        sb.writeln('No source builder for ${name}=: $dillBuilder');
-        isEquivalent = false;
-      }
-    });
-    if (isEquivalent) {
-      return null;
-    }
-    return sb.toString();
+    return areNameSpacesEquivalent(
+        importUri: sourceLibraryBuilder.importUri,
+        sourceNameSpace: sourceLibraryBuilder.exportNameSpace,
+        dillNameSpace: dillLibraryBuilder.exportNameSpace);
   }
 
   /// Compute which libraries to output and which (previous) errors/warnings we
@@ -819,24 +767,24 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
   /// happen because of experimental invalidation.
   void _experimentalInvalidationFillReplacementMaps(
       Map<LibraryBuilder, CompilationUnit> rebuildBodiesMap,
-      Map<LibraryBuilder, Map<String, Builder>> replacementMap,
-      Map<LibraryBuilder, Map<String, Builder>> replacementSettersMap) {
+      Map<LibraryBuilder, Map<String, NamedBuilder>> replacementMap,
+      Map<LibraryBuilder, Map<String, NamedBuilder>> replacementSettersMap) {
     for (MapEntry<LibraryBuilder, CompilationUnit> entry
         in rebuildBodiesMap.entries) {
-      Map<String, Builder> childReplacementMap = {};
-      Map<String, Builder> childReplacementSettersMap = {};
+      Map<String, NamedBuilder> childReplacementMap = {};
+      Map<String, NamedBuilder> childReplacementSettersMap = {};
       CompilationUnit mainCompilationUnit = rebuildBodiesMap[entry.key]!;
       replacementMap[entry.key] = childReplacementMap;
       replacementSettersMap[entry.key] = childReplacementSettersMap;
-      NameIterator iterator =
-          mainCompilationUnit.libraryBuilder.localMembersNameIterator;
+      Iterator<NamedBuilder> iterator =
+          mainCompilationUnit.libraryBuilder.localMembersIterator;
       while (iterator.moveNext()) {
-        Builder childBuilder = iterator.current;
+        NamedBuilder childBuilder = iterator.current;
         if (childBuilder is SourceExtensionBuilder &&
             childBuilder.isUnnamedExtension) {
           continue;
         }
-        String name = iterator.name;
+        String name = childBuilder.name;
         Map<String, Builder> map;
         if (isMappedAsSetter(childBuilder)) {
           map = childReplacementSettersMap;
@@ -889,10 +837,10 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
       Map<DillLibraryBuilder, CompilationUnit> rebuildBodiesMap) {
     if (experimentalInvalidation != null) {
       // Maps from old library builder to map of new content.
-      Map<LibraryBuilder, Map<String, Builder>> replacementMap = {};
+      Map<LibraryBuilder, Map<String, NamedBuilder>> replacementMap = {};
 
       // Maps from old library builder to map of new content.
-      Map<LibraryBuilder, Map<String, Builder>> replacementSettersMap = {};
+      Map<LibraryBuilder, Map<String, NamedBuilder>> replacementSettersMap = {};
 
       _experimentalInvalidationFillReplacementMaps(
           rebuildBodiesMap, replacementMap, replacementSettersMap);
@@ -961,10 +909,10 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
         if (builder.isBuiltAndMarked) {
           // Clear cached calculations in classes which upon calculation can
           // mark things as needed.
-          for (Builder builder in builder.libraryNameSpace.localMembers) {
-            if (builder is DillClassBuilder) {
-              builder.clearCachedValues();
-            }
+          Iterator<DillClassBuilder> iterator = builder.libraryNameSpace
+              .filteredIterator(includeDuplicates: true);
+          while (iterator.moveNext()) {
+            iterator.current.clearCachedValues();
           }
           builder.isBuiltAndMarked = false;
         }
@@ -1883,7 +1831,7 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
               packageLanguageVersion:
                   new ImplicitLanguageVersion(libraryBuilder.languageVersion),
               loader: lastGoodKernelTarget.loader,
-              nameOrigin: libraryBuilder,
+              resolveInLibrary: libraryBuilder,
               isUnsupported: libraryBuilder.isUnsupported,
               forAugmentationLibrary: false,
               forPatchLibrary: false,
@@ -1897,17 +1845,6 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
 
       SourceLibraryBuilder debugLibrary = debugCompilationUnit.createLibrary();
       debugLibrary.buildNameSpace();
-      libraryBuilder.libraryNameSpace.forEachLocalMember((name, member) {
-        debugLibrary.libraryNameSpace
-            .addLocalMember(name, member, setter: false);
-      });
-      libraryBuilder.libraryNameSpace.forEachLocalSetter((name, member) {
-        debugLibrary.libraryNameSpace
-            .addLocalMember(name, member, setter: true);
-      });
-      libraryBuilder.libraryNameSpace.forEachLocalExtension((member) {
-        debugLibrary.libraryNameSpace.addExtension(member);
-      });
       debugLibrary.buildScopes(lastGoodKernelTarget.loader.coreLibrary);
       _ticker.logMs("Created debug library");
 
@@ -1939,15 +1876,14 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
         debugLibrary.addImportsToScope();
         _ticker.logMs("Added imports");
       }
-      SourceCompilationUnit? orgCompilationUnit = debugCompilationUnit;
       debugCompilationUnit = new SourceCompilationUnitImpl(
           importUri: libraryUri,
           fileUri: debugExprUri,
           originImportUri: libraryUri,
           packageLanguageVersion:
               new ImplicitLanguageVersion(libraryBuilder.languageVersion),
           loader: lastGoodKernelTarget.loader,
-          nameOrigin: libraryBuilder,
+          resolveInLibrary: libraryBuilder,
           parentScope: debugCompilationUnit.compilationUnitScope,
           isUnsupported: libraryBuilder.isUnsupported,
           forAugmentationLibrary: false,
@@ -1962,24 +1898,6 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
 
       debugLibrary = debugCompilationUnit.createLibrary();
 
-      // Copy over the prefix namespace for extensions
-      // (`forEachExtensionInScope`) to be found when imported via prefixes.
-      // TODO(johnniwinther): Do we still need these with the new scope
-      //  structure?
-      orgCompilationUnit.prefixNameSpace.forEachLocalMember((name, member) {
-        debugCompilationUnit.prefixNameSpace
-            .addLocalMember(name, member, setter: false);
-      });
-      // Does a prefix namespace ever have anything but locals?
-      orgCompilationUnit.prefixNameSpace.forEachLocalSetter((name, member) {
-        debugCompilationUnit.prefixNameSpace
-            .addLocalMember(name, member, setter: true);
-      });
-      orgCompilationUnit.prefixNameSpace.forEachLocalExtension((member) {
-        debugCompilationUnit.prefixNameSpace.addExtension(member);
-      });
-      orgCompilationUnit = null;
-
       HybridFileSystem hfs =
           lastGoodKernelTarget.fileSystem as HybridFileSystem;
       MemoryFileSystem fs = hfs.memory;

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

@@ -7,16 +7,17 @@ import '../builder/member_builder.dart';
 import 'scope.dart';
 
 abstract class LookupResult {
-  /// The [Builder] used for reading this entity, if any.
-  Builder? get getable;
+  /// The [NamedBuilder] used for reading this entity, if any.
+  NamedBuilder? get getable;
 
-  /// The [Builder] used for writing to this entity, if any.
-  Builder? get setable;
+  /// The [NamedBuilder] used for writing to this entity, if any.
+  NamedBuilder? get setable;
 
   /// Creates a [LookupResult] for [getable] and [setable] which filters
   /// instance members if [staticOnly] is `true`, and creates an
   /// [AmbiguousBuilder] for duplicates using [fileUri] and [fileOffset].
-  static LookupResult? createProcessedResult(Builder? getable, Builder? setable,
+  static LookupResult? createProcessedResult(
+      NamedBuilder? getable, NamedBuilder? setable,
       {required String name,
       required Uri fileUri,
       required int fileOffset,
@@ -50,11 +51,13 @@ abstract class LookupResult {
     return _fromBuilders(getable, setable, assertNoGetterSetterConflict: true);
   }
 
-  static LookupResult? createResult(Builder? getable, Builder? setable) {
+  static LookupResult? createResult(
+      NamedBuilder? getable, NamedBuilder? setable) {
     return _fromBuilders(getable, setable, assertNoGetterSetterConflict: false);
   }
 
-  static LookupResult? _fromBuilders(Builder? getable, Builder? setable,
+  static LookupResult? _fromBuilders(
+      NamedBuilder? getable, NamedBuilder? setable,
       {required bool assertNoGetterSetterConflict}) {
     if (getable is LookupResult) {
       LookupResult lookupResult = getable as LookupResult;
@@ -98,30 +101,30 @@ abstract class LookupResult {
 
 class GetableResult implements LookupResult {
   @override
-  final Builder getable;
+  final NamedBuilder getable;
 
   GetableResult(this.getable);
 
   @override
-  Builder? get setable => null;
+  NamedBuilder? get setable => null;
 }
 
 class SetableResult implements LookupResult {
   @override
-  final Builder setable;
+  final NamedBuilder setable;
 
   SetableResult(this.setable);
 
   @override
-  Builder? get getable => null;
+  NamedBuilder? get getable => null;
 }
 
 class GetableSetableResult implements LookupResult {
   @override
-  final Builder getable;
+  final NamedBuilder getable;
 
   @override
-  final Builder setable;
+  final NamedBuilder setable;
 
   GetableSetableResult(this.getable, this.setable);
 }