[cfe] Compute name spaces for dill directly

This changes the computation of the name space for dill libraries and
declarations to directly filling out the underlying maps.

This is a step towards avoiding computation of lookup results for
conflicting upon lookup. Name spaces from dill can have no conflicts
so the computation is not needed.

The remaining piece is name spaces for exports and prefixes. These
also cannot have conflicts but instead of ambiguous lookups which only
trigger errors on use.

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

Author: johnniwinther

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

@@ -66,10 +66,7 @@ abstract class LookupResult {
   /// instance members if [staticOnly] is `true`, and creates an
   /// [AmbiguousBuilder] for duplicates using [fileUri] and [fileOffset].
   static LookupResult? createProcessedResult(LookupResult? result,
-      {required String name,
-      required Uri fileUri,
-      required int fileOffset,
-      required bool staticOnly}) {
+      {required String name, required Uri fileUri, required int fileOffset}) {
     if (result == null) return null;
     NamedBuilder? getable = result.getable;
     NamedBuilder? setable = result.setable;
@@ -80,26 +77,19 @@ abstract class LookupResult {
         getable = new AmbiguousBuilder(name, getable, fileOffset, fileUri);
         changed = true;
       }
-      if (staticOnly && getable.isDeclarationInstanceMember) {
-        getable = null;
-        changed = true;
-      }
     }
     if (setable != null) {
       if (setable.next != null) {
         // Coverage-ignore-block(suite): Not run.
         setable = new AmbiguousBuilder(name, setable, fileOffset, fileUri);
         changed = true;
       }
-      if (staticOnly && setable.isDeclarationInstanceMember) {
-        setable = null;
-        changed = true;
-      }
     }
     if (!changed) {
       return result;
     }
 
+    // Coverage-ignore(suite): Not run.
     return _fromBuilders(getable, setable, assertNoGetterSetterConflict: true);
   }
 
@@ -155,32 +145,6 @@ abstract class LookupResult {
       }
     }
   }
