Skip to content

Commit 94e312f

Browse files
kallentucommit-bot@chromium.org
kallentu
authored and
commit-bot@chromium.org
committed
Added errors for variance positions in method members.
When an explicitly defined type variable is used in an incorrect variance position in a method (whether in the return or the parameters), an error is emitted. Change-Id: I7e49687579bb2ce6e293b052b657b98741cb04f8 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/122740 Commit-Queue: Kallen Tu <kallentu@google.com> Reviewed-by: Leaf Petersen <leafp@google.com> Reviewed-by: Dmitry Stefantsov <dmitryas@google.com>
1 parent 4895910 commit 94e312f

File tree

11 files changed

+1017
-29
lines changed

11 files changed

+1017
-29
lines changed

‎pkg/front_end/lib/src/fasta/builder/class_builder.dart‎

Lines changed: 85 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import 'package:kernel/ast.dart'
3232
TypeParameter,
3333
TypeParameterType,
3434
VariableDeclaration,
35+
Variance,
3536
VoidType;
3637

3738
import 'package:kernel/ast.dart' show FunctionType, TypeParameterType;
@@ -43,7 +44,11 @@ import 'package:kernel/clone.dart' show CloneWithoutBody;
4344
import 'package:kernel/core_types.dart' show CoreTypes;
4445

4546
import 'package:kernel/src/bounds_checks.dart'
46-
show TypeArgumentIssue, findTypeArgumentIssues, getGenericTypeName;
47+
show
48+
TypeArgumentIssue,
49+
computeVariance,
50+
findTypeArgumentIssues,
51+
getGenericTypeName;
4752
import 'package:kernel/text/text_serialization_verifier.dart';
4853

4954
import 'package:kernel/type_algebra.dart' show Substitution, substitute;
@@ -80,6 +85,8 @@ import '../fasta_codes.dart'
8085
templateIncorrectTypeArgumentInSupertypeInferred,
8186
templateInterfaceCheck,
8287
templateInternalProblemNotFoundIn,
88+
templateInvalidTypeVariableVariancePosition,
89+
templateInvalidTypeVariableVariancePositionInReturnType,
8390
templateMixinApplicationIncompatibleSupertype,
8491
templateNamedMixinOverride,
8592
templateOverriddenMethodCause,
@@ -224,7 +231,7 @@ abstract class ClassBuilder implements DeclarationBuilder {
224231
void checkBoundsInSupertype(
225232
Supertype supertype, TypeEnvironment typeEnvironment);
226233

227-
void checkBoundsInOutline(TypeEnvironment typeEnvironment);
234+
void checkTypesInOutline(TypeEnvironment typeEnvironment);
228235

229236
void addRedirectingConstructor(
230237
ProcedureBuilder constructorBuilder, SourceLibraryBuilder library);
@@ -845,7 +852,7 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
845852
}
846853

847854
@override
848-
void checkBoundsInOutline(TypeEnvironment typeEnvironment) {
855+
void checkTypesInOutline(TypeEnvironment typeEnvironment) {
849856
SourceLibraryBuilder library = this.library;
850857

851858
// Check in bounds of own type variables.
@@ -902,6 +909,7 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
902909
library.checkBoundsInField(field, typeEnvironment);
903910
}
904911
for (Procedure procedure in cls.procedures) {
912+
checkVarianceInFunction(procedure, typeEnvironment, cls.typeParameters);
905913
library.checkBoundsInFunctionNode(
906914
procedure.function, typeEnvironment, fileUri);
907915
}
@@ -919,6 +927,80 @@ abstract class ClassBuilderImpl extends DeclarationBuilderImpl
919927
}
920928
}
921929

