Fix handling of function type parameters

Closes #30826

Change-Id: I804bff2a204aa210ba59f691c5d680d8a53aadeb
Reviewed-on: https://dart-review.googlesource.com/7553
Reviewed-by: Sigmund Cherem <sigmund@google.com>

pkg/compiler/lib/src/kernel/element_map_impl.dart

@@ -1457,6 +1457,8 @@ class KernelElementEnvironment implements ElementEnvironment {
 /// Visitor that converts kernel dart types into [DartType].
 class DartTypeConverter extends ir.DartTypeVisitor<DartType> {
   final KernelToElementMapBase elementMap;
+  final Set<ir.TypeParameter> currentFunctionTypeParameters =
+      new Set<ir.TypeParameter>();
   bool topLevel = true;
 
   DartTypeConverter(this.elementMap);
@@ -1485,12 +1487,18 @@ class DartTypeConverter extends ir.DartTypeVisitor<DartType> {
 
   @override
   DartType visitTypeParameterType(ir.TypeParameterType node) {
+    if (currentFunctionTypeParameters.contains(node.parameter)) {
+      // TODO(johnniwinther): Map function type parameters to a new
+      // [FunctionTypeParameter] type.
+      return const DynamicType();
+    }
     return new TypeVariableType(elementMap.getTypeVariable(node.parameter));
   }
 
   @override
   DartType visitFunctionType(ir.FunctionType node) {
-    return new FunctionType(
+    currentFunctionTypeParameters.addAll(node.typeParameters);
+    FunctionType type = new FunctionType(
         visitType(node.returnType),
         visitTypes(node.positionalParameters
             .take(node.requiredParameterCount)
@@ -1500,6 +1508,8 @@ class DartTypeConverter extends ir.DartTypeVisitor<DartType> {
             .toList()),
         node.namedParameters.map((n) => n.name).toList(),
         node.namedParameters.map((n) => visitType(n.type)).toList());
+    currentFunctionTypeParameters.removeAll(node.typeParameters);
+    return type;
   }
 
   @override

pkg/compiler/lib/src/resolution/class_hierarchy.dart

@@ -30,6 +30,7 @@ import 'members.dart' show lookupInScope;
 import 'registry.dart' show ResolutionRegistry;
 import 'resolution_common.dart' show CommonResolverVisitor, MappingVisitor;
 import 'scope.dart' show Scope, TypeDeclarationScope;
+import 'type_resolver.dart' show FunctionTypeParameterScope;
 
 /// If `true` compatible mixin applications are shared within a library. This
 /// matches the mixins generated by fasta.
@@ -73,8 +74,9 @@ class TypeDefinitionVisitor extends MappingVisitor<ResolutionDartType> {
 
       TypeVariableElementX variableElement = typeVariable.element;
       if (typeNode.bound != null) {
-        ResolutionDartType boundType = typeResolver
-            .resolveNominalTypeAnnotation(this, typeNode.bound, const []);
+        ResolutionDartType boundType =
+            typeResolver.resolveNominalTypeAnnotation(
+                this, typeNode.bound, const FunctionTypeParameterScope());
         variableElement.boundCache = boundType;
 
         void checkTypeVariableBound() {
@@ -667,7 +669,8 @@ class ClassResolverVisitor extends TypeDefinitionVisitor {
   }
 
   ResolutionDartType resolveNominalType(NominalTypeAnnotation node) {
-    return typeResolver.resolveNominalTypeAnnotation(this, node, const []);
+    return typeResolver.resolveNominalTypeAnnotation(
+        this, node, const FunctionTypeParameterScope());
   }
 
   ResolutionDartType resolveSupertype(

pkg/compiler/lib/src/resolution/members.dart

@@ -52,6 +52,7 @@ import 'resolution_result.dart';
 import 'scope.dart' show BlockScope, MethodScope, Scope;
 import 'send_structure.dart';
 import 'signatures.dart' show SignatureResolver;
+import 'type_resolver.dart' show FunctionTypeParameterScope;
 import 'variables.dart' show VariableDefinitionsVisitor;
 
 /// The state of constants in resolutions.
@@ -580,6 +581,7 @@ class ResolverVisitor extends MappingVisitor<ResolutionResult> {
     function.functionSignature = SignatureResolver.analyze(
         resolution,
         scope,
+        const FunctionTypeParameterScope(),
         node.typeVariables,
         node.parameters,
         node.returnType,
@@ -4132,10 +4134,13 @@ class ResolverVisitor extends MappingVisitor<ResolutionResult> {
   }
 
   ResolutionDartType resolveTypeAnnotation(TypeAnnotation node,
-      {bool malformedIsError: false,
+      {FunctionTypeParameterScope functionTypeParameters:
+          const FunctionTypeParameterScope(),
+      bool malformedIsError: false,
       bool deferredIsMalformed: true,
       bool registerCheckedModeCheck: true}) {
-    ResolutionDartType type = typeResolver.resolveTypeAnnotation(this, node,
+    ResolutionDartType type = typeResolver.resolveTypeAnnotation(
+        this, node, functionTypeParameters,
         malformedIsError: malformedIsError,
         deferredIsMalformed: deferredIsMalformed);
     if (registerCheckedModeCheck) {

pkg/compiler/lib/src/resolution/resolution.dart

@@ -61,6 +61,7 @@ import 'resolution_result.dart';
 import 'signatures.dart';
 import 'tree_elements.dart';
 import 'typedefs.dart';
+import 'type_resolver.dart' show FunctionTypeParameterScope;
 
 class ResolverTask extends CompilerTask {
   final ConstantCompiler constantCompiler;
@@ -1042,6 +1043,7 @@ class ResolverTask extends CompilerTask {
       return measure(() => SignatureResolver.analyze(
           resolution,
           element.enclosingElement.buildScope(),
+          const FunctionTypeParameterScope(),
           node.typeVariables,
           node.parameters,
           node.returnType,

pkg/compiler/lib/src/resolution/signatures.dart

@@ -25,6 +25,7 @@ import 'members.dart' show ResolverVisitor;
 import 'registry.dart' show ResolutionRegistry;
 import 'resolution_common.dart' show MappingVisitor;
 import 'scope.dart' show Scope, TypeVariablesScope;
+import 'type_resolver.dart' show FunctionTypeParameterScope;
 
 /**
  * [SignatureResolver] resolves function signatures.
@@ -35,22 +36,17 @@ class SignatureResolver extends MappingVisitor<FormalElementX> {
   final Scope scope;
   final MessageKind defaultValuesError;
   final bool createRealParameters;
+  final FunctionTypeParameterScope functionTypeParameters;
   List<FormalElement> optionalParameters = const <FormalElement>[];
   int optionalParameterCount = 0;
   bool isOptionalParameter = false;
   bool optionalParametersAreNamed = false;
   VariableDefinitions currentDefinitions;
 
-  SignatureResolver(
-      Resolution resolution,
-      FunctionTypedElement enclosingElement,
-      Scope scope,
-      ResolutionRegistry registry,
-      {this.defaultValuesError,
-      this.createRealParameters})
-      : this.scope = scope,
-        this.enclosingElement = enclosingElement,
-        this.resolver = new ResolverVisitor(
+  SignatureResolver(Resolution resolution, this.enclosingElement, this.scope,
+      this.functionTypeParameters, ResolutionRegistry registry,
+      {this.defaultValuesError, this.createRealParameters})
+      : this.resolver = new ResolverVisitor(
             resolution, enclosingElement, registry,
             scope: scope),
         super(resolution, registry);
@@ -121,7 +117,8 @@ class SignatureResolver extends MappingVisitor<FormalElementX> {
       FunctionSignature functionSignature = SignatureResolver.analyze(
           resolution,
           scope,
-          functionExpression.typeVariables,
+          functionTypeParameters.expand(functionExpression.typeVariables),
+          null, // Don't create type variable types for the type parameters.
           functionExpression.parameters,
           functionExpression.returnType,
           element,
@@ -310,6 +307,7 @@ class SignatureResolver extends MappingVisitor<FormalElementX> {
   static FunctionSignature analyze(
       Resolution resolution,
       Scope scope,
+      FunctionTypeParameterScope functionTypeParameters,
       NodeList typeVariables,
       NodeList formalParameters,
       Node returnNode,
@@ -353,7 +351,7 @@ class SignatureResolver extends MappingVisitor<FormalElementX> {
         createTypeVariables(typeVariables);
     scope = new FunctionSignatureBuildingScope(scope, typeVariableTypes);
     SignatureResolver visitor = new SignatureResolver(
-        resolution, element, scope, registry,
+        resolution, element, scope, functionTypeParameters, registry,
         defaultValuesError: defaultValuesError,
         createRealParameters: createRealParameters);
     List<FormalElement> parameters = const <FormalElement>[];
@@ -493,7 +491,8 @@ class SignatureResolver extends MappingVisitor<FormalElementX> {
 
   ResolutionDartType resolveReturnType(TypeAnnotation annotation) {
     if (annotation == null) return const ResolutionDynamicType();
-    ResolutionDartType result = resolver.resolveTypeAnnotation(annotation);
+    ResolutionDartType result = resolver.resolveTypeAnnotation(annotation,
+        functionTypeParameters: functionTypeParameters);
     if (result == null) {
       return const ResolutionDynamicType();
     }

pkg/compiler/lib/src/resolution/type_resolver.dart

@@ -74,34 +74,24 @@ class TypeResolver {
     return element;
   }
 
-  ResolutionDartType resolveTypeAnnotation(
-      MappingVisitor visitor, TypeAnnotation node,
+  ResolutionDartType resolveTypeAnnotation(MappingVisitor visitor,
+      TypeAnnotation node, FunctionTypeParameterScope functionTypeParameters,
       {bool malformedIsError: false, bool deferredIsMalformed: true}) {
-    return _resolveTypeAnnotation(visitor, node, const [],
+    return _resolveTypeAnnotation(visitor, node, functionTypeParameters,
         malformedIsError: malformedIsError,
         deferredIsMalformed: deferredIsMalformed);
   }
 
-  // TODO(floitsch): the [visibleTypeParameterNames] is a hack to put
-  // type parameters in scope for the nested types.
-  //
-  // For example, in the following example, the generic type "A" would be stored
-  // in `visibleTypeParameterNames`.
-  // `typedef F = Function(List<A> Function<A>(A x))`.
-  //
-  // They are resolved to `dynamic` until dart2js supports generic methods.
   ResolutionDartType _resolveTypeAnnotation(MappingVisitor visitor,
-      TypeAnnotation node, List<List<String>> visibleTypeParameterNames,
+      TypeAnnotation node, FunctionTypeParameterScope functionTypeParameters,
       {bool malformedIsError: false, bool deferredIsMalformed: true}) {
     if (node.asNominalTypeAnnotation() != null) {
-      return resolveNominalTypeAnnotation(
-          visitor, node, visibleTypeParameterNames,
+      return resolveNominalTypeAnnotation(visitor, node, functionTypeParameters,
           malformedIsError: malformedIsError,
           deferredIsMalformed: deferredIsMalformed);
     }
     assert(node.asFunctionTypeAnnotation() != null);
-    return _resolveFunctionTypeAnnotation(
-        visitor, node, visibleTypeParameterNames,
+    return _resolveFunctionTypeAnnotation(visitor, node, functionTypeParameters,
         malformedIsError: malformedIsError,
         deferredIsMalformed: deferredIsMalformed);
   }
@@ -114,10 +104,9 @@ class TypeResolver {
   /// However, it does work with nested generalized function types:
   ///   `foo(int Function(String) x)`.
   _FormalsTypeResolutionResult _resolveFormalTypes(MappingVisitor visitor,
-      NodeList formals, List<List<String>> visibleTypeParameterNames) {
+      NodeList formals, FunctionTypeParameterScope functionTypeParameters) {
     ResolutionDartType resolvePositionalType(VariableDefinitions node) {
-      return _resolveTypeAnnotation(
-          visitor, node.type, visibleTypeParameterNames);
+      return _resolveTypeAnnotation(visitor, node.type, functionTypeParameters);
     }
 
     void fillNamedTypes(NodeList namedFormals, List<String> names,
@@ -141,7 +130,7 @@ class TypeResolver {
         ResolutionDartType type = node.type == null
             ? const ResolutionDynamicType()
             : _resolveTypeAnnotation(
-                visitor, node.type, visibleTypeParameterNames);
+                visitor, node.type, functionTypeParameters);
         names.add(name);
         types.add(type);
       }
@@ -186,26 +175,22 @@ class TypeResolver {
         requiredTypes, orderedTypes, names, namedTypes);
   }
 
-  ResolutionFunctionType _resolveFunctionTypeAnnotation(MappingVisitor visitor,
-      FunctionTypeAnnotation node, List<List<String>> visibleTypeParameterNames,
-      {bool malformedIsError: false, bool deferredIsMalformed: true}) {
-    assert(visibleTypeParameterNames != null);
+  ResolutionFunctionType _resolveFunctionTypeAnnotation(
+      MappingVisitor visitor,
+      FunctionTypeAnnotation node,
+      FunctionTypeParameterScope functionTypeParameters,
+      {bool malformedIsError: false,
+      bool deferredIsMalformed: true}) {
+    assert(functionTypeParameters != null);
 
-    if (node.typeParameters != null) {
-      List<String> newTypeNames = node.typeParameters.map((_node) {
-        TypeVariable node = _node;
-        return node.name.asIdentifier().source;
-      }).toList();
-      visibleTypeParameterNames = visibleTypeParameterNames.toList()
-        ..add(newTypeNames);
-    }
+    functionTypeParameters = functionTypeParameters.expand(node.typeParameters);
 
     ResolutionDartType returnType = node.returnType == null
         ? const ResolutionDynamicType()
         : _resolveTypeAnnotation(
-            visitor, node.returnType, visibleTypeParameterNames);
+            visitor, node.returnType, functionTypeParameters);
     var formalTypes =
-        _resolveFormalTypes(visitor, node.formals, visibleTypeParameterNames);
+        _resolveFormalTypes(visitor, node.formals, functionTypeParameters);
     var result = new ResolutionFunctionType.generalized(
         returnType,
         formalTypes.requiredTypes,
@@ -213,12 +198,18 @@ class TypeResolver {
         formalTypes.names,
         formalTypes.nameTypes);
     visitor.registry.useType(node, result);
+
+    functionTypeParameters = functionTypeParameters.parent;
+
     return result;
   }
 
-  ResolutionDartType resolveNominalTypeAnnotation(MappingVisitor visitor,
-      NominalTypeAnnotation node, List<List<String>> visibleTypeParameterNames,
-      {bool malformedIsError: false, bool deferredIsMalformed: true}) {
+  ResolutionDartType resolveNominalTypeAnnotation(
+      MappingVisitor visitor,
+      NominalTypeAnnotation node,
+      FunctionTypeParameterScope functionTypeParameters,
+      {bool malformedIsError: false,
+      bool deferredIsMalformed: true}) {
     ResolutionRegistry registry = visitor.registry;
 
     Identifier typeName;
@@ -227,7 +218,7 @@ class TypeResolver {
     ResolutionDartType checkNoTypeArguments(ResolutionDartType type) {
       List<ResolutionDartType> arguments = new List<ResolutionDartType>();
       bool hasTypeArgumentMismatch = resolveTypeArguments(visitor, node,
-          const <ResolutionDartType>[], arguments, visibleTypeParameterNames);
+          const <ResolutionDartType>[], arguments, functionTypeParameters);
       if (hasTypeArgumentMismatch) {
         return new MalformedType(
             new ErroneousElementX(MessageKind.TYPE_ARGUMENT_COUNT_MISMATCH,
@@ -279,19 +270,16 @@ class TypeResolver {
       }
       List<ResolutionDartType> arguments = <ResolutionDartType>[];
       resolveTypeArguments(visitor, node, const <ResolutionDartType>[],
-          arguments, visibleTypeParameterNames);
+          arguments, functionTypeParameters);
       return new MalformedType(
           erroneousElement, userProvidedBadType, arguments);
     }
 
     Element element;
-    // Resolve references to type names as dynamic.
-    // TODO(floitsch): this hackishly resolves generic function type arguments
-    // to dynamic.
-    if (prefixName == null &&
-        visibleTypeParameterNames.any((n) => n.contains(typeName.source))) {
-      type = const ResolutionDynamicType();
-    } else {
+    if (prefixName == null) {
+      type = functionTypeParameters.lookup(typeName.source);
+    }
+    if (type == null) {
       element = resolveTypeName(prefixName, typeName, visitor.scope,
           deferredIsMalformed: deferredIsMalformed);
     }
@@ -340,7 +328,7 @@ class TypeResolver {
         cls.computeType(resolution);
         List<ResolutionDartType> arguments = <ResolutionDartType>[];
         bool hasTypeArgumentMismatch = resolveTypeArguments(visitor, node,
-            cls.typeVariables, arguments, visibleTypeParameterNames);
+            cls.typeVariables, arguments, functionTypeParameters);
         if (hasTypeArgumentMismatch) {
           type = new BadInterfaceType(
               cls.declaration,
@@ -363,7 +351,7 @@ class TypeResolver {
         typdef.computeType(resolution);
         List<ResolutionDartType> arguments = <ResolutionDartType>[];
         bool hasTypeArgumentMismatch = resolveTypeArguments(visitor, node,
-            typdef.typeVariables, arguments, visibleTypeParameterNames);
+            typdef.typeVariables, arguments, functionTypeParameters);
         if (hasTypeArgumentMismatch) {
           type = new BadTypedefType(
               typdef,
@@ -441,7 +429,7 @@ class TypeResolver {
       NominalTypeAnnotation node,
       List<ResolutionDartType> typeVariables,
       List<ResolutionDartType> arguments,
-      List<List<String>> visibleTypeParameterNames) {
+      FunctionTypeParameterScope functionTypeParameters) {
     if (node.typeArguments == null) {
       return false;
     }
@@ -457,7 +445,7 @@ class TypeResolver {
         typeArgumentCountMismatch = true;
       }
       ResolutionDartType argType = _resolveTypeAnnotation(
-          visitor, typeArguments.head, visibleTypeParameterNames);
+          visitor, typeArguments.head, functionTypeParameters);
       // TODO(karlklose): rewrite to not modify [arguments].
       arguments.add(argType);
     }
@@ -469,3 +457,40 @@ class TypeResolver {
     return typeArgumentCountMismatch;
   }
 }
+
+/// [FunctionTypeParameterScope] put type parameters in scope for the nested
+/// types.
+///
+/// For example, in the following examples, the generic types `A` would be stored
+/// in a [FunctionTypeParameterScope].
+///
+///     typedef F = List<A> Function<A>(A x)
+///     typedef F = Function(List<A> Function<A>(A x))
+///
+/// They are resolved to `dynamic` until dart2js supports generic methods.
+class FunctionTypeParameterScope {
+  final FunctionTypeParameterScope parent;
+  final Map<String, ResolutionDartType> _map;
+
+  const FunctionTypeParameterScope()
+      : parent = null,
+        _map = const {};
+
+  FunctionTypeParameterScope._(this.parent, this._map);
+
+  FunctionTypeParameterScope expand(NodeList typeParameters) {
+    if (typeParameters == null)
+      return new FunctionTypeParameterScope._(this, const {});
+    Map<String, ResolutionDartType> map = <String, ResolutionDartType>{};
+    for (TypeVariable node in typeParameters) {
+      /// TODO(johnniwinther): Create a special [FunctionTypeVariableType]
+      /// instead of [ResolutionDynamicType].
+      map[node.name.source] = const ResolutionDynamicType();
+    }
+    return new FunctionTypeParameterScope._(this, map);
+  }
+
+  ResolutionDartType lookup(String name) {
+    return _map[name] ?? parent?.lookup(name);
+  }
+}

pkg/compiler/lib/src/resolution/typedefs.dart

@@ -16,6 +16,7 @@ import 'class_hierarchy.dart' show TypeDefinitionVisitor;
 import 'registry.dart' show ResolutionRegistry;
 import 'scope.dart' show MethodScope, TypeDeclarationScope;
 import 'signatures.dart' show SignatureResolver;
+import 'type_resolver.dart' show FunctionTypeParameterScope;
 
 class TypedefResolverVisitor extends TypeDefinitionVisitor {
   TypedefElementX get element => enclosingElement;
@@ -29,8 +30,18 @@ class TypedefResolverVisitor extends TypeDefinitionVisitor {
     scope = new TypeDeclarationScope(scope, element);
     resolveTypeVariableBounds(node.templateParameters);
 
-    FunctionSignature signature = SignatureResolver.analyze(resolution, scope,
-        node.typeParameters, node.formals, node.returnType, element, registry,
+    FunctionTypeParameterScope functionTypeParameters =
+        const FunctionTypeParameterScope().expand(node.typeParameters);
+
+    FunctionSignature signature = SignatureResolver.analyze(
+        resolution,
+        scope,
+        functionTypeParameters,
+        null, // Don't create type variable types for the type parameters.
+        node.formals,
+        node.returnType,
+        element,
+        registry,
         defaultValuesError: MessageKind.TYPEDEF_FORMAL_WITH_DEFAULT);
     element.functionSignature = signature;
 

pkg/pkg.status

@@ -235,8 +235,6 @@ kernel/test/closures_test: Fail
 [ $runtime == vm ]
 analyzer_cli/test/driver_test: Pass, Slow, Timeout
 
-compiler/tool/perf_test: Fail # Unimplemented Typedef handling in kernel transformation.
-
 [ $runtime == vm && $system == windows ]
 analyzer/test/src/task/strong/checker_test: Pass, Slow
 

tests/compiler/dart2js/dart2js.status

@@ -16,8 +16,6 @@ kernel/*: Slow, Pass
 
 boolified_operator_test: Fail # Issue 8001
 
-kernel/closed_world2_test: Fail # Issue 30826
-
 # Skip most serialization tests. These are very slow and are no longer a 
 # priority.
 serialization/analysis1_test: Skip