-
-  static void addNamedBuilder(
-      Map<String, LookupResult> content, String name, NamedBuilder member,
-      {required bool setter}) {
-    LookupResult? existing = content[name];
-    if (existing != null) {
-      if (setter) {
-        assert(existing.getable != null,
-            "No existing getable for $name: $existing, trying to add $member.");
-        content[name] = new GetableSetableResult(existing.getable!, member);
-        return;
-      } else {
-        assert(existing.setable != null,
-            "No existing setable for $name: $existing, trying to add $member.");
-        content[name] = new GetableSetableResult(member, existing.setable!);
-        return;
-      }
-    }
-    if (member is LookupResult) {
-      content[name] = member as LookupResult;
-    } else {
-      // Coverage-ignore-block(suite): Not run.
-      content[name] =
-          setter ? new SetableResult(member) : new GetableResult(member);
-    }
-  }
 }
 
 abstract class InvalidLookupResult implements LookupResult {
@@ -275,6 +239,22 @@ class GetableSetableResult with LookupResultMixin implements LookupResult {
   GetableSetableResult(this.getable, this.setable);
 }
 
+class GetableSetableMemberResult
+    with LookupResultMixin
+    implements MemberLookupResult {
+  @override
+  final MemberBuilder getable;
+
+  @override
+  final MemberBuilder setable;
+
+  @override
+  final bool isStatic;
+
+  GetableSetableMemberResult(this.getable, this.setable,
+      {required this.isStatic});
+}
+
 mixin LookupResultMixin implements LookupResult {
   @override
   bool get isInvalidLookup =>

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

@@ -13,14 +13,10 @@ abstract class NameSpace {
   /// Returns the [LookupResult] for the [Builder]s of the given [name] in the
   /// name space.
   ///
-  /// If [staticOnly] is `true`, instance members are not returned.
-  ///
   /// If the [Builder]s are duplicates, an [AmbiguousBuilder] is created for
   /// the access, using the [fileUri] and [fileOffset].
   LookupResult? lookupLocal(String name,
-      {required Uri fileUri,
-      required int fileOffset,
-      required bool staticOnly});
+      {required Uri fileUri, required int fileOffset});
 
   /// Returns the [LookupResult] for the [Builder]s of the given [name] in the
   /// name space.
@@ -57,49 +53,27 @@ abstract class ComputedMutableNameSpace
 }
 
 abstract class DeclarationNameSpace implements NameSpace {
-  MemberLookupResult? lookupConstructor(String name);
-}
-
-abstract class MutableDeclarationNameSpace
-    implements DeclarationNameSpace, MutableNameSpace {
-  void addConstructor(String name, MemberLookupResult builder);
-}
-
-base class NameSpaceImpl implements MutableNameSpace {
-  Map<String, LookupResult>? _content;
-  Set<ExtensionBuilder>? _extensions;
+  final Map<String, MemberLookupResult> _content;
+  final Map<String, MemberLookupResult> _constructors;
 
-  NameSpaceImpl._(
-      {Map<String, LookupResult>? content, Set<ExtensionBuilder>? extensions})
+  DeclarationNameSpace(
+      {required Map<String, MemberLookupResult> content,
+      required Map<String, MemberLookupResult> constructors})
       : _content = content,
-        _extensions = extensions;
+        _constructors = constructors;
 
   @override
-  void addLocalMember(String name, NamedBuilder member,
-      {required bool setter}) {
-    LookupResult.addNamedBuilder(_content ??= {}, name, member, setter: setter);
-  }
+  void forEachLocalExtension(void Function(ExtensionBuilder member) f) {}
 
   @override
-  // Coverage-ignore(suite): Not run.
-  void forEachLocalExtension(void Function(ExtensionBuilder member) f) {
-    _extensions?.forEach(f);
-  }
+  MemberLookupResult? lookupLocal(String name,
+          {required Uri fileUri, required int fileOffset}) =>
+      _content[name];
 
   @override
-  LookupResult? lookupLocal(String name,
-      {required Uri fileUri,
-      required int fileOffset,
-      required bool staticOnly}) {
-    return LookupResult.createProcessedResult(_content?[name],
-        name: name,
-        fileUri: fileUri,
-        fileOffset: fileOffset,
-        staticOnly: staticOnly);
-  }
+  MemberLookupResult? lookupLocalMember(String name) => _content[name];
 
-  @override
-  LookupResult? lookupLocalMember(String name) => _content?[name];
+  MemberLookupResult? lookupConstructor(String name) => _constructors[name];
 }
 
 base class ComputedMutableNameSpaceImpl implements ComputedMutableNameSpace {
@@ -207,31 +181,25 @@ base class ComputedMutableNameSpaceImpl implements ComputedMutableNameSpace {
 
   @override
   LookupResult? lookupLocal(String name,
-      {required Uri fileUri,
-      required int fileOffset,
-      required bool staticOnly}) {
+      {required Uri fileUri, required int fileOffset}) {
     return LookupResult.createProcessedResult(_content?[name],
-        name: name,
-        fileUri: fileUri,
-        fileOffset: fileOffset,
-        staticOnly: staticOnly);
+        name: name, fileUri: fileUri, fileOffset: fileOffset);
   }
 
   @override
   LookupResult? lookupLocalMember(String name) => _content?[name];
 }
 
-final class SourceLibraryNameSpace implements NameSpace {
+final class LibraryNameSpace implements NameSpace {
   final Map<String, LookupResult> _content;
-  Set<ExtensionBuilder>? _extensions;
+  final Set<ExtensionBuilder>? _extensions;
 
-  SourceLibraryNameSpace(
+  LibraryNameSpace(
       {required Map<String, LookupResult> content,
       required Set<ExtensionBuilder> extensions})
       : _content = content,
         _extensions = extensions;
 
-  // Coverage-ignore(suite): Not run.
   void addLocalMember(String name, LookupResult member) {
     assert(!_content.containsKey(name),
         "Unexpected replacement of ${_content[name]} by $member.");
@@ -245,9 +213,7 @@ final class SourceLibraryNameSpace implements NameSpace {
 
   @override
   LookupResult? lookupLocal(String name,
-      {required Uri fileUri,
-      required int fileOffset,
-      required bool staticOnly}) {
+      {required Uri fileUri, required int fileOffset}) {
     return _content[name];
   }
 
@@ -325,37 +291,15 @@ String? areNameSpacesEquivalent(
   return sb.toString();
 }
 
-abstract base class DeclarationNameSpaceBase extends NameSpaceImpl
-    implements DeclarationNameSpace, MutableDeclarationNameSpace {
-  Map<String, MemberLookupResult>? _constructors;
-
-  DeclarationNameSpaceBase._(
-      {super.content,
-      super.extensions,
-      Map<String, MemberLookupResult>? constructors})
-      : _constructors = constructors,
-        super._();
-
-  @override
-  void addConstructor(String name, MemberLookupResult constructor) {
-    (_constructors ??= {})[name] = constructor;
-  }
-
-  @override
-  MemberLookupResult? lookupConstructor(String name) => _constructors?[name];
-}
-
-final class SourceDeclarationNameSpace implements DeclarationNameSpace {
-  final Map<String, MemberLookupResult> _content;
-  final Map<String, MemberLookupResult> _constructors;
-
+final class SourceDeclarationNameSpace extends DeclarationNameSpace {
   SourceDeclarationNameSpace(
-      {required Map<String, MemberLookupResult> content,
-      required Map<String, MemberLookupResult> constructors})
-      : _content = content,
-        _constructors = constructors;
+      {required super.content, required super.constructors});
 
   void addConstructor(String name, MemberLookupResult constructor) {
+    assert(
+        !_constructors.containsKey(name),
+        "Unexpected existing constructor ${_constructors[name]}, "
+        "trying to add ${constructor}.");
     _constructors[name] = constructor;
   }
 
@@ -364,35 +308,11 @@ final class SourceDeclarationNameSpace implements DeclarationNameSpace {
         "Unexpected replacement of ${_content[name]} by $member.");
     _content[name] = member;
   }
-
-  @override
-  void forEachLocalExtension(void Function(ExtensionBuilder member) f) {}
-
-  @override
-  MemberLookupResult? lookupLocal(String name,
-      {required Uri fileUri,
-      required int fileOffset,
-      required bool staticOnly}) {
-    MemberLookupResult? result = _content[name];
-    if (staticOnly && result != null && !result.isStatic) {
-      result = null;
-    }
-    return result;
-  }
-
-  @override
-  LookupResult? lookupLocalMember(String name) => _content[name];
-
-  @override
-  MemberLookupResult? lookupConstructor(String name) => _constructors[name];
-}
-
-final class DillDeclarationNameSpace extends DeclarationNameSpaceBase {
-  DillDeclarationNameSpace() : super._();
 }
 
-final class DillLibraryNameSpace extends ComputedMutableNameSpaceImpl {
-  DillLibraryNameSpace() : super._();
+final class DillDeclarationNameSpace extends DeclarationNameSpace {
+  DillDeclarationNameSpace(
+      {required super.content, required super.constructors});
 }
 
 final class DillExportNameSpace extends ComputedMutableNameSpaceImpl {

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

@@ -141,7 +141,7 @@ abstract class BaseNameSpaceLookupScope implements LookupScope {
   @override
   LookupResult? lookup(String name, int fileOffset, Uri fileUri) {
     return _nameSpace.lookupLocal(name,
-            fileUri: fileUri, fileOffset: fileOffset, staticOnly: false) ??
+            fileUri: fileUri, fileOffset: fileOffset) ??
         _parent?.lookup(name, fileOffset, fileUri);
   }
 

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

@@ -21,15 +21,18 @@ import 'type_builder.dart';
 mixin DeclarationBuilderMixin implements IDeclarationBuilder {
   /// Lookup a static member of this declaration.
   @override
-  LookupResult? findStaticBuilder(String name, int fileOffset, Uri fileUri,
-      LibraryBuilder accessingLibrary) {
+  MemberLookupResult? findStaticBuilder(String name, int fileOffset,
+      Uri fileUri, LibraryBuilder accessingLibrary) {
     if (accessingLibrary.nameOriginBuilder !=
             libraryBuilder.nameOriginBuilder &&
         name.startsWith("_")) {
       return null;
     }
-    return nameSpace.lookupLocal(name,
-        fileUri: fileUri, fileOffset: fileOffset, staticOnly: true);
+    MemberLookupResult? result = nameSpace.lookupLocalMember(name);
+    if (result != null && !result.isStatic) {
+      result = null;
+    }
+    return result;
   }
 
   @override

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

@@ -18,8 +18,8 @@ abstract class IDeclarationBuilder implements ITypeDeclarationBuilder {
   Uri get fileUri;
 
   /// Lookup a member accessed statically through this declaration.
-  LookupResult? findStaticBuilder(String name, int fileOffset, Uri fileUri,
-      LibraryBuilder accessingLibrary);
+  MemberLookupResult? findStaticBuilder(String name, int fileOffset,
+      Uri fileUri, LibraryBuilder accessingLibrary);
 
   MemberLookupResult? findConstructorOrFactory(
       String name, LibraryBuilder accessingLibrary);