[cfe] Add LocalTypeParameterScope

This cleans up the LocalScope interface and the handling of named function expressions.

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

Author: johnniwinther

pkg/_fe_analyzer_shared/lib/src/parser/forwarding_listener.dart

@@ -966,8 +966,9 @@ class ForwardingListener implements Listener {
   }
 
   @override
-  void endFunctionName(Token beginToken, Token token) {
-    listener?.endFunctionName(beginToken, token);
+  void endFunctionName(
+      Token beginToken, Token token, bool isFunctionExpression) {
+    listener?.endFunctionName(beginToken, token, isFunctionExpression);
   }
 
   @override

pkg/_fe_analyzer_shared/lib/src/parser/listener.dart

@@ -812,7 +812,23 @@ class Listener implements UnescapeErrorListener {
 
   void beginFunctionName(Token token) {}
 
-  void endFunctionName(Token beginToken, Token token) {
+  /// The end of the function name in either a local function declaration, like
+  /// 'local' in:
+  ///
+  ///     void m() {
+  ///       void local() {}
+  ///     }
+  ///
+  /// or an erroneous function expression, like 'local' in:
+  ///
+  ///     void m() {
+  ///       var f = void local() {};
+  ///     }
+  ///
+  /// The boolean [isFunctionExpression] indicates that we are in the latter
+  /// case.
+  void endFunctionName(
+      Token beginToken, Token token, bool isFunctionExpression) {
     logEvent("FunctionName");
   }
 

pkg/_fe_analyzer_shared/lib/src/parser/parser_impl.dart

@@ -5222,7 +5222,7 @@ class Parser {
       reportRecoverableError(
           beforeName.next!, codes.messageNamedFunctionExpression);
     }
-    listener.endFunctionName(begin, token);
+    listener.endFunctionName(begin, token, isFunctionExpression);
     token = parseFormalParametersRequiredOpt(formals, MemberKind.Local);
     token = parseInitializersOpt(token);
     token = parseAsyncOptBody(

pkg/analyzer/lib/src/fasta/ast_builder.dart

@@ -2105,7 +2105,8 @@ class AstBuilder extends StackListener {
   }
 
   @override
-  void endFunctionName(Token beginToken, Token token) {
+  void endFunctionName(
+      Token beginToken, Token token, bool isFunctionExpression) {
     debugEvent("FunctionName");
   }
 

pkg/analyzer/test/generated/parser_fasta_listener.dart

@@ -981,9 +981,9 @@ class ForwardingTestListener extends ForwardingListener {
   }
 
   @override
-  void endFunctionName(Token beginToken, Token token) {
+  void endFunctionName(Token beginToken, Token token, bool isFunctionExpression) {
     end('FunctionName');
-    super.endFunctionName(beginToken, token);
+    super.endFunctionName(beginToken, token, isFunctionExpression);
   }
 
   @override

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

@@ -4,6 +4,7 @@
 
 import '../builder/builder.dart';
 import '../builder/declaration_builders.dart';
+import '../builder/variable_builder.dart';
 import 'scope.dart';
 
 abstract class LocalScope implements LookupScope {
@@ -27,9 +28,7 @@ abstract class LocalScope implements LookupScope {
   /// If name was used previously in this scope, this method returns the read
   /// offsets which can be used for reporting a compile-time error about
   /// [name] being used before its declared.
-  List<int>? declare(String name, Builder builder);
-
-  void addLocalVariable(String name, Builder builder);
+  List<int>? declare(String name, VariableBuilder builder);
 
   @override
   Builder? lookupGetable(String name, int charOffset, Uri fileUri);
@@ -117,7 +116,7 @@ final class LocalScopeImpl extends BaseLocalScope
 
   /// Names declared in this scope.
   @override
-  Map<String, Builder>? _local;
+  Map<String, VariableBuilder>? _local;
 
   @override
   Map<String, List<int>>? usedNames;
@@ -128,12 +127,7 @@ final class LocalScopeImpl extends BaseLocalScope
   LocalScopeImpl(this._parent, this.kind, this.classNameOrDebugName);
 
   @override
-  void addLocalVariable(String name, Builder builder) {
-    (_local ??= {})[name] = builder;
-  }
-
-  @override
-  List<int>? declare(String name, Builder builder) {
+  List<int>? declare(String name, VariableBuilder builder) {
     List<int>? previousOffsets = usedNames?[name];
     if (previousOffsets != null && previousOffsets.isNotEmpty) {
       return previousOffsets;
@@ -157,12 +151,7 @@ final class LocalScopeImpl extends BaseLocalScope
 
 mixin ImmutableLocalScopeMixin implements LocalScope {
   @override
-  void addLocalVariable(String name, Builder builder) {
-    throw new UnsupportedError('$runtimeType($kind).addLocalMember');
-  }
-
-  @override
-  List<int>? declare(String name, Builder builder) {
+  List<int>? declare(String name, VariableBuilder builder) {
     throw new UnsupportedError('$runtimeType($kind).declare');
   }
 
@@ -171,6 +160,34 @@ mixin ImmutableLocalScopeMixin implements LocalScope {
   Map<String, List<int>>? get usedNames => null;
 }
 
+final class LocalTypeParameterScope extends BaseLocalScope
+    with LookupScopeMixin, ImmutableLocalScopeMixin, LocalScopeMixin {
+  @override
+  final LocalScope? _parent;
+  @override
+  final ScopeKind kind;
+  @override
+  final Map<String, TypeParameterBuilder>? _local;
+
+  final String _debugName;
+
+  LocalTypeParameterScope(
+      {required this.kind,
+      LocalScope? parent,
+      Map<String, TypeParameterBuilder>? local,
+      required String debugName})
+      : _parent = parent,
+        _local = local,
+        _debugName = debugName;
+
+  @override
+  String get classNameOrDebugName => _debugName;
+
+  @override
+  String toString() =>
+      "$runtimeType(${kind}, $classNameOrDebugName, ${_local?.keys})";
+}
+
 final class FixedLocalScope extends BaseLocalScope
     with LookupScopeMixin, ImmutableLocalScopeMixin, LocalScopeMixin {
   @override

pkg/front_end/lib/src/kernel/body_builder.dart

@@ -5085,41 +5085,49 @@ class BodyBuilder extends StackListenerImpl
   void enterNominalVariablesScope(
       List<NominalParameterBuilder>? nominalVariableBuilders) {
     debugEvent("enterNominalVariableScope");
-    enterLocalScope(_localScope.createNestedScope(
-        debugName: "function-type scope", kind: ScopeKind.typeParameters));
+    Map<String, TypeParameterBuilder> typeParameters = {};
     if (nominalVariableBuilders != null) {
       for (NominalParameterBuilder builder in nominalVariableBuilders) {
         if (builder.isWildcard) continue;
         String name = builder.name;
-        Builder? existing = _localScope.lookupLocalVariable(name);
+        TypeParameterBuilder? existing = typeParameters[name];
         if (existing == null) {
-          _localScope.addLocalVariable(name, builder);
+          typeParameters[name] = builder;
         } else {
           // Coverage-ignore-block(suite): Not run.
           reportDuplicatedDeclaration(existing, name, builder.fileOffset);
         }
       }
     }
+    enterLocalScope(new LocalTypeParameterScope(
+        local: typeParameters,
+        parent: _localScope,
+        debugName: "local function type parameter scope",
+        kind: ScopeKind.typeParameters));
   }
 
   void enterStructuralVariablesScope(
       List<StructuralParameterBuilder>? structuralVariableBuilders) {
     debugEvent("enterStructuralVariableScope");
-    enterLocalScope(_localScope.createNestedScope(
-        debugName: "function-type scope", kind: ScopeKind.typeParameters));
+    Map<String, TypeParameterBuilder> typeParameters = {};
     if (structuralVariableBuilders != null) {
       for (StructuralParameterBuilder builder in structuralVariableBuilders) {
         if (builder.isWildcard) continue;
         String name = builder.name;
-        Builder? existing = _localScope.lookupLocalVariable(name);
+        TypeParameterBuilder? existing = typeParameters[name];
         if (existing == null) {
-          _localScope.addLocalVariable(name, builder);
+          typeParameters[name] = builder;
         } else {
           // Coverage-ignore-block(suite): Not run.
           reportDuplicatedDeclaration(existing, name, builder.fileOffset);
         }
       }
     }
+    enterLocalScope(new LocalTypeParameterScope(
+        local: typeParameters,
+        parent: _localScope,
+        debugName: "function-type scope",
+        kind: ScopeKind.typeParameters));
   }
 
   @override
@@ -7273,7 +7281,8 @@ class BodyBuilder extends StackListenerImpl
   void handleNamedRecordField(Token colon) => handleNamedArgument(colon);
 
   @override
-  void endFunctionName(Token beginToken, Token token) {
+  void endFunctionName(
+      Token beginToken, Token token, bool isFunctionExpression) {
     debugEvent("FunctionName");
     Identifier name = pop() as Identifier;
     Token nameToken = name.token;
@@ -7290,20 +7299,24 @@ class BodyBuilder extends StackListenerImpl
         isLocalFunction: true,
         isWildcard: isWildcard)
       ..fileOffset = name.nameOffset;
-    // TODO(ahe): Why are we looking up in local scope, but declaring in parent
-    // scope?
-    Builder? existing = _localScope.lookupLocalVariable(name.name);
-    if (existing != null) {
-      // Coverage-ignore-block(suite): Not run.
-      reportDuplicatedDeclaration(existing, name.name, name.nameOffset);
-    }
     push(new FunctionDeclarationImpl(
         variable,
         // The real function node is created later.
         dummyFunctionNode)
       ..fileOffset = beginToken.charOffset);
     if (!(libraryFeatures.wildcardVariables.isEnabled && variable.isWildcard)) {
-      declareVariable(variable, _localScopes.previous);
+      // The local scope stack contains a type parameter scope for the local
+      // function on top of the scope for the block in which the local function
+      // declaration occurs. So for a local function declaration, we add the
+      // declaration to the previous scope, i.e. the block scope.
+      //
+      // For a named function expression, a nested scope is created to hold the
+      // name, so that it doesn't pollute the block scope (the named function
+      // expression is erroneous and should introduce the name in the scope) and
+      // we therefore use the current scope in this case.
+      LocalScope scope =
+          isFunctionExpression ? _localScope : _localScopes.previous;
+      declareVariable(variable, scope);
     }
   }
 

pkg/front_end/lib/src/util/parser_ast_helper.dart

@@ -1054,9 +1054,12 @@ abstract class AbstractParserAstListener implements Listener {
   }
 
   @override
-  void endFunctionName(Token beginToken, Token token) {
+  void endFunctionName(
+      Token beginToken, Token token, bool isFunctionExpression) {
     FunctionNameEnd data = new FunctionNameEnd(ParserAstType.END,
-        beginToken: beginToken, token: token);
+        beginToken: beginToken,
+        token: token,
+        isFunctionExpression: isFunctionExpression);
     seen(data);
   }
 
@@ -5373,15 +5376,19 @@ class FunctionNameBegin extends ParserAstNode {
 class FunctionNameEnd extends ParserAstNode {
   final Token beginToken;
   final Token token;
+  final bool isFunctionExpression;
 
   FunctionNameEnd(ParserAstType type,
-      {required this.beginToken, required this.token})
+      {required this.beginToken,
+      required this.token,
+      required this.isFunctionExpression})
       : super("FunctionName", type);
 
   @override
   Map<String, Object?> get deprecatedArguments => {
         "beginToken": beginToken,
         "token": token,
+        "isFunctionExpression": isFunctionExpression,
       };
 
   @override

pkg/front_end/parser_testcases/error_recovery/issue_49116.dart.expect

@@ -818,7 +818,7 @@ beginCompilationUnit(Future)
           handleType(await, null)
           beginFunctionName(foo)
             handleIdentifier(foo, localFunctionDeclaration)
-          endFunctionName(await, ()
+          endFunctionName(await, (, false)
           beginFormalParameters((, MemberKind.Local)
             beginMetadataStar(int)
             endMetadataStar(0)
@@ -855,7 +855,7 @@ beginCompilationUnit(Future)
           handleType(await, null)
           beginFunctionName(bar)
             handleIdentifier(bar, localFunctionDeclaration)
-          endFunctionName(await, ()
+          endFunctionName(await, (, false)
           beginFormalParameters((, MemberKind.Local)
             beginMetadataStar(await)
             endMetadataStar(0)

pkg/front_end/parser_testcases/error_recovery/issue_49116.dart.intertwined.expect

@@ -2019,7 +2019,7 @@ parseUnit(Future)
                     listener: beginFunctionName(foo)
                     ensureIdentifier(await, localFunctionDeclaration)
                       listener: handleIdentifier(foo, localFunctionDeclaration)
-                    listener: endFunctionName(await, ()
+                    listener: endFunctionName(await, (, false)
                     parseFormalParametersRequiredOpt(foo, MemberKind.Local)
                       parseFormalParametersRest((, MemberKind.Local)
                         listener: beginFormalParameters((, MemberKind.Local)
@@ -2096,7 +2096,7 @@ parseUnit(Future)
                     listener: beginFunctionName(bar)
                     ensureIdentifier(await, localFunctionDeclaration)
                       listener: handleIdentifier(bar, localFunctionDeclaration)
-                    listener: endFunctionName(await, ()
+                    listener: endFunctionName(await, (, false)
                     parseFormalParametersRequiredOpt(bar, MemberKind.Local)
                       parseFormalParametersRest((, MemberKind.Local)
                         listener: beginFormalParameters((, MemberKind.Local)