Skip to content

Commit 19920f0

Browse files
Revert "Fix #30858, generic function types not serializable when const."
This reverts commit 60ee2c0. Reason for revert: Requires https://dart-review.googlesource.com/c/sdk/+/33180 which can land first. Original change's description: > Fix #30858, generic function types not serializable when const. > > Previously declaring for instance const lists (or even final lists) of > generic function types ie `final x = <void Function()>[]` would throw > errors during summarization. > > The type parameters don't seem correctly stored yet, but that is an > edge case. Left a TODO, for now this should go in to prevent the > analyzer crash. > > One thing I changed as well is that `serializeType` assumed it was > getting a type name, and `serializeTypeName` accepted types but threw > when it didn't get a type name. I flipped the names so that > `serializeTypeName` accepts a type name and `serializeType` accepts a > type in general and decides whether its a type name or a generic > function type to proceed with specialization. > > Bug: 30858 > Change-Id: Id128f8625cbf03bb94d05ff0efdbac3b158e637e > Reviewed-on: https://dart-review.googlesource.com/20481 > Reviewed-by: Paul Berry <[email protected]> > Reviewed-by: Konstantin Shcheglov <[email protected]> [email protected],[email protected],[email protected] Change-Id: I6fae28335f7815b1e8c43597fb9451519a13335e No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 30858 Reviewed-on: https://dart-review.googlesource.com/33200 Reviewed-by: Mike Fairhurst <[email protected]> Commit-Queue: Mike Fairhurst <[email protected]>
1 parent dd8decb commit 19920f0

File tree

10 files changed

+89
-172
lines changed

10 files changed

+89
-172
lines changed

