Skip to content

Commit a52b9ed

Browse files
chloestefantsovaCommit Queue
authored and
Commit Queue
committed
[cfe] Reland "Check type variable dependency cycles via extension types"
The original CL: https://dart-review.googlesource.com/c/sdk/+/337183 Closes #54097 Closes #54164 Part of #49731 Change-Id: I3291d17b6edcde87b56ff25d3d99902882100f07 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/342581 Commit-Queue: Chloe Stefantsova <[email protected]> Reviewed-by: Johnni Winther <[email protected]>
1 parent bf96a72 commit a52b9ed

File tree

77 files changed

+4243
-306
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

77 files changed

+4243
-306
lines changed

pkg/_fe_analyzer_shared/lib/src/messages/codes_generated.dart

+4-4
Original file line numberDiff line numberDiff line change
@@ -2239,7 +2239,7 @@ const Template<Message Function(String name, String string)>
22392239
problemMessageTemplate:
22402240
r"""Type '#name' is a bound of itself via '#string'.""",
22412241
correctionMessageTemplate:
2242-
r"""Try breaking the cycle by removing at least on of the 'extends' clauses in the cycle.""",
2242+
r"""Try breaking the cycle by removing at least one of the 'extends' clauses in the cycle.""",
22432243
withArguments: _withArgumentsCycleInTypeVariables);
22442244

22452245
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
@@ -2258,7 +2258,7 @@ Message _withArgumentsCycleInTypeVariables(String name, String string) {
22582258
problemMessage:
22592259
"""Type '${name}' is a bound of itself via '${string}'.""",
22602260
correctionMessage:
2261-
"""Try breaking the cycle by removing at least on of the 'extends' clauses in the cycle.""",
2261+
"""Try breaking the cycle by removing at least one of the 'extends' clauses in the cycle.""",
22622262
arguments: {'name': name, 'string': string});
22632263
}
22642264

@@ -2580,7 +2580,7 @@ const Template<
25802580
Message Function(String name)>("DirectCycleInTypeVariables",
25812581
problemMessageTemplate: r"""Type '#name' can't use itself as a bound.""",
25822582
correctionMessageTemplate:
2583-
r"""Try breaking the cycle by removing at least on of the 'extends' clauses in the cycle.""",
2583+
r"""Try breaking the cycle by removing at least one of the 'extends' clauses in the cycle.""",
25842584
withArguments: _withArgumentsDirectCycleInTypeVariables);
25852585

25862586
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
@@ -2595,7 +2595,7 @@ Message _withArgumentsDirectCycleInTypeVariables(String name) {
25952595
return new Message(codeDirectCycleInTypeVariables,
25962596
problemMessage: """Type '${name}' can't use itself as a bound.""",
25972597
correctionMessage:
2598-
"""Try breaking the cycle by removing at least on of the 'extends' clauses in the cycle.""",
2598+
"""Try breaking the cycle by removing at least one of the 'extends' clauses in the cycle.""",
25992599
arguments: {'name': name});
26002600
}
26012601

pkg/front_end/lib/src/fasta/builder/declaration_builders.dart

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import 'package:kernel/src/unaliasing.dart';
88

99
import '../fasta_codes.dart';
1010
import '../kernel/body_builder_context.dart';
11+
import '../kernel/type_builder_computer.dart';
1112
import '../messages.dart';
1213
import '../modifier.dart';
1314
import '../problems.dart' show internalProblem, unexpected, unhandled;

pkg/front_end/lib/src/fasta/builder/type_alias_builder.dart

