Skip to content

Commit 9bb61c0

Browse files
chloestefantsovaCommit Queue
authored and
Commit Queue
committed
[cfe] Check type variable dependency cycles via extension types
Closes #54097 Closes #54164 Part of #49731 Change-Id: I73aac5f7e2c7f05fd0872b37e6f39fa7b5ed4862 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/337183 Commit-Queue: Chloe Stefantsova <[email protected]> Reviewed-by: Johnni Winther <[email protected]>
1 parent 7320da0 commit 9bb61c0

File tree

64 files changed

+4075
-302
lines changed

Some content is hidden

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

64 files changed

+4075
-302
lines changed

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

+4-4
Original file line numberDiff line numberDiff line change
@@ -2214,7 +2214,7 @@ const Template<Message Function(String name, String string)>
22142214
problemMessageTemplate:
22152215
r"""Type '#name' is a bound of itself via '#string'.""",
22162216
correctionMessageTemplate:
2217-
r"""Try breaking the cycle by removing at least on of the 'extends' clauses in the cycle.""",
2217+
r"""Try breaking the cycle by removing at least one of the 'extends' clauses in the cycle.""",
22182218
withArguments: _withArgumentsCycleInTypeVariables);
22192219

22202220
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
@@ -2233,7 +2233,7 @@ Message _withArgumentsCycleInTypeVariables(String name, String string) {
22332233
problemMessage:
22342234
"""Type '${name}' is a bound of itself via '${string}'.""",
22352235
correctionMessage:
2236-
"""Try breaking the cycle by removing at least on of the 'extends' clauses in the cycle.""",
2236+
"""Try breaking the cycle by removing at least one of the 'extends' clauses in the cycle.""",
22372237
arguments: {'name': name, 'string': string});
22382238
}
22392239

@@ -2555,7 +2555,7 @@ const Template<
25552555
Message Function(String name)>("DirectCycleInTypeVariables",
25562556
problemMessageTemplate: r"""Type '#name' can't use itself as a bound.""",
25572557
correctionMessageTemplate:
2558-
r"""Try breaking the cycle by removing at least on of the 'extends' clauses in the cycle.""",
2558+
r"""Try breaking the cycle by removing at least one of the 'extends' clauses in the cycle.""",
25592559
withArguments: _withArgumentsDirectCycleInTypeVariables);
25602560

25612561
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
@@ -2570,7 +2570,7 @@ Message _withArgumentsDirectCycleInTypeVariables(String name) {
25702570
return new Message(codeDirectCycleInTypeVariables,
25712571
problemMessage: """Type '${name}' can't use itself as a bound.""",
25722572
correctionMessage:
2573-
"""Try breaking the cycle by removing at least on of the 'extends' clauses in the cycle.""",
2573+
"""Try breaking the cycle by removing at least one of the 'extends' clauses in the cycle.""",
25742574
arguments: {'name': name});
25752575
}
25762576

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
@@ -8786,6 +8786,7 @@ class BodyBuilder extends StackListenerImpl
87868786
// Peek to leave type parameters on top of stack.
87878787
List<TypeVariableBuilderBase> typeVariables =
87888788
peek() as List<TypeVariableBuilderBase>;
8789+
libraryBuilder.checkTypeVariableDependencies(typeVariables);
87898790

87908791
List<TypeBuilder> unboundTypes = [];
87918792
List<StructuralVariableBuilder> unboundTypeVariables = [];

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

-64
Original file line numberDiff line numberDiff line change
@@ -3639,70 +3639,6 @@ class OutlineBuilder extends StackListenerImpl {
36393639
libraryFeatures.enhancedEnums, beginToken.charOffset, noLength);
36403640
}
36413641

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

0 commit comments

Comments
 (0)