pkg/analyzer/lib/src/dart/element/type.dart

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -418,17 +418,14 @@ class FunctionTypeImpl extends TypeImpl implements FunctionType {
418418
* Initialize a newly created function type that is semantically the same as
419419
* [original], but which has been syntactically renamed with fresh type
420420
* parameters at its outer binding site (if any).
421-
*
422-
* If type formals is empty, this returns the original unless [force] is set
423-
* to [true].
424421
*/
425-
factory FunctionTypeImpl.fresh(FunctionType original, {bool force = false}) {
422+
factory FunctionTypeImpl.fresh(FunctionType original) {
426423
// We build up a substitution for the type parameters,
427424
// {variablesFresh/variables} then apply it.
428425

429426
var originalFormals = original.typeFormals;
430427
var formalCount = originalFormals.length;
431-
if (formalCount == 0 && !force) return original;
428+
if (formalCount == 0) return original;
432429

433430
// Allocate fresh type variables
434431
var typeVars = <DartType>[];

pkg/analyzer/lib/src/summary/format.dart

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2307,12 +2307,6 @@ class EntityRefBuilder extends Object
23072307
_implicitFunctionTypeIndices ??= <int>[];
23082308

23092309
/**
2310-
* Notice: This will be deprecated. However, its not deprecated yet, as we're
2311-
* keeping it for backwards compatibilty, and marking it deprecated makes it
2312-
* unreadable.
2313-
*
2314-
* TODO(mfairhurst) mark this deprecated, and remove its logic.
2315-
*
23162310
* If this is a reference to a function type implicitly defined by a
23172311
* function-typed parameter, a list of zero-based indices indicating the path
23182312
* from the entity referred to by [reference] to the appropriate type

pkg/analyzer/lib/src/summary/format.fbs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,12 +1215,6 @@ table EntityRef {
12151215
entityKind:EntityRefKind (id: 8);
12161216

12171217
/**
1218-
* Notice: This will be deprecated. However, its not deprecated yet, as we're
1219-
* keeping it for backwards compatibilty, and marking it deprecated makes it
1220-
* unreadable.
1221-
*
1222-
* TODO(mfairhurst) mark this deprecated, and remove its logic.
1223-
*
12241218
* If this is a reference to a function type implicitly defined by a
12251219
* function-typed parameter, a list of zero-based indices indicating the path
12261220
* from the entity referred to by [reference] to the appropriate type

pkg/analyzer/lib/src/summary/idl.dart

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -405,12 +405,6 @@ abstract class EntityRef extends base.SummaryClass {
405405
EntityRefKind get entityKind;
406406

407407
/**
408-
* Notice: This will be deprecated. However, its not deprecated yet, as we're
409-
* keeping it for backwards compatibilty, and marking it deprecated makes it
410-
* unreadable.
411-
*
412-
* TODO(mfairhurst) mark this deprecated, and remove its logic.
413-
*
414408
* If this is a reference to a function type implicitly defined by a
415409
* function-typed parameter, a list of zero-based indices indicating the path
416410
* from the entity referred to by [reference] to the appropriate type

pkg/analyzer/lib/src/summary/link.dart

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -229,18 +229,14 @@ EntityRefBuilder _createLinkedType(
229229
// TODO(paulberry): do I need to store type arguments?
230230
return result;
231231
}
232-
if (element is GenericFunctionTypeElementForLink) {
233-
// Function types are their own type parameter context
234-
typeParameterContext = element;
232+
if (element is GenericFunctionTypeElement) {
235233
result.entityKind = EntityRefKind.genericFunctionType;
236234
result.syntheticReturnType = _createLinkedType(
237235
type.returnType, compilationUnit, typeParameterContext);
238236
result.syntheticParams = type.parameters
239237
.map((ParameterElement param) => _serializeSyntheticParam(
240238
param, compilationUnit, typeParameterContext))
241239
.toList();
242-
_storeTypeArguments(
243-
type.typeArguments, result, compilationUnit, typeParameterContext);
244240
return result;
245241
}
246242
// TODO(paulberry): implement other cases.

pkg/analyzer/lib/src/summary/summarize_ast.dart

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,7 @@ class _ConstExprSerializer extends AbstractConstExprSerializer {
7474
@override
7575
EntityRefBuilder serializeConstructorRef(DartType type, Identifier typeName,
7676
TypeArgumentList typeArguments, SimpleIdentifier name) {
77-
EntityRefBuilder typeBuilder =
78-
serializeTypeName(type, typeName, typeArguments);
77+
EntityRefBuilder typeBuilder = serializeType(type, typeName, typeArguments);
7978
if (name == null) {
8079
return typeBuilder;
8180
} else {
@@ -144,14 +143,10 @@ class _ConstExprSerializer extends AbstractConstExprSerializer {
144143
}
145144

146145
@override
147-
EntityRefBuilder serializeTypeName(
146+
EntityRefBuilder serializeType(
148147
DartType type, Identifier name, TypeArgumentList arguments) {
149-
return visitor.serializeTypeName(name, arguments);
148+
return visitor.serializeType(name, arguments);
150149
}
151-
152-
@override
153-
EntityRefBuilder serializeGenericFunctionType(GenericFunctionType node) =>
154-
visitor.serializeGenericFunctionType(node);
155150
}
156151

157152
/**
@@ -454,15 +449,16 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
454449
b.typeParameters =
455450
serializeTypeParameters(typeParameters, typeParameterScope);
456451
if (superclass != null) {
457-
b.supertype = serializeType(superclass);
452+
b.supertype = serializeTypeName(superclass);
458453
} else {
459454
b.hasNoSupertype = isCoreLibrary && name == 'Object';
460455
}
461456
if (withClause != null) {
462-
b.mixins = withClause.mixinTypes.map(serializeType).toList();
457+
b.mixins = withClause.mixinTypes.map(serializeTypeName).toList();
463458
}
464459
if (implementsClause != null) {
465-
b.interfaces = implementsClause.interfaces.map(serializeType).toList();
460+
b.interfaces =
461+
implementsClause.interfaces.map(serializeTypeName).toList();
466462
}
467463
if (members != null) {
468464
scopes.add(buildClassMemberScope(name, members));
@@ -628,7 +624,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
628624
if (!isTopLevel) {
629625
b.isStatic = isDeclaredStatic;
630626
}
631-
b.returnType = serializeType(returnType);
627+
b.returnType = serializeTypeName(returnType);
632628
bool isSemanticallyStatic = isTopLevel || isDeclaredStatic;
633629
if (formalParameters != null) {
634630
bool oldMayInheritCovariance = _parametersMayInheritCovariance;
@@ -739,7 +735,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
739735
*/
740736
void serializeFunctionTypedParameterDetails(UnlinkedParamBuilder b,
741737
TypeAnnotation returnType, FormalParameterList parameters) {
742-
EntityRefBuilder serializedReturnType = serializeType(returnType);
738+
EntityRefBuilder serializedReturnType = serializeTypeName(returnType);
743739
if (serializedReturnType != null) {
744740
b.type = serializedReturnType;
745741
}
@@ -763,7 +759,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
763759
serializeTypeParameters(node.typeParameters, typeParameterScope);
764760
b.syntheticReturnType = node.returnType == null
765761
? serializeDynamic()
766-
: serializeType(node.returnType);
762+
: serializeTypeName(node.returnType);
767763
b.syntheticParams = node.parameters.parameters
768764
.map((FormalParameter p) => p.accept(this) as UnlinkedParamBuilder)
769765
.toList();
@@ -870,7 +866,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
870866
* a [EntityRef]. Note that this method does the right thing if the
871867
* name doesn't refer to an entity other than a type (e.g. a class member).
872868
*/
873-
EntityRefBuilder serializeTypeName(
869+
EntityRefBuilder serializeType(
874870
Identifier identifier, TypeArgumentList typeArguments) {
875871
if (identifier == null) {
876872
return null;
@@ -915,7 +911,8 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
915911
'Unexpected identifier type: ${identifier.runtimeType}');
916912
}
917913
if (typeArguments != null) {
918-
b.typeArguments = typeArguments.arguments.map(serializeType).toList();
914+
b.typeArguments =
915+
typeArguments.arguments.map(serializeTypeName).toList();
919916
}
920917
return b;
921918
}
@@ -927,9 +924,9 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
927924
* a [EntityRef]. Note that this method does the right thing if the
928925
* name doesn't refer to an entity other than a type (e.g. a class member).
929926
*/
930-
EntityRefBuilder serializeType(TypeAnnotation node) {
927+
EntityRefBuilder serializeTypeName(TypeAnnotation node) {
931928
if (node is TypeName) {
932-
return serializeTypeName(node?.name, node?.typeArguments);
929+
return serializeType(node?.name, node?.typeArguments);
933930
} else if (node is GenericFunctionType) {
934931
return serializeGenericFunctionType(node);
935932
} else if (node != null) {
@@ -977,7 +974,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
977974
b.isStatic = isDeclaredStatic;
978975
b.name = variable.name.name;
979976
b.nameOffset = variable.name.offset;
980-
b.type = serializeType(variables.type);
977+
b.type = serializeTypeName(variables.type);
981978
b.documentationComment = serializeDocumentation(documentationComment);
982979
b.annotations = serializeAnnotations(annotations);
983980
b.codeRange = serializeCodeRange(variables.parent);
@@ -1161,7 +1158,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
11611158
if (node.parameters != null) {
11621159
serializeFunctionTypedParameterDetails(b, node.type, node.parameters);
11631160
} else {
1164-
b.type = serializeType(node.type);
1161+
b.type = serializeTypeName(node.type);
11651162
}
11661163
}
11671164
return b;
@@ -1225,7 +1222,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
12251222
b.nameOffset = node.name.offset;
12261223
b.typeParameters =
12271224
serializeTypeParameters(node.typeParameters, typeParameterScope);
1228-
EntityRefBuilder serializedReturnType = serializeType(node.returnType);
1225+
EntityRefBuilder serializedReturnType = serializeTypeName(node.returnType);
12291226
if (serializedReturnType != null) {
12301227
b.returnType = serializedReturnType;
12311228
}
@@ -1348,7 +1345,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
13481345
@override
13491346
UnlinkedParamBuilder visitSimpleFormalParameter(SimpleFormalParameter node) {
13501347
UnlinkedParamBuilder b = serializeParameter(node);
1351-
b.type = serializeType(node.type);
1348+
b.type = serializeTypeName(node.type);
13521349
return b;
13531350
}
13541351

@@ -1364,7 +1361,7 @@ class _SummarizeAstVisitor extends RecursiveAstVisitor {
13641361
b.name = node.name.name;
13651362
b.nameOffset = node.name.offset;
13661363
if (node.bound != null) {
1367-
b.bound = serializeType(node.bound);
1364+
b.bound = serializeTypeName(node.bound);
13681365
}
13691366
b.annotations = serializeAnnotations(node.metadata);
13701367
b.codeRange = serializeCodeRange(node);

pkg/analyzer/lib/src/summary/summarize_const_expr.dart

Lines changed: 9 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -198,25 +198,15 @@ abstract class AbstractConstExprSerializer {
198198
* given [name] and [arguments]. The parameter [type] might be `null` if the
199199
* type is not resolved.
200200
*/
201-
EntityRefBuilder serializeTypeName(
201+
EntityRefBuilder serializeType(
202202
DartType type, Identifier name, TypeArgumentList arguments);
203203

204-
/**
205-
* Return [EntityRefBuilder] that corresponds to the [type], which is defined
206-
* using generic function type syntax. These may appear as the type arguments
207-
* of a const list, etc.
208-
*/
209-
EntityRefBuilder serializeGenericFunctionType(GenericFunctionType type);
210-
211204
/**
212205
* Return [EntityRefBuilder] that corresponds to the given [type].
213206
*/
214-
EntityRefBuilder serializeType(TypeAnnotation type) {
207+
EntityRefBuilder serializeTypeName(TypeAnnotation type) {
215208
if (type is TypeName) {
216-
return serializeTypeName(type?.type, type?.name, type?.typeArguments);
217-
}
218-
if (type is GenericFunctionType) {
219-
return serializeGenericFunctionType(type);
209+
return serializeType(type?.type, type?.name, type?.typeArguments);
220210
}
221211
throw new ArgumentError(
222212
'Cannot serialize an instance of ${type.runtimeType}');
@@ -417,12 +407,12 @@ abstract class AbstractConstExprSerializer {
417407
} else if (expr is AsExpression) {
418408
isValidConst = false;
419409
_serialize(expr.expression);
420-
references.add(serializeType(expr.type));
410+
references.add(serializeTypeName(expr.type));
421411
operations.add(UnlinkedExprOperation.typeCast);
422412
} else if (expr is IsExpression) {
423413
isValidConst = false;
424414
_serialize(expr.expression);
425-
references.add(serializeType(expr.type));
415+
references.add(serializeTypeName(expr.type));
426416
operations.add(UnlinkedExprOperation.typeCheck);
427417
} else if (expr is SuperExpression) {
428418
operations.add(UnlinkedExprOperation.pushSuper);
@@ -564,7 +554,7 @@ abstract class AbstractConstExprSerializer {
564554
}
565555
if (expr.typeArguments != null &&
566556
expr.typeArguments.arguments.length == 1) {
567-
references.add(serializeType(expr.typeArguments.arguments[0]));
557+
references.add(serializeTypeName(expr.typeArguments.arguments[0]));
568558
operations.add(UnlinkedExprOperation.makeTypedList);
569559
} else {
570560
operations.add(UnlinkedExprOperation.makeUntypedList);
@@ -583,8 +573,8 @@ abstract class AbstractConstExprSerializer {
583573
}
584574
if (expr.typeArguments != null &&
585575
expr.typeArguments.arguments.length == 2) {
586-
references.add(serializeType(expr.typeArguments.arguments[0]));
587-
references.add(serializeType(expr.typeArguments.arguments[1]));
576+
references.add(serializeTypeName(expr.typeArguments.arguments[0]));
577+
references.add(serializeTypeName(expr.typeArguments.arguments[1]));
588578
operations.add(UnlinkedExprOperation.makeTypedMap);
589579
} else {
590580
operations.add(UnlinkedExprOperation.makeUntypedMap);
@@ -708,7 +698,7 @@ abstract class AbstractConstExprSerializer {
708698
} else {
709699
ints.add(typeArguments.arguments.length);
710700
for (TypeAnnotation type in typeArguments.arguments) {
711-
references.add(serializeType(type));
701+
references.add(serializeTypeName(type));
712702
}
713703
}
714704
}

pkg/analyzer/lib/src/task/strong_mode.dart

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -169,16 +169,7 @@ class InstanceMemberInferrer {
169169
parameter, index, overriddenTypes[i].parameters);
170170
DartType type = matchingParameter?.type ?? typeProvider.dynamicType;
171171
if (parameterType == null) {
172-
if (type is FunctionType) {
173-
// The resulting parameter's type element has an `enclosingElement` of
174-
// the overridden parameter. Change it to the overriding parameter.
175-
parameterType = new FunctionTypeImpl.fresh(type, force: true);
176-
(parameterType.element as ElementImpl).enclosingElement = parameter;
177-
// TODO(mfairhurst) handle cases where non-functions contain functions
178-
// See test_inferredType_parameter_genericFunctionType_asTypeArgument
179-
} else {
180-
parameterType = type;
181-
}
172+
parameterType = type;
182173
} else if (parameterType != type) {
183174
if (parameter is ParameterElementForLink) {
184175
parameter.setInferenceError(new TopLevelInferenceErrorBuilder(

pkg/analyzer/test/src/summary/linker_test.dart

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -467,33 +467,6 @@ class C extends A<int> {
467467
expect(fType.parameters[0].type.toString(), 'int');
468468
}
469469

470-
@failingTest
471-
void test_inferredType_parameter_genericFunctionType_asTypeArgument() {
472-
var bundle = createPackageBundle('''
473-
class A<T> {
474-
A<R> map<R>(List<R Function(T)> fs) => null;
475-
}
476-
''', path: '/a.dart');
477-
addBundle('/a.ds', bundle);
478-
createLinker('''
479-
import 'a.dart';
480-
class C extends A<int> {
481-
map<R2>(fs) => null;
482-
}
483-
''');
484-
LibraryElementForLink library = linker.getLibrary(linkerInputs.testDartUri);
485-
library.libraryCycleForLink.ensureLinked();
486-
ClassElementForLink_Class c = library.getContainedName('C');
487-
expect(c.methods, hasLength(1));
488-
MethodElementForLink map = c.methods[0];
489-
expect(map.parameters, hasLength(1));
490-
InterfaceType iType = map.parameters[0].type;
491-
expect(iType.typeArguments, hasLength(1));
492-
FunctionType fType = iType.typeArguments[0];
493-
expect(fType.returnType.toString(), 'R2');
494-
expect(fType.parameters[0].type.toString(), 'int');
495-
}
496-
497470
void test_inferredType_staticField_dynamic() {
498471
createLinker('''
499472
dynamic x = null;

0 commit comments

Comments
 (0)