Skip to content

Commit dd17a4c

Browse files
leafpetersencommit-bot@chromium.org
leafpetersen
authored and
[email protected]
committed
Alpha-vary generic function types before performing inference.
Generic function types were not previously renamed before using them for inference, which resulted in capture problems when doing inference on recursive generic method invocations. Fixes #30980 Bug: Change-Id: I1c610606dd65d86735cb615e077d720066a85e1a Reviewed-on: https://dart-review.googlesource.com/15040 Commit-Queue: Leaf Petersen <[email protected]> Reviewed-by: Brian Wilkerson <[email protected]> Reviewed-by: Jenny Messerly <[email protected]>
1 parent b573b1b commit dd17a4c

File tree

5 files changed

+157
-49
lines changed

5 files changed

+157
-49
lines changed

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

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -408,6 +408,61 @@ class FunctionTypeImpl extends TypeImpl implements FunctionType {
408408
: this._(element, element?.name, prunedTypedefs, null, null, null, null,
409409
false);
410410

411+
/**
412+
* Initialize a newly created function type that is semantically the same as
413+
* [original], but which has been syntactically renamed with fresh type
414+
* parameters at its outer binding site (if any).
415+
*/
416+
factory FunctionTypeImpl.fresh(FunctionType original) {
417+
// We build up a substitution for the type parameters,
418+
// {variablesFresh/variables} then apply it.
419+
420+
var originalFormals = original.typeFormals;
421+
var formalCount = originalFormals.length;
422+
if (formalCount == 0) return original;
423+
424+
// Allocate fresh type variables
425+
var typeVars = <DartType>[];
426+
var freshTypeVars = <DartType>[];
427+
var freshVarElements = <TypeParameterElement>[];
428+
for (int i = 0; i < formalCount; i++) {
429+
var typeParamElement = originalFormals[i];
430+
var freshElement =
431+
new TypeParameterElementImpl.synthetic(typeParamElement.name);
432+
var freshTypeVar = new TypeParameterTypeImpl(freshElement);
433+
freshElement.type = freshTypeVar;
434+
435+
typeVars.add(typeParamElement.type);
436+
freshTypeVars.add(freshTypeVar);
437+
freshVarElements.add(freshElement);
438+
}
439+
440+
// Simultaneous substitution to rename the bounds
441+
for (int i = 0; i < formalCount; i++) {
442+
var typeParamElement = originalFormals[i];
443+
var bound = typeParamElement.bound;
444+
if (bound != null) {
445+
var freshElement = freshVarElements[i] as TypeParameterElementImpl;
446+
freshElement.bound = bound.substitute2(freshTypeVars, typeVars);
447+
}
448+
}
449+
450+
// Instantiate the original type with the fresh type variables
451+
// (replacing the old type variables)
452+
var newType = original.instantiate(freshTypeVars);
453+
454+
// Build a synthetic element for the type, binding the fresh type parameters
455+
var name = original.name ?? "";
456+
var element = original.element;
457+
var function = new FunctionElementImpl(name, -1);
458+
function.enclosingElement = element.enclosingElement;
459+
function.isSynthetic = true;
460+
function.returnType = newType.returnType;
461+
function.typeParameters = freshVarElements;
462+
function.shareParameters(newType.parameters);
463+
return function.type = new FunctionTypeImpl(function);
464+
}
465+
411466
/**
412467
* Private constructor.
413468
*/
@@ -607,7 +662,6 @@ class FunctionTypeImpl extends TypeImpl implements FunctionType {
607662
specializedParams[i] = new FieldFormalParameterMember(parameter, this);
608663
continue;
609664
}
610-
611665
var baseType = parameter.type as TypeImpl;
612666
TypeImpl type;
613667
if (typeArguments.isEmpty ||
@@ -939,10 +993,6 @@ class FunctionTypeImpl extends TypeImpl implements FunctionType {
939993
FunctionType substitute2(
940994
List<DartType> argumentTypes, List<DartType> parameterTypes,
941995
[List<FunctionTypeAliasElement> prune]) {
942-
// Pruned types should only ever result from performing type variable
943-
// substitution, and it doesn't make sense to substitute again after
944-
// substituting once.
945-
assert(this.prunedTypedefs == null);
946996
if (argumentTypes.length != parameterTypes.length) {
947997
throw new ArgumentError(
948998
"argumentTypes.length (${argumentTypes.length}) != parameterTypes.length (${parameterTypes.length})");
@@ -1048,14 +1098,12 @@ class FunctionTypeImpl extends TypeImpl implements FunctionType {
10481098
// instantiated type arguments, that way we wouldn't need to recover them.
10491099
//
10501100
// For now though, this is a pretty quick operation.
1051-
assert(identical(g.element, f.element));
10521101
if (g.typeFormals.isEmpty) {
10531102
assert(g == f);
10541103
return DartType.EMPTY_LIST;
10551104
}
10561105
assert(f.typeFormals.isEmpty);
1057-
assert(g.typeFormals.length + g.typeArguments.length ==
1058-
f.typeArguments.length);
1106+
assert(g.typeFormals.length <= f.typeArguments.length);
10591107

10601108
// Instantiation in Analyzer works like this:
10611109
// Given:
@@ -1068,7 +1116,7 @@ class FunctionTypeImpl extends TypeImpl implements FunctionType {
10681116
//
10691117
// Therefore, we can recover the typeArguments from our instantiated
10701118
// function.
1071-
return f.typeArguments.skip(g.typeArguments.length);
1119+
return f.typeArguments.skip(f.typeArguments.length - g.typeFormals.length);
10721120
}
10731121

10741122
/**

‎pkg/analyzer/lib/src/generated/static_type_analyzer.dart

Lines changed: 12 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1906,8 +1906,12 @@ class StaticTypeAnalyzer extends SimpleAstVisitor<Object> {
19061906
*/
19071907
void _inferGenericInvocationExpression(InvocationExpression node) {
19081908
ArgumentList arguments = node.argumentList;
1909-
FunctionType inferred = _inferGenericInvoke(node, node.function.staticType,
1910-
node.typeArguments, arguments, node.function);
1909+
var type = node.function.staticType;
1910+
var freshType =
1911+
type is FunctionType ? new FunctionTypeImpl.fresh(type) : type;
1912+
1913+
FunctionType inferred = _inferGenericInvoke(
1914+
node, freshType, node.typeArguments, arguments, node.function);
19111915
if (inferred != null && inferred != node.staticInvokeType) {
19121916
// Fix up the parameter elements based on inferred method.
19131917
arguments.correspondingStaticParameters = ResolverVisitor
@@ -2276,44 +2280,20 @@ class StaticTypeAnalyzer extends SimpleAstVisitor<Object> {
22762280
return type;
22772281
}
22782282

2279-
// TODO(jmesserly): feels like we should be able to do this with less code.
2280-
2281-
// Create fresh type formals. This avoids capture if we're inferring the
2282-
// constructor to the class from inside it.
2283-
2284-
// We build up a substitution for the type parameters,
2285-
// {variablesFresh/variables} then apply it.
2286-
var typeVars = <DartType>[];
2287-
var freshTypeVars = <DartType>[];
2288-
var freshVarElements = <TypeParameterElement>[];
2289-
for (int i = 0; i < cls.typeParameters.length; i++) {
2290-
var typeParamElement = cls.typeParameters[i];
2291-
var freshElement =
2292-
new TypeParameterElementImpl.synthetic(typeParamElement.name);
2293-
var freshTypeVar = new TypeParameterTypeImpl(freshElement);
2294-
freshElement.type = freshTypeVar;
2295-
2296-
typeVars.add(typeParamElement.type);
2297-
freshTypeVars.add(freshTypeVar);
2298-
freshVarElements.add(freshElement);
2299-
2300-
var bound = typeParamElement.bound ?? DynamicTypeImpl.instance;
2301-
freshElement.bound = bound.substitute2(freshTypeVars, typeVars);
2302-
}
2303-
2304-
type = type.substitute2(freshTypeVars, typeVars);
2305-
2283+
// Create a synthetic function type using the class type parameters,
2284+
// and then rename it with fresh variables.
23062285
var name = cls.name;
23072286
if (constructor.name != null) {
23082287
name += '.' + constructor.name;
23092288
}
23102289
var function = new FunctionElementImpl(name, -1);
2311-
function.enclosingElement = cls;
2290+
function.enclosingElement = cls.enclosingElement;
23122291
function.isSynthetic = true;
23132292
function.returnType = type.returnType;
2314-
function.typeParameters = freshVarElements;
2293+
function.shareTypeParameters(cls.typeParameters);
23152294
function.shareParameters(type.parameters);
2316-
return function.type = new FunctionTypeImpl(function);
2295+
function.type = new FunctionTypeImpl(function);
2296+
return new FunctionTypeImpl.fresh(function.type);
23172297
}
23182298

23192299
/**

‎pkg/analyzer/test/generated/strong_mode_test.dart

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1585,6 +1585,90 @@ Consider passing explicit type argument(s) to the generic.
15851585
verify([source]);
15861586
}
15871587

1588+
test_inference_simplePolymorphicRecursion_function() async {
1589+
// Regression test for https://github.com/dart-lang/sdk/issues/30980
1590+
// Check that inference works properly when inferring the type argument
1591+
// for a self-recursive call with a function type
1592+
var source = addSource(r'''
1593+
void _mergeSort<T>(T Function(T) list, int compare(T a, T b), T Function(T) target) {
1594+
_mergeSort(list, compare, target);
1595+
_mergeSort(list, compare, list);
1596+
_mergeSort(target, compare, target);
1597+
_mergeSort(target, compare, list);
1598+
}
1599+
''');
1600+
var analysisResult = await computeAnalysisResult(source);
1601+
assertNoErrors(source);
1602+
verify([source]);
1603+
var unit = analysisResult.unit;
1604+
var body = (AstFinder
1605+
.getTopLevelFunction(unit, '_mergeSort')
1606+
.functionExpression
1607+
.body as BlockFunctionBody);
1608+
var stmts = body.block.statements;
1609+
for (ExpressionStatement stmt in stmts) {
1610+
MethodInvocation invoke = stmt.expression;
1611+
ParameterizedType fType = invoke.staticInvokeType;
1612+
expect(fType.typeArguments[0].toString(), 'T');
1613+
}
1614+
}
1615+
1616+
test_inference_simplePolymorphicRecursion_interface() async {
1617+
// Regression test for https://github.com/dart-lang/sdk/issues/30980
1618+
// Check that inference works properly when inferring the type argument
1619+
// for a self-recursive call with an interface type
1620+
var source = addSource(r'''
1621+
void _mergeSort<T>(List<T> list, int compare(T a, T b), List<T> target) {
1622+
_mergeSort(list, compare, target);
1623+
_mergeSort(list, compare, list);
1624+
_mergeSort(target, compare, target);
1625+
_mergeSort(target, compare, list);
1626+
}
1627+
''');
1628+
var analysisResult = await computeAnalysisResult(source);
1629+
assertNoErrors(source);
1630+
verify([source]);
1631+
var unit = analysisResult.unit;
1632+
var body = (AstFinder
1633+
.getTopLevelFunction(unit, '_mergeSort')
1634+
.functionExpression
1635+
.body as BlockFunctionBody);
1636+
var stmts = body.block.statements;
1637+
for (ExpressionStatement stmt in stmts) {
1638+
MethodInvocation invoke = stmt.expression;
1639+
ParameterizedType fType = invoke.staticInvokeType;
1640+
expect(fType.typeArguments[0].toString(), 'T');
1641+
}
1642+
}
1643+
1644+
test_inference_simplePolymorphicRecursion_simple() async {
1645+
// Regression test for https://github.com/dart-lang/sdk/issues/30980
1646+
// Check that inference works properly when inferring the type argument
1647+
// for a self-recursive call with a simple type parameter
1648+
var source = addSource(r'''
1649+
void _mergeSort<T>(T list, int compare(T a, T b), T target) {
1650+
_mergeSort(list, compare, target);
1651+
_mergeSort(list, compare, list);
1652+
_mergeSort(target, compare, target);
1653+
_mergeSort(target, compare, list);
1654+
}
1655+
''');
1656+
var analysisResult = await computeAnalysisResult(source);
1657+
assertNoErrors(source);
1658+
verify([source]);
1659+
var unit = analysisResult.unit;
1660+
var body = (AstFinder
1661+
.getTopLevelFunction(unit, '_mergeSort')
1662+
.functionExpression
1663+
.body as BlockFunctionBody);
1664+
var stmts = body.block.statements;
1665+
for (ExpressionStatement stmt in stmts) {
1666+
MethodInvocation invoke = stmt.expression;
1667+
ParameterizedType fType = invoke.staticInvokeType;
1668+
expect(fType.typeArguments[0].toString(), 'T');
1669+
}
1670+
}
1671+
15881672
test_inferGenericInstantiation() async {
15891673
// Verify that we don't infer '?` when we instantiate a generic function.
15901674
var source = addSource(r'''

‎pkg/analyzer/test/src/task/strong/front_end_inference_test.dart

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -344,10 +344,8 @@ class _InstrumentationVisitor extends RecursiveAstVisitor<Null> {
344344

345345
/// Based on DDC code generator's `_recoverTypeArguments`
346346
Iterable<DartType> _recoverTypeArguments(FunctionType g, FunctionType f) {
347-
assert(identical(g.element, f.element));
348347
assert(g.typeFormals.isNotEmpty && f.typeFormals.isEmpty);
349-
assert(g.typeFormals.length + g.typeArguments.length ==
350-
f.typeArguments.length);
351-
return f.typeArguments.skip(g.typeArguments.length);
348+
assert(g.typeFormals.length <= f.typeArguments.length);
349+
return f.typeArguments.skip(f.typeArguments.length - g.typeFormals.length);
352350
}
353351
}

‎pkg/dev_compiler/lib/src/analyzer/code_generator.dart

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3799,10 +3799,8 @@ class CodeGenerator extends Object
37993799
Iterable<DartType> _recoverTypeArguments(FunctionType g, FunctionType f) {
38003800
// TODO(jmesserly): this design is a bit unfortunate. It would be nice if
38013801
// resolution could simply create a synthetic type argument list.
3802-
assert(identical(g.element, f.element));
38033802
assert(g.typeFormals.isNotEmpty && f.typeFormals.isEmpty);
3804-
assert(g.typeFormals.length + g.typeArguments.length ==
3805-
f.typeArguments.length);
3803+
assert(g.typeFormals.length <= f.typeArguments.length);
38063804

38073805
// Instantiation in Analyzer works like this:
38083806
// Given:
@@ -3815,7 +3813,7 @@ class CodeGenerator extends Object
38153813
//
38163814
// Therefore, we can recover the typeArguments from our instantiated
38173815
// function.
3818-
return f.typeArguments.skip(g.typeArguments.length);
3816+
return f.typeArguments.skip(f.typeArguments.length - g.typeFormals.length);
38193817
}
38203818

38213819
/// Emits code for the `JS(...)` macro.

0 commit comments

Comments
 (0)