930+
void checkVarianceInFunction(Procedure procedure,
931+
TypeEnvironment typeEnvironment, List<TypeParameter> typeParameters) {
932+
List<TypeParameter> functionTypeParameters =
933+
procedure.function.typeParameters;
934+
List<VariableDeclaration> positionalParameters =
935+
procedure.function.positionalParameters;
936+
List<VariableDeclaration> namedParameters =
937+
procedure.function.namedParameters;
938+
DartType returnType = procedure.function.returnType;
939+
940+
if (functionTypeParameters != null) {
941+
for (TypeParameter functionParameter in functionTypeParameters) {
942+
for (TypeParameter typeParameter in typeParameters) {
943+
int typeVariance = Variance.combine(Variance.invariant,
944+
computeVariance(typeParameter, functionParameter.bound));
945+
reportVariancePositionIfInvalid(typeVariance, typeParameter, fileUri,
946+
functionParameter.fileOffset);
947+
}
948+
}
949+
}
950+
if (positionalParameters != null) {
951+
for (VariableDeclaration formal in positionalParameters) {
952+
if (!formal.isCovariant) {
953+
for (TypeParameter typeParameter in typeParameters) {
954+
int formalVariance = Variance.combine(Variance.contravariant,
955+
computeVariance(typeParameter, formal.type));
956+
reportVariancePositionIfInvalid(
957+
formalVariance, typeParameter, fileUri, formal.fileOffset);
958+
}
959+
}
960+
}
961+
}
962+
if (namedParameters != null) {
963+
for (VariableDeclaration named in namedParameters) {
964+
for (TypeParameter typeParameter in typeParameters) {
965+
int namedVariance = Variance.combine(Variance.contravariant,
966+
computeVariance(typeParameter, named.type));
967+
reportVariancePositionIfInvalid(
968+
namedVariance, typeParameter, fileUri, named.fileOffset);
969+
}
970+
}
971+
}
972+
if (returnType != null) {
973+
for (TypeParameter typeParameter in typeParameters) {
974+
int returnTypeVariance = computeVariance(typeParameter, returnType);
975+
reportVariancePositionIfInvalid(returnTypeVariance, typeParameter,
976+
fileUri, procedure.function.fileOffset,
977+
isReturnType: true);
978+
}
979+
}
980+
}
981+
982+
void reportVariancePositionIfInvalid(
983+
int variance, TypeParameter typeParameter, Uri fileUri, int fileOffset,
984+
{bool isReturnType: false}) {
985+
SourceLibraryBuilder library = this.library;
986+
if (!typeParameter.isLegacyCovariant &&
987+
!Variance.greaterThanOrEqual(variance, typeParameter.variance)) {
988+
Message message;
989+
if (isReturnType) {
990+
message = templateInvalidTypeVariableVariancePositionInReturnType
991+
.withArguments(Variance.keywordString(typeParameter.variance),
992+
typeParameter.name, Variance.keywordString(variance));
993+
} else {
994+
message = templateInvalidTypeVariableVariancePosition.withArguments(
995+
Variance.keywordString(typeParameter.variance),
996+
typeParameter.name,
997+
Variance.keywordString(variance));
998+
}
999+
library.reportTypeArgumentIssue(
1000+
message, fileUri, fileOffset, typeParameter);
1001+
}
1002+
}
1003+
9221004
@override
9231005
void addRedirectingConstructor(
9241006
ProcedureBuilder constructorBuilder, SourceLibraryBuilder library) {

‎pkg/front_end/lib/src/fasta/fasta_codes_generated.dart‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5027,6 +5027,69 @@ Message _withArgumentsInvalidTypeVariableInSupertypeWithVariance(
50275027
});
50285028
}
50295029

5030+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
5031+
const Template<
5032+
Message Function(
5033+
String string,
5034+
String name,
5035+
String
5036+
string2)> templateInvalidTypeVariableVariancePosition = const Template<
5037+
Message Function(String string, String name, String string2)>(
5038+
messageTemplate:
5039+
r"""Can't use '#string' type variable '#name' in an '#string2' position.""",
5040+
withArguments: _withArgumentsInvalidTypeVariableVariancePosition);
5041+
5042+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
5043+
const Code<Message Function(String string, String name, String string2)>
5044+
codeInvalidTypeVariableVariancePosition =
5045+
const Code<Message Function(String string, String name, String string2)>(
5046+
"InvalidTypeVariableVariancePosition",
5047+
templateInvalidTypeVariableVariancePosition,
5048+
);
5049+
5050+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
5051+
Message _withArgumentsInvalidTypeVariableVariancePosition(
5052+
String string, String name, String string2) {
5053+
if (string.isEmpty) throw 'No string provided';
5054+
if (name.isEmpty) throw 'No name provided';
5055+
name = demangleMixinApplicationName(name);
5056+
if (string2.isEmpty) throw 'No string provided';
5057+
return new Message(codeInvalidTypeVariableVariancePosition,
5058+
message:
5059+
"""Can't use '${string}' type variable '${name}' in an '${string2}' position.""",
5060+
arguments: {'string': string, 'name': name, 'string2': string2});
5061+
}
5062+
5063+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
5064+
const Template<Message Function(String string, String name, String string2)>
5065+
templateInvalidTypeVariableVariancePositionInReturnType = const Template<
5066+
Message Function(String string, String name, String string2)>(
5067+
messageTemplate:
5068+
r"""Can't use '#string' type variable '#name' in an '#string2' position in the return type.""",
5069+
withArguments:
5070+
_withArgumentsInvalidTypeVariableVariancePositionInReturnType);
5071+
5072+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
5073+
const Code<Message Function(String string, String name, String string2)>
5074+
codeInvalidTypeVariableVariancePositionInReturnType =
5075+
const Code<Message Function(String string, String name, String string2)>(
5076+
"InvalidTypeVariableVariancePositionInReturnType",
5077+
templateInvalidTypeVariableVariancePositionInReturnType,
5078+
);
5079+
5080+
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
5081+
Message _withArgumentsInvalidTypeVariableVariancePositionInReturnType(
5082+
String string, String name, String string2) {
5083+
if (string.isEmpty) throw 'No string provided';
5084+
if (name.isEmpty) throw 'No name provided';
5085+
name = demangleMixinApplicationName(name);
5086+
if (string2.isEmpty) throw 'No string provided';
5087+
return new Message(codeInvalidTypeVariableVariancePositionInReturnType,
5088+
message:
5089+
"""Can't use '${string}' type variable '${name}' in an '${string2}' position in the return type.""",
5090+
arguments: {'string': string, 'name': name, 'string2': string2});
5091+
}
5092+
50305093
// DO NOT EDIT. THIS FILE IS GENERATED. SEE TOP OF FILE.
50315094
const Code<Null> codeInvalidUnicodeEscape = messageInvalidUnicodeEscape;
50325095