+18-4
Original file line numberDiff line numberDiff line change
@@ -282,10 +282,24 @@ abstract class TypeAliasBuilderImpl extends TypeDeclarationBuilderImpl
282282
if (typeVariables != null) {
283283
if (typeArguments == null ||
284284
typeVariables.length != typeArguments.length) {
285-
typeArguments = <TypeBuilder>[
286-
for (NominalVariableBuilder typeVariable in typeVariables)
287-
typeVariable.defaultType!
288-
];
285+
if (typeVariables.isEmpty) {
286+
typeArguments = <TypeBuilder>[];
287+
} else {
288+
if (typeVariables.first.kind == TypeVariableKind.fromKernel) {
289+
TypeBuilderComputer typeBuilderComputer =
290+
new TypeBuilderComputer(libraryBuilder.loader);
291+
typeArguments = <TypeBuilder>[
292+
for (NominalVariableBuilder typeVariable in typeVariables)
293+
typeVariable.parameter.defaultType
294+
.accept(typeBuilderComputer)
295+
];
296+
} else {
297+
typeArguments = <TypeBuilder>[
298+
for (NominalVariableBuilder typeVariable in typeVariables)
299+
typeVariable.defaultType!
300+
];
301+
}
302+
}
289303
}
290304
return unaliasedRhsType!.subst(
291305
new Map<NominalVariableBuilder, TypeBuilder>.fromIterables(

pkg/front_end/lib/src/fasta/builder/type_variable_builder.dart

+142-10
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,98 @@ sealed class TypeVariableBuilderBase extends TypeDeclarationBuilderImpl
8383

8484
void finish(SourceLibraryBuilder library, ClassBuilder object,
8585
TypeBuilder dynamicType);
86+
87+
TypeBuilder? _unaliasAndErase(TypeBuilder? typeBuilder) {
88+
if (typeBuilder is! NamedTypeBuilder) {
89+
return typeBuilder;
90+
} else {
91+
TypeDeclarationBuilder? declaration = typeBuilder.declaration;
92+
if (declaration is TypeAliasBuilder) {
93+
// We pass empty lists as [unboundTypes] and [unboundTypeVariables]
94+
// because new builders can be generated during unaliasing. We ignore
95+
// the returned builders, however, because they will not be used in the
96+
// output and are needed only for the checks.
97+
//
98+
// We also don't instantiate-to-bound raw types because it won't affect
99+
// the dependency cycle analysis.
100+
return _unaliasAndErase(declaration.unalias(typeBuilder.typeArguments,
101+
unboundTypes: [], unboundTypeVariables: []));
102+
} else if (declaration is ExtensionTypeDeclarationBuilder) {
103+
TypeBuilder? representationType =
104+
declaration.declaredRepresentationTypeBuilder;
105+
if (representationType == null) {
106+
return null;
107+
} else {
108+
List<NominalVariableBuilder>? typeParameters =
109+
declaration.typeParameters;
110+
List<TypeBuilder>? typeArguments = typeBuilder.typeArguments;
111+
if (typeParameters != null && typeArguments != null) {
112+
representationType = representationType.subst(
113+
new Map<NominalVariableBuilder, TypeBuilder>.fromIterables(
114+
typeParameters, typeArguments));
115+
}
116+
return _unaliasAndErase(representationType);
117+
}
118+
} else {
119+
return typeBuilder;
120+
}
121+
}
122+
}
123+
124+
TypeVariableCyclicDependency? findCyclicDependency(
125+
{Map<TypeVariableBuilderBase, TypeVariableTraversalState>?
126+
typeVariablesTraversalState,
127+
Map<TypeVariableBuilderBase, TypeVariableBuilderBase>? cycleElements}) {
128+
typeVariablesTraversalState ??= {};
129+
cycleElements ??= {};
130+
131+
switch (typeVariablesTraversalState[this] ??=
132+
TypeVariableTraversalState.unvisited) {
133+
case TypeVariableTraversalState.visited:
134+
return null;
135+
case TypeVariableTraversalState.active:
136+
typeVariablesTraversalState[this] = TypeVariableTraversalState.visited;
137+
List<TypeVariableBuilderBase>? viaTypeVariables;
138+
TypeVariableBuilderBase? nextViaTypeVariable = cycleElements[this];
139+
while (nextViaTypeVariable != null && nextViaTypeVariable != this) {
140+
(viaTypeVariables ??= []).add(nextViaTypeVariable);
141+
nextViaTypeVariable = cycleElements[nextViaTypeVariable];
142+
}
143+
return new TypeVariableCyclicDependency(this,
144+
viaTypeVariables: viaTypeVariables);
145+
case TypeVariableTraversalState.unvisited:
146+
typeVariablesTraversalState[this] = TypeVariableTraversalState.active;
147+
TypeBuilder? bound = this.bound;
148+
if (bound is NamedTypeBuilder) {
149+
TypeBuilder? unaliasedAndErasedBound = _unaliasAndErase(bound);
150+
TypeDeclarationBuilder? unaliasedAndErasedBoundDeclaration =
151+
unaliasedAndErasedBound?.declaration;
152+
TypeVariableBuilderBase? nextVariable;
153+
if (unaliasedAndErasedBoundDeclaration is TypeVariableBuilderBase) {
154+
nextVariable = unaliasedAndErasedBoundDeclaration;
155+
}
156+
157+
if (nextVariable != null) {
158+
cycleElements[this] = nextVariable;
159+
TypeVariableCyclicDependency? result =
160+
nextVariable.findCyclicDependency(
161+
typeVariablesTraversalState: typeVariablesTraversalState,
162+
cycleElements: cycleElements);
163+
typeVariablesTraversalState[this] =
164+
TypeVariableTraversalState.visited;
165+
return result;
166+
} else {
167+
typeVariablesTraversalState[this] =
168+
TypeVariableTraversalState.visited;
169+
return null;
170+
}
171+
} else {
172+
typeVariablesTraversalState[this] =
173+
TypeVariableTraversalState.visited;
174+
return null;
175+
}
176+
}
177+
}
86178
}
87179

88180
class NominalVariableBuilder extends TypeVariableBuilderBase {
@@ -310,21 +402,17 @@ class NominalVariableBuilder extends TypeVariableBuilderBase {
310402
}
311403
}
312404

313-
List< /* TypeVariableBuilder | FunctionTypeTypeVariableBuilder */ Object>
314-
sortAllTypeVariablesTopologically(
315-
Iterable< /* TypeVariableBuilder | FunctionTypeTypeVariableBuilder */
316-
Object>
317-
typeVariables) {
405+
List<TypeVariableBuilderBase> sortAllTypeVariablesTopologically(
406+
Iterable<TypeVariableBuilderBase> typeVariables) {
318407
assert(typeVariables.every((typeVariable) =>
319408
typeVariable is NominalVariableBuilder ||
320409
typeVariable is StructuralVariableBuilder));
321410

322-
Set< /* TypeVariableBuilder | FunctionTypeTypeVariableBuilder */ Object>
323-
unhandled = new Set<Object>.identity()..addAll(typeVariables);
324-
List< /* TypeVariableBuilder | FunctionTypeTypeVariableBuilder */ Object>
325-
result = <Object>[];
411+
Set<TypeVariableBuilderBase> unhandled =
412+
new Set<TypeVariableBuilderBase>.identity()..addAll(typeVariables);
413+
List<TypeVariableBuilderBase> result = <TypeVariableBuilderBase>[];
326414
while (unhandled.isNotEmpty) {
327-
Object rootVariable = unhandled.first;
415+
TypeVariableBuilderBase rootVariable = unhandled.first;
328416
unhandled.remove(rootVariable);
329417

330418
TypeBuilder? rootVariableBound;
@@ -684,3 +772,47 @@ class FreshStructuralVariableBuildersFromNominalVariableBuilders {
684772
FreshStructuralVariableBuildersFromNominalVariableBuilders(
685773
this.freshStructuralVariableBuilders, this.substitutionMap);
686774
}
775+
776+
/// This enum is used internally for dependency analysis of type variables.
777+
enum TypeVariableTraversalState {
778+
/// An [unvisited] type variable isn't yet visited by the traversal algorithm.
779+
unvisited,
780+
781+
/// An [active] type variable is traversed, but not fully processed.
782+
active,
783+
784+
/// A [visited] type variable is fully processed.
785+
visited;
786+
}
787+
788+
/// Represents a cyclic dependency of a type variable on itself.
789+
///
790+
/// An examples of such dependencies are X in the following cases.
791+
///
792+
/// typedef F<Y> = Y;
793+
/// extension type E<Y>(Y it) {}
794+
///
795+
/// class A<X extends X> {} // Error.
796+
/// class B<X extends Y, Y extends X> {} // Error.
797+
/// class C<X extends F<Y>, Y extends X> {} // Error.
798+
/// class D<X extends E<Y>, Y extends X> {} // Error.
799+
class TypeVariableCyclicDependency {
800+
/// Type variable that's the bound of itself.
801+
final TypeVariableBuilderBase typeVariableBoundOfItself;
802+
803+
/// The elements in a non-trivial self-dependency cycle.
804+
///
805+
/// The loop is considered non-trivial if it includes more than one type
806+
/// variable.
807+
final List<TypeVariableBuilderBase>? viaTypeVariables;
808+
809+
TypeVariableCyclicDependency(this.typeVariableBoundOfItself,
810+
{this.viaTypeVariables});
811+
812+
@override
813+
String toString() {
814+
return "TypeVariableCyclicDependency("
815+
"typeVariableBoundOfItself=${typeVariableBoundOfItself}, "
816+
"viaTypeVariable=${viaTypeVariables})";
817+
}
818+
}

pkg/front_end/lib/src/fasta/kernel/body_builder.dart

+1
Original file line numberDiff line numberDiff line change
@@ -8784,6 +8784,7 @@ class BodyBuilder extends StackListenerImpl
87848784
// Peek to leave type parameters on top of stack.
87858785
List<TypeVariableBuilderBase> typeVariables =
87868786
peek() as List<TypeVariableBuilderBase>;
8787+
libraryBuilder.checkTypeVariableDependencies(typeVariables);
87878788

87888789
List<TypeBuilder> unboundTypes = [];
87898790
List<StructuralVariableBuilder> unboundTypeVariables = [];

pkg/front_end/lib/src/fasta/source/outline_builder.dart

-64
Original file line numberDiff line numberDiff line change
@@ -3643,70 +3643,6 @@ class OutlineBuilder extends StackListenerImpl {
36433643
libraryFeatures.enhancedEnums, beginToken.charOffset, noLength);
36443644
}
36453645

3646-
// Peek to leave type parameters on top of stack.
3647-
List<TypeVariableBuilderBase>? typeParameters =
3648-
peek() as List<TypeVariableBuilderBase>?;
3649-
3650-
Map<String, TypeVariableBuilderBase>? typeVariablesByName;
3651-
if (typeParameters != null) {
3652-
for (TypeVariableBuilderBase builder in typeParameters) {
3653-
if (builder.bound != null) {
3654-
typeVariablesByName ??= <String, TypeVariableBuilderBase>{
3655-
for (TypeVariableBuilderBase builder in typeParameters)
3656-
builder.name: builder
3657-
};
3658-
3659-
// Find cycle: If there's no cycle we can at most step through all
3660-
// `typeParameters` (at which point the last builders bound will be
3661-
// null).
3662-
// If there is a cycle with `builder` 'inside' the steps to get back
3663-
// to it will also be bound by `typeParameters.length`.
3664-
// If there is a cycle without `builder` 'inside' we will just ignore
3665-
// it for now. It will be reported when processing one of the
3666-
// `builder`s that is in fact `inside` the cycle. This matches the
3667-
// cyclic class hierarchy error.
3668-
TypeVariableBuilderBase? bound = builder;
3669-
for (int steps = 0;
3670-
bound!.bound != null && steps < typeParameters.length;
3671-
++steps) {
3672-
TypeName? typeName = bound.bound!.typeName;
3673-
bound = typeVariablesByName[typeName?.name];
3674-
if (bound == null || bound == builder) break;
3675-
}
3676-
if (bound == builder && bound!.bound != null) {
3677-
// Write out cycle.
3678-
List<String> via = <String>[];
3679-
TypeName? typeName = bound.bound!.typeName;
3680-
bound = typeVariablesByName[typeName?.name];
3681-
while (bound != builder) {
3682-
via.add(bound!.name);
3683-
TypeName? typeName = bound.bound!.typeName;
3684-
bound = typeVariablesByName[typeName?.name];
3685-
}
3686-
Message message = via.isEmpty
3687-
? templateDirectCycleInTypeVariables.withArguments(builder.name)
3688-
: templateCycleInTypeVariables.withArguments(
3689-
builder.name, via.join("', '"));
3690-
addProblem(message, builder.charOffset, builder.name.length);
3691-
builder.bound = new NamedTypeBuilderImpl(
3692-
new SyntheticTypeName(builder.name, builder.charOffset),
3693-
const NullabilityBuilder.omitted(),
3694-
fileUri: uri,
3695-
charOffset: builder.charOffset,
3696-
instanceTypeVariableAccess:
3697-
//InstanceTypeVariableAccessState.Unexpected
3698-
declarationContext.instanceTypeVariableAccessState)
3699-
..bind(
3700-
libraryBuilder,
3701-
new InvalidTypeDeclarationBuilder(
3702-
builder.name,
3703-
message.withLocation(
3704-
uri, builder.charOffset, builder.name.length)));
3705-
}
3706-
}
3707-
}
3708-
}
3709-
37103646
if (inConstructorName) {
37113647
addProblem(messageConstructorWithTypeParameters,
37123648
offsetForToken(beginToken), lengthOfSpan(beginToken, endToken));

0 commit comments

Comments
 (0)