From c0c63bf16e27a091991181d7523bbfd26e541276 Mon Sep 17 00:00:00 2001 From: Johnni Winther Date: Tue, 26 Sep 2017 08:23:34 +0000 Subject: [PATCH] Fix handling of function type parameters Closes #30826 Change-Id: I804bff2a204aa210ba59f691c5d680d8a53aadeb Reviewed-on: https://dart-review.googlesource.com/7553 Reviewed-by: Sigmund Cherem --- .../lib/src/kernel/element_map_impl.dart | 12 +- .../lib/src/resolution/class_hierarchy.dart | 9 +- pkg/compiler/lib/src/resolution/members.dart | 9 +- .../lib/src/resolution/resolution.dart | 2 + .../lib/src/resolution/signatures.dart | 25 ++-- .../lib/src/resolution/type_resolver.dart | 125 +++++++++++------- pkg/compiler/lib/src/resolution/typedefs.dart | 15 ++- pkg/pkg.status | 2 - tests/compiler/dart2js/dart2js.status | 2 - 9 files changed, 126 insertions(+), 75 deletions(-) diff --git a/pkg/compiler/lib/src/kernel/element_map_impl.dart b/pkg/compiler/lib/src/kernel/element_map_impl.dart index fc2c4c188c3..8ca8fa6820d 100644 --- a/pkg/compiler/lib/src/kernel/element_map_impl.dart +++ b/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 { final KernelToElementMapBase elementMap; + final Set currentFunctionTypeParameters = + new Set(); bool topLevel = true; DartTypeConverter(this.elementMap); @@ -1485,12 +1487,18 @@ class DartTypeConverter extends ir.DartTypeVisitor { @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 { .toList()), node.namedParameters.map((n) => n.name).toList(), node.namedParameters.map((n) => visitType(n.type)).toList()); + currentFunctionTypeParameters.removeAll(node.typeParameters); + return type; } @override diff --git a/pkg/compiler/lib/src/resolution/class_hierarchy.dart b/pkg/compiler/lib/src/resolution/class_hierarchy.dart index 7eec9b8fd12..6233855efd8 100644 --- a/pkg/compiler/lib/src/resolution/class_hierarchy.dart +++ b/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 { 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( diff --git a/pkg/compiler/lib/src/resolution/members.dart b/pkg/compiler/lib/src/resolution/members.dart index e6dff359aa9..3a900008b5e 100644 --- a/pkg/compiler/lib/src/resolution/members.dart +++ b/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 { function.functionSignature = SignatureResolver.analyze( resolution, scope, + const FunctionTypeParameterScope(), node.typeVariables, node.parameters, node.returnType, @@ -4132,10 +4134,13 @@ class ResolverVisitor extends MappingVisitor { } 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) { diff --git a/pkg/compiler/lib/src/resolution/resolution.dart b/pkg/compiler/lib/src/resolution/resolution.dart index 3267764b3e1..aabe838ea85 100644 --- a/pkg/compiler/lib/src/resolution/resolution.dart +++ b/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, diff --git a/pkg/compiler/lib/src/resolution/signatures.dart b/pkg/compiler/lib/src/resolution/signatures.dart index ddc35286d56..36890b2aab0 100644 --- a/pkg/compiler/lib/src/resolution/signatures.dart +++ b/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 { final Scope scope; final MessageKind defaultValuesError; final bool createRealParameters; + final FunctionTypeParameterScope functionTypeParameters; List optionalParameters = const []; 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 { 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 { static FunctionSignature analyze( Resolution resolution, Scope scope, + FunctionTypeParameterScope functionTypeParameters, NodeList typeVariables, NodeList formalParameters, Node returnNode, @@ -353,7 +351,7 @@ class SignatureResolver extends MappingVisitor { 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 parameters = const []; @@ -493,7 +491,8 @@ class SignatureResolver extends MappingVisitor { 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(); } diff --git a/pkg/compiler/lib/src/resolution/type_resolver.dart b/pkg/compiler/lib/src/resolution/type_resolver.dart index 3d45c9d632f..40d5dd9f521 100644 --- a/pkg/compiler/lib/src/resolution/type_resolver.dart +++ b/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 Function(A x))`. - // - // They are resolved to `dynamic` until dart2js supports generic methods. ResolutionDartType _resolveTypeAnnotation(MappingVisitor visitor, - TypeAnnotation node, List> 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> 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 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> 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 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> 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 arguments = new List(); bool hasTypeArgumentMismatch = resolveTypeArguments(visitor, node, - const [], arguments, visibleTypeParameterNames); + const [], arguments, functionTypeParameters); if (hasTypeArgumentMismatch) { return new MalformedType( new ErroneousElementX(MessageKind.TYPE_ARGUMENT_COUNT_MISMATCH, @@ -279,19 +270,16 @@ class TypeResolver { } List arguments = []; resolveTypeArguments(visitor, node, const [], - 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 arguments = []; 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 arguments = []; 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 typeVariables, List arguments, - List> 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 Function(A x) +/// typedef F = Function(List Function(A x)) +/// +/// They are resolved to `dynamic` until dart2js supports generic methods. +class FunctionTypeParameterScope { + final FunctionTypeParameterScope parent; + final Map _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 map = {}; + 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); + } +} diff --git a/pkg/compiler/lib/src/resolution/typedefs.dart b/pkg/compiler/lib/src/resolution/typedefs.dart index 16823a00547..e3e58eca74c 100644 --- a/pkg/compiler/lib/src/resolution/typedefs.dart +++ b/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; diff --git a/pkg/pkg.status b/pkg/pkg.status index 6ca08f01b63..5ecf1619d45 100644 --- a/pkg/pkg.status +++ b/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 diff --git a/tests/compiler/dart2js/dart2js.status b/tests/compiler/dart2js/dart2js.status index e03ed59a196..86ed9f84121 100644 --- a/tests/compiler/dart2js/dart2js.status +++ b/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