‎pkg/front_end/lib/src/fasta/source/source_library_builder.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2910,7 +2910,7 @@ class SourceLibraryBuilder extends LibraryBuilderImpl {
29102910
checkBoundsInFunctionNode(declaration.procedure.function,
29112911
typeEnvironment, declaration.fileUri);
29122912
} else if (declaration is ClassBuilder) {
2913-
declaration.checkBoundsInOutline(typeEnvironment);
2913+
declaration.checkTypesInOutline(typeEnvironment);
29142914
}
29152915
}
29162916
inferredTypes.clear();

‎pkg/front_end/messages.status‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,12 @@ InvalidTypeVariableInSupertype/analyzerCode: Fail
348348
InvalidTypeVariableInSupertypeWithVariance/analyzerCode: Fail
349349
InvalidTypeVariableInSupertypeWithVariance/part_wrapped_script: Fail # Triggers multiple errors
350350
InvalidTypeVariableInSupertypeWithVariance/script: Fail # Triggers multiple errors
351+
InvalidTypeVariableVariancePosition/analyzerCode: Fail
352+
InvalidTypeVariableVariancePosition/part_wrapped_script: Fail # Triggers multiple errors
353+
InvalidTypeVariableVariancePosition/script: Fail # Triggers multiple errors
354+
InvalidTypeVariableVariancePositionInReturnType/analyzerCode: Fail
355+
InvalidTypeVariableVariancePositionInReturnType/part_wrapped_script: Fail # Triggers multiple errors
356+
InvalidTypeVariableVariancePositionInReturnType/script: Fail # Triggers multiple errors
351357
InvalidUseOfNullAwareAccess/example: Fail
352358
InvalidVoid/part_wrapped_script1: Fail
353359
InvalidVoid/part_wrapped_script2: Fail

‎pkg/front_end/messages.yaml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3631,6 +3631,22 @@ InvalidTypeVariableInSupertypeWithVariance:
36313631
class A<out X> {}
36323632
class B<out Y> extends A<Function(Y)> {}
36333633
3634+
InvalidTypeVariableVariancePosition:
3635+
template: "Can't use '#string' type variable '#name' in an '#string2' position."
3636+
script: >
3637+
class A<out T> {
3638+
void method(T x) {}
3639+
}
3640+
3641+
InvalidTypeVariableVariancePositionInReturnType:
3642+
template: "Can't use '#string' type variable '#name' in an '#string2' position in the return type."
3643+
script: >
3644+
class A<in T> {
3645+
T method() {
3646+
return null;
3647+
}
3648+
}
3649+
36343650
IllegalRecursiveType:
36353651
template: "Illegal recursive type '#type'."
36363652
script: >

‎pkg/kernel/lib/src/bounds_checks.dart‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -514,11 +514,13 @@ class VarianceCalculator
514514
int visitInterfaceType(InterfaceType node,
515515
Map<TypeParameter, Map<DartType, int>> computedVariances) {
516516
int result = Variance.unrelated;
517-
for (DartType argument in node.typeArguments) {
517+
for (int i = 0; i < node.typeArguments.length; ++i) {
518518
result = Variance.meet(
519519
result,
520-
computeVariance(typeParameter, argument,
521-
computedVariances: computedVariances));
520+
Variance.combine(
521+
node.classNode.typeParameters[i].variance,
522+
computeVariance(typeParameter, node.typeArguments[i],
523+
computedVariances: computedVariances)));
522524
}
523525
return result;
524526
}

0 commit comments

Comments
 (0)