From 1f7e6370b46b2fbaffefca4686059e77079e88f9 Mon Sep 17 00:00:00 2001 From: Paul Berry Date: Mon, 12 Aug 2019 17:56:45 +0000 Subject: [PATCH] Migration: store decorated type parameter bounds separately from other element decorated types. We will need some special treatment for decorated type parameter bounds, because sometimes we will need to create type parameters (e.g. when comparing decorated types). So we store the bounds of non-ephemeral type parameters (the ones whose enclosing element is non-null) in the Variables data structure, and we store the bounds of ephemeral type parameters (the ones whose enclosing element is null) in an expando. Change-Id: I918c9baffedab9a8871ecea37c8101b5ef44a5d9 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112743 Reviewed-by: Konstantin Shcheglov Commit-Queue: Paul Berry --- .../lib/src/decorated_type.dart | 30 ++++++++++ pkg/nnbd_migration/lib/src/edge_builder.dart | 8 ++- pkg/nnbd_migration/lib/src/node_builder.dart | 9 ++- pkg/nnbd_migration/lib/src/variables.dart | 56 ++++++++++++++++--- .../test/edge_builder_test.dart | 5 +- .../test/node_builder_test.dart | 5 +- 6 files changed, 98 insertions(+), 15 deletions(-) diff --git a/pkg/nnbd_migration/lib/src/decorated_type.dart b/pkg/nnbd_migration/lib/src/decorated_type.dart index e5bf21a1388..4ed4cc53b23 100644 --- a/pkg/nnbd_migration/lib/src/decorated_type.dart +++ b/pkg/nnbd_migration/lib/src/decorated_type.dart @@ -13,6 +13,14 @@ import 'package:nnbd_migration/src/potential_modification.dart'; /// tracking the (unmigrated) [DartType], we track the [ConstraintVariable]s /// indicating whether the type, and the types that compose it, are nullable. class DecoratedType { + /// Mapping from type parameter elements to the decorated types of those type + /// parameters' bounds. + /// + /// This expando only applies to type parameters whose enclosing element is + /// `null`. Type parameters whose enclosing element is not `null` should be + /// stored in [Variables._decoratedTypeParameterBounds]. + static final _decoratedTypeParameterBounds = Expando(); + final DartType type; final NullabilityNode node; @@ -302,6 +310,28 @@ class DecoratedType { returnType._substitute(substitution, undecoratedResult.returnType), positionalParameters: newPositionalParameters); } + + /// Retrieves the decorated bound of the given [typeParameter]. + /// + /// [typeParameter] must have an enclosing element of `null`. Type parameters + /// whose enclosing element is not `null` are tracked by the [Variables] + /// class. + static DecoratedType decoratedTypeParameterBound( + TypeParameterElement typeParameter) { + assert(typeParameter.enclosingElement == null); + return _decoratedTypeParameterBounds[typeParameter]; + } + + /// Stores he decorated bound of the given [typeParameter]. + /// + /// [typeParameter] must have an enclosing element of `null`. Type parameters + /// whose enclosing element is not `null` are tracked by the [Variables] + /// class. + static void recordTypeParameterBound( + TypeParameterElement typeParameter, DecoratedType bound) { + assert(typeParameter.enclosingElement == null); + _decoratedTypeParameterBounds[typeParameter] = bound; + } } /// A [DecoratedType] based on a type annotation appearing explicitly in the diff --git a/pkg/nnbd_migration/lib/src/edge_builder.dart b/pkg/nnbd_migration/lib/src/edge_builder.dart index 312a3dd1b45..9bdae8a144d 100644 --- a/pkg/nnbd_migration/lib/src/edge_builder.dart +++ b/pkg/nnbd_migration/lib/src/edge_builder.dart @@ -1005,13 +1005,15 @@ $stackTrace'''); for (int i = 0; i < instantiatedType.typeArguments.length; i++) { _unionDecoratedTypes( instantiatedType.typeArguments[i], - _variables.decoratedElementType(element.typeParameters[i]), + _variables.decoratedTypeParameterBound(element.typeParameters[i]), origin); } } else { for (int i = 0; i < typeArguments.length; i++) { DecoratedType bound; - bound = _variables.decoratedElementType(element.typeParameters[i]); + bound = + _variables.decoratedTypeParameterBound(element.typeParameters[i]); + assert(bound != null); var argumentType = _variables.decoratedTypeAnnotation(_source, typeArguments[i]); if (argumentType == null) { @@ -1188,7 +1190,7 @@ $stackTrace'''); // TODO(paulberry): once we've wired up flow analysis, return promoted // bounds if applicable. return _variables - .decoratedElementType((type.type as TypeParameterType).element); + .decoratedTypeParameterBound((type.type as TypeParameterType).element); } /// Creates the necessary constraint(s) for an assignment of the given diff --git a/pkg/nnbd_migration/lib/src/node_builder.dart b/pkg/nnbd_migration/lib/src/node_builder.dart index 306ac474ac7..1a30d118df5 100644 --- a/pkg/nnbd_migration/lib/src/node_builder.dart +++ b/pkg/nnbd_migration/lib/src/node_builder.dart @@ -346,7 +346,7 @@ $stackTrace'''); AlwaysNullableTypeOrigin(_source, node.offset)); decoratedBound = DecoratedType(_typeProvider.objectType, nullabilityNode); } - _variables.recordDecoratedElementType(element, decoratedBound); + _variables.recordDecoratedTypeParameterBound(element, decoratedBound); return null; } @@ -548,6 +548,10 @@ abstract class VariableRecorder { Source source, TypeAnnotation node, DecoratedTypeAnnotation type, {bool potentialModification: true}); + /// Stores he decorated bound of the given [typeParameter]. + void recordDecoratedTypeParameterBound( + TypeParameterElement typeParameter, DecoratedType bound); + /// Records that [node] is associated with the question of whether the named /// [parameter] should be optional (should not have a `required` /// annotation added to it). @@ -580,6 +584,9 @@ abstract class VariableRepository { DecoratedType decoratedTypeAnnotation( Source source, TypeAnnotation typeAnnotation); + /// Retrieves the decorated bound of the given [typeParameter]. + DecoratedType decoratedTypeParameterBound(TypeParameterElement typeParameter); + /// Records conditional discard information for the given AST node (which is /// an `if` statement or a conditional (`?:`) expression). void recordConditionalDiscard( diff --git a/pkg/nnbd_migration/lib/src/variables.dart b/pkg/nnbd_migration/lib/src/variables.dart index 1b52ddfd9e5..acd3a687781 100644 --- a/pkg/nnbd_migration/lib/src/variables.dart +++ b/pkg/nnbd_migration/lib/src/variables.dart @@ -20,6 +20,8 @@ class Variables implements VariableRecorder, VariableRepository { final _decoratedElementTypes = {}; + final _decoratedTypeParameterBounds = {}; + final _decoratedDirectSupertypes = >{}; @@ -42,8 +44,12 @@ class Variables implements VariableRecorder, VariableRepository { } @override - DecoratedType decoratedElementType(Element element) => - _decoratedElementTypes[element] ??= _createDecoratedElementType(element); + DecoratedType decoratedElementType(Element element) { + assert(element is! TypeParameterElement, + 'Use decoratedTypeParameterBound instead'); + return _decoratedElementTypes[element] ??= + _createDecoratedElementType(element); + } @override DecoratedType decoratedTypeAnnotation( @@ -62,6 +68,35 @@ class Variables implements VariableRecorder, VariableRepository { return decoratedTypeAnnotation; } + @override + DecoratedType decoratedTypeParameterBound( + TypeParameterElement typeParameter) { + if (typeParameter.enclosingElement == null) { + var decoratedType = + DecoratedType.decoratedTypeParameterBound(typeParameter); + if (decoratedType == null) { + throw StateError( + 'A decorated type for the bound of $typeParameter should ' + 'have been stored by the NodeBuilder via recordTypeParameterBound'); + } + return decoratedType; + } else { + var decoratedType = _decoratedTypeParameterBounds[typeParameter]; + if (decoratedType == null) { + if (_graph.isBeingMigrated(typeParameter.library.source)) { + throw StateError( + 'A decorated type for the bound of $typeParameter should ' + 'have been stored by the NodeBuilder via ' + 'recordTypeParameterBound'); + } + decoratedType = _alreadyMigratedCodeDecorator + .decorate(typeParameter.bound ?? DynamicTypeImpl.instance); + _decoratedTypeParameterBounds[typeParameter] = decoratedType; + } + return decoratedType; + } + } + Map> getPotentialModifications() => _potentialModifications; @@ -87,6 +122,8 @@ class Variables implements VariableRecorder, VariableRepository { void recordDecoratedElementType(Element element, DecoratedType type) { assert(() { + assert(element is! TypeParameterElement, + 'Use recordDecoratedTypeParameterBound instead'); var library = element.library; if (library == null) { // No problem; the element is probably a parameter of a function type @@ -109,6 +146,16 @@ class Variables implements VariableRecorder, VariableRepository { {})[_uniqueOffsetForTypeAnnotation(node)] = type; } + @override + void recordDecoratedTypeParameterBound( + TypeParameterElement typeParameter, DecoratedType bound) { + if (typeParameter.enclosingElement == null) { + DecoratedType.recordTypeParameterBound(typeParameter, bound); + } else { + _decoratedTypeParameterBounds[typeParameter] = bound; + } + } + @override void recordExpressionChecks( Source source, Expression expression, ExpressionChecks checks) { @@ -187,11 +234,6 @@ class Variables implements VariableRecorder, VariableRepository { decoratedType = _alreadyMigratedCodeDecorator.decorate(element.type); } else if (element is TopLevelVariableElement) { decoratedType = _alreadyMigratedCodeDecorator.decorate(element.type); - } else if (element is TypeParameterElement) { - // By convention, type parameter elements are decorated with the type of - // their bounds. - decoratedType = _alreadyMigratedCodeDecorator - .decorate(element.bound ?? DynamicTypeImpl.instance); } else { // TODO(paulberry) throw UnimplementedError('Decorating ${element.runtimeType}'); diff --git a/pkg/nnbd_migration/test/edge_builder_test.dart b/pkg/nnbd_migration/test/edge_builder_test.dart index cd1f159c892..4fa94373959 100644 --- a/pkg/nnbd_migration/test/edge_builder_test.dart +++ b/pkg/nnbd_migration/test/edge_builder_test.dart @@ -3626,7 +3626,7 @@ void f(Point x) {} var pointClass = findNode.typeName('Point').name.staticElement as ClassElement; var pointBound = - variables.decoratedElementType(pointClass.typeParameters[0]); + variables.decoratedTypeParameterBound(pointClass.typeParameters[0]); expect(pointBound.type.toString(), 'num'); assertEdge(decoratedTypeAnnotation('int>').node, pointBound.node, hard: true); @@ -3637,7 +3637,8 @@ void f(Point x) {} void f(List x) {} '''); var listClass = typeProvider.listType.element; - var listBound = variables.decoratedElementType(listClass.typeParameters[0]); + var listBound = + variables.decoratedTypeParameterBound(listClass.typeParameters[0]); expect(listBound.type.toString(), 'dynamic'); assertEdge(decoratedTypeAnnotation('int>').node, listBound.node, hard: true); diff --git a/pkg/nnbd_migration/test/node_builder_test.dart b/pkg/nnbd_migration/test/node_builder_test.dart index 19a25467564..227839edd4b 100644 --- a/pkg/nnbd_migration/test/node_builder_test.dart +++ b/pkg/nnbd_migration/test/node_builder_test.dart @@ -23,8 +23,9 @@ class NodeBuilderTest extends MigrationVisitorTestBase { variables.decoratedElementType( findNode.functionDeclaration(search).declaredElement); - DecoratedType decoratedTypeParameterBound(String search) => variables - .decoratedElementType(findNode.typeParameter(search).declaredElement); + DecoratedType decoratedTypeParameterBound(String search) => + variables.decoratedTypeParameterBound( + findNode.typeParameter(search).declaredElement); test_class_alias_synthetic_constructors_no_parameters() async { await analyze('''