[cfe] Remove ProblemBuilder and clean up lookup methods

This removes the ProblemBuilder which is no longer needed. With the removal, the `lookupLocal` and `lookupLocalMember` are no longer different and are merged to `lookup`. Furthermore since the `fileUri` is not used and `fileOffset` is only used on `LocalScope`, the signature of `lookup` is reduced to only taking the name, and `lookup` on `LocalScope` takes an optional `fileOffset`.

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

Author: johnniwinther

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

@@ -1689,9 +1689,8 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
 
       Class? cls;
       if (className != null) {
-        Builder? scopeMember = libraryBuilder.libraryNameSpace
-            .lookupLocalMember(className)
-            ?.getable;
+        Builder? scopeMember =
+            libraryBuilder.libraryNameSpace.lookup(className)?.getable;
         if (scopeMember is ClassBuilder) {
           cls = scopeMember.cls;
         } else {
@@ -1706,9 +1705,8 @@ class IncrementalCompiler implements IncrementalKernelGenerator {
         if (indexOfDot >= 0) {
           String beforeDot = methodName.substring(0, indexOfDot);
           String afterDot = methodName.substring(indexOfDot + 1);
-          Builder? builder = libraryBuilder.libraryNameSpace
-              .lookupLocalMember(beforeDot)
-              ?.getable;
+          Builder? builder =
+              libraryBuilder.libraryNameSpace.lookup(beforeDot)?.getable;
           extensionName = beforeDot;
           if (builder is ExtensionBuilder) {
             extension = builder.extension;

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

@@ -11,6 +11,9 @@ abstract class LocalScope implements LookupScope {
   @override
   ScopeKind get kind;
 
+  @override
+  LookupResult? lookup(String name, {int fileOffset = -1});
+
   LocalScope createNestedScope(
       {required String debugName, required ScopeKind kind});
 
@@ -51,17 +54,17 @@ abstract base class BaseLocalScope implements LocalScope {
 }
 
 mixin LocalScopeMixin implements LocalScope {
-  LookupScope? get _parent;
+  LocalScope? get _parent;
 
   Map<String, VariableBuilder>? get _local;
 
   @override
   Iterable<VariableBuilder> get localVariables => _local?.values ?? const [];
 
   @override
-  LookupResult? lookup(String name, int fileOffset, Uri fileUri) {
+  LookupResult? lookup(String name, {int fileOffset = -1}) {
     _recordUse(name, fileOffset);
-    return _local?[name] ?? _parent?.lookup(name, fileOffset, fileUri);
+    return _local?[name] ?? _parent?.lookup(name, fileOffset: fileOffset);
   }
 
   @override
@@ -156,8 +159,8 @@ final class LocalTypeParameterScope extends BaseLocalScope
   Iterable<VariableBuilder> get localVariables => const [];
 
   @override
-  LookupResult? lookup(String name, int fileOffset, Uri fileUri) {
-    return _local?[name] ?? _parent?.lookup(name, fileOffset, fileUri);
+  LookupResult? lookup(String name, {int fileOffset = -1}) {
+    return _local?[name] ?? _parent?.lookup(name, fileOffset: fileOffset);
   }
 
   @override
@@ -201,13 +204,13 @@ final class FixedLocalScope extends BaseLocalScope
 final class FormalParameterScope extends BaseLocalScope
     with ImmutableLocalScopeMixin, LocalScopeMixin {
   @override
-  final LookupScope? _parent;
+  final LocalScope? _parent;
   @override
   final Map<String, VariableBuilder>? _local;
 
   FormalParameterScope(
-      {LookupScope? parent, Map<String, VariableBuilder>? local})
-      : _parent = parent,
+      {required LookupScope parent, Map<String, VariableBuilder>? local})
+      : _parent = new EnclosingLocalScope(parent),
         _local = local;
 
   @override
@@ -232,8 +235,8 @@ final class EnclosingLocalScope extends BaseLocalScope
   Iterable<VariableBuilder> get localVariables => const [];
 
   @override
-  LookupResult? lookup(String name, int fileOffset, Uri fileUri) {
-    return _scope.lookup(name, fileOffset, fileUri);
+  LookupResult? lookup(String name, {int fileOffset = -1}) {
+    return _scope.lookup(name);
   }
 
   @override

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

@@ -9,7 +9,6 @@ import '../builder/declaration_builders.dart';
 import '../builder/member_builder.dart';
 import '../codes/cfe_codes.dart';
 import 'compiler_context.dart';
-import 'scope.dart';
 
 abstract class LookupResult {
   /// The [NamedBuilder] used for reading this entity, if any.
@@ -62,37 +61,6 @@ abstract class LookupResult {
     return new InvalidExpression(text)..fileOffset = fileOffset;
   }
 
-  /// 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(LookupResult? result,
-      {required String name, required Uri fileUri, required int fileOffset}) {
-    if (result == null) return null;
-    NamedBuilder? getable = result.getable;
-    NamedBuilder? setable = result.setable;
-    bool changed = false;
-    if (getable != null) {
-      if (getable.next != null) {
-        // Coverage-ignore-block(suite): Not run.
-        getable = new AmbiguousBuilder(name, getable, fileOffset, fileUri);
-        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 (!changed) {
-      return result;
-    }
-
-    // Coverage-ignore(suite): Not run.
-    return _fromBuilders(getable, setable, assertNoGetterSetterConflict: true);
-  }
-
   static LookupResult? createResult(
       NamedBuilder? getable, NamedBuilder? setable) {
     return _fromBuilders(getable, setable, assertNoGetterSetterConflict: false);

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

@@ -12,18 +12,7 @@ import 'uris.dart';
 abstract class NameSpace {
   /// Returns the [LookupResult] for the [Builder]s of the given [name] in the
   /// name space.
-  ///
-  /// 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});
-
-  /// Returns the [LookupResult] for the [Builder]s of the given [name] in the
-  /// name space.
-  ///
-  /// The returned [LookupResult] contains the [Builder]s directly mapped in the
-  /// name space without any filtering or processed of duplicates.
-  LookupResult? lookupLocalMember(String name);
+  LookupResult? lookup(String name);
 
   void forEachLocalExtension(void Function(ExtensionBuilder member) f);
 }
@@ -66,12 +55,7 @@ abstract class DeclarationNameSpace implements NameSpace {
   void forEachLocalExtension(void Function(ExtensionBuilder member) f) {}
 
   @override
-  MemberLookupResult? lookupLocal(String name,
-          {required Uri fileUri, required int fileOffset}) =>
-      _content[name];
-
-  @override
-  MemberLookupResult? lookupLocalMember(String name) => _content[name];
+  MemberLookupResult? lookup(String name) => _content[name];
 
   MemberLookupResult? lookupConstructor(String name) => _constructors[name];
 }
@@ -180,14 +164,7 @@ base class ComputedMutableNameSpaceImpl implements ComputedMutableNameSpace {
   }
 
   @override
-  LookupResult? lookupLocal(String name,
-      {required Uri fileUri, required int fileOffset}) {
-    return LookupResult.createProcessedResult(_content?[name],
-        name: name, fileUri: fileUri, fileOffset: fileOffset);
-  }
-
-  @override
-  LookupResult? lookupLocalMember(String name) => _content?[name];
+  LookupResult? lookup(String name) => _content?[name];
 }
 
 final class LibraryNameSpace implements NameSpace {
@@ -212,13 +189,7 @@ final class LibraryNameSpace implements NameSpace {
   }
 
   @override
-  LookupResult? lookupLocal(String name,
-      {required Uri fileUri, required int fileOffset}) {
-    return _content[name];
-  }
-
-  @override
-  LookupResult? lookupLocalMember(String name) => _content[name];
+  LookupResult? lookup(String name) => _content[name];
 }
 
 // Coverage-ignore(suite): Not run.
@@ -356,16 +327,16 @@ final class DillExportNameSpace extends ComputedMutableNameSpaceImpl {
         LookupResult? replacementResult;
         if (existingGetter != null && existingSetter != null) {
           if (existingGetter == existingSetter) {
-            replacementResult = replacementNameSpaceMap[existingGetter.parent]!
-                .lookupLocalMember(name);
+            replacementResult =
+                replacementNameSpaceMap[existingGetter.parent]!.lookup(name);
           } else {
             NamedBuilder? replacementGetter =
                 replacementNameSpaceMap[existingGetter.parent]
-                    ?.lookupLocalMember(name)
+                    ?.lookup(name)
                     ?.getable;
             NamedBuilder? replacementSetter =
                 replacementNameSpaceMap[existingSetter.parent]
-                    ?.lookupLocalMember(name)
+                    ?.lookup(name)
                     ?.setable;
             replacementResult = LookupResult.createResult(
                 replacementGetter ?? existingGetter,
@@ -374,14 +345,14 @@ final class DillExportNameSpace extends ComputedMutableNameSpaceImpl {
         } else if (existingGetter != null) {
           replacementResult = LookupResult.createResult(
               replacementNameSpaceMap[existingGetter.parent]
-                  ?.lookupLocalMember(name)
+                  ?.lookup(name)
                   ?.getable,
               null);
         } else if (existingSetter != null) {
           replacementResult = LookupResult.createResult(
               null,
               replacementNameSpaceMap[existingSetter.parent]
-                  ?.lookupLocalMember(name)
+                  ?.lookup(name)
                   ?.setable);
         }
         if (replacementResult != null) {
@@ -411,12 +382,12 @@ final class DillExportNameSpace extends ComputedMutableNameSpaceImpl {
           if (replacementNameSpaceMap
               .containsKey(extensionBuilder.libraryBuilder)) {
             assert(replacementNameSpaceMap[extensionBuilder.libraryBuilder]!
-                    .lookupLocalMember(extensionBuilder.name)!
+                    .lookup(extensionBuilder.name)!
                     .getable !=
                 null);
             extensionsReplacement.add(
                 replacementNameSpaceMap[extensionBuilder.libraryBuilder]!
-                    .lookupLocalMember(extensionBuilder.name)!
+                    .lookup(extensionBuilder.name)!
                     .getable as ExtensionBuilder);
             break;
           } else {

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

@@ -122,7 +122,7 @@ enum ScopeKind {
 
 abstract class LookupScope {
   ScopeKind get kind;
-  LookupResult? lookup(String name, int fileOffset, Uri fileUri);
+  LookupResult? lookup(String name);
   // TODO(johnniwinther): Should this be moved to an outer scope interface?
   void forEachExtension(void Function(ExtensionBuilder) f);
 }
@@ -139,10 +139,8 @@ abstract class BaseNameSpaceLookupScope implements LookupScope {
   LookupScope? get _parent;
 
   @override
-  LookupResult? lookup(String name, int fileOffset, Uri fileUri) {
-    return _nameSpace.lookupLocal(name,
-            fileUri: fileUri, fileOffset: fileOffset) ??
-        _parent?.lookup(name, fileOffset, fileUri);
+  LookupResult? lookup(String name) {
+    return _nameSpace.lookup(name) ?? _parent?.lookup(name);
   }
 
   @override
@@ -174,13 +172,12 @@ abstract class AbstractTypeParameterScope implements LookupScope {
   TypeParameterBuilder? getTypeParameter(String name);
 
   @override
-  // Coverage-ignore(suite): Not run.
   ScopeKind get kind => ScopeKind.typeParameters;
 
   @override
-  LookupResult? lookup(String name, int fileOffset, Uri fileUri) {
+  LookupResult? lookup(String name) {
     LookupResult? result = getTypeParameter(name);
-    return result ?? _parent.lookup(name, fileOffset, fileUri);
+    return result ?? _parent.lookup(name);
   }
 
   @override
@@ -379,48 +376,6 @@ NamedBuilder computeAmbiguousDeclarationForImport(
       errorHasBeenReported: false);
 }
 
-// Coverage-ignore(suite): Not run.
-abstract class ProblemBuilder extends NamedBuilderImpl {
-  @override
-  final String name;
-
-  final NamedBuilder builder;
-
-  @override
-  final int fileOffset;
-
-  @override
-  final Uri fileUri;
-
-  ProblemBuilder(this.name, this.builder, this.fileOffset, this.fileUri);
-
-  Message get message;
-
-  @override
-  String get fullNameForErrors => name;
-}
-
-// Coverage-ignore(suite): Not run.
-class AmbiguousBuilder extends ProblemBuilder {
-  AmbiguousBuilder(
-      String name, NamedBuilder builder, int charOffset, Uri fileUri)
-      : super(name, builder, charOffset, fileUri);
-
-  @override
-  Builder? get parent => null;
-
-  @override
-  Message get message => templateDuplicatedDeclarationUse.withArguments(name);
-
-  NamedBuilder getFirstDeclaration() {
-    NamedBuilder declaration = builder;
-    while (declaration.next != null) {
-      declaration = declaration.next!;
-    }
-    return declaration;
-  }
-}
-
 mixin ErroneousMemberBuilderMixin implements SourceMemberBuilder {
   @override
   // Coverage-ignore(suite): Not run.

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

@@ -28,7 +28,7 @@ mixin DeclarationBuilderMixin implements IDeclarationBuilder {
         name.startsWith("_")) {
       return null;
     }
-    MemberLookupResult? result = nameSpace.lookupLocalMember(name);
+    MemberLookupResult? result = nameSpace.lookup(name);
     if (result != null && !result.isStatic) {
       result = null;
     }
@@ -60,7 +60,7 @@ mixin DeclarationBuilderMixin implements IDeclarationBuilder {
 
   @override
   LookupResult? lookupLocalMember(String name, {bool required = false}) {
-    LookupResult? result = nameSpace.lookupLocalMember(name);
+    LookupResult? result = nameSpace.lookup(name);
     if (required && result == null) {
       internalProblem(
           templateInternalProblemNotFoundIn.withArguments(

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

@@ -39,15 +39,14 @@ class ConstructorReferenceBuilder {
     if (qualifier != null) {
       String prefix = qualifier;
       String middle = typeName.name;
-      declaration = scope.lookup(prefix, charOffset, fileUri)?.getable;
+      declaration = scope.lookup(prefix)?.getable;
       if (declaration is TypeAliasBuilder) {
         TypeAliasBuilder aliasBuilder = declaration;
         declaration = aliasBuilder.unaliasDeclaration(typeArguments);
       }
       if (declaration is PrefixBuilder) {
         PrefixBuilder prefix = declaration;
-        declaration =
-            prefix.lookup(middle, typeName.nameOffset, fileUri)?.getable;
+        declaration = prefix.lookup(middle)?.getable;
       } else if (declaration is DeclarationBuilder) {
         MemberLookupResult? result =
             declaration.findConstructorOrFactory(middle, accessingLibrary);
@@ -57,7 +56,7 @@ class ConstructorReferenceBuilder {
         }
       }
     } else {
-      declaration = scope.lookup(typeName.name, charOffset, fileUri)?.getable;
+      declaration = scope.lookup(typeName.name)?.getable;
       if (declaration is TypeAliasBuilder) {
         TypeAliasBuilder aliasBuilder = declaration;
         declaration = aliasBuilder.unaliasDeclaration(typeArguments);

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

@@ -83,6 +83,7 @@ class InvalidBuilder extends TypeDeclarationBuilderImpl
   }
 
   @override
+  // TODO(johnniwinther): This should probably return `null`.
   MemberBuilder get getable => this;
 
   @override