Skip to content

Commit 6a2c350

Browse files
Dmitry Stefantsovcommit-bot@chromium.org
Dmitry Stefantsov
authored and
commit-bot@chromium.org
committed
[cfe] Fix isSubtypeOf for FutureOrs
Previously it was assumed both A <: B implies FutureOr<A> <: FutureOr<B> and FutureOr<A> <: FutureOr<B> implies A <: B. This CL fixes the implementation. Closes #38905. Bug: http://dartbug.com/38905 Change-Id: I2c8d105659b310e3ab5db261ebdc81cbfa456ae1 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/135301 Reviewed-by: Johnni Winther <johnniwinther@google.com> Commit-Queue: Dmitry Stefantsov <dmitryas@google.com>
1 parent 0299903 commit 6a2c350

5 files changed

Lines changed: 75 additions & 8 deletions

File tree

‎pkg/front_end/lib/src/fasta/kernel/types.dart‎

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,38 @@ class IsFutureOrSubtypeOf extends TypeRelation<InterfaceType> {
734734
// This follows from combining rules 7, 10, and 11.
735735
DartType sArgument = sFutureOr.typeArguments.single;
736736
DartType tArgument = tFutureOr.typeArguments.single;
737-
return types.performNullabilityAwareSubtypeCheck(sArgument, tArgument);
737+
DartType sFutureOfArgument = new InterfaceType(types.hierarchy.futureClass,
738+
Nullability.nonNullable, sFutureOr.typeArguments);
739+
DartType tFutureOfArgument = new InterfaceType(types.hierarchy.futureClass,
740+
Nullability.nonNullable, tFutureOr.typeArguments);
741+
Nullability sNullability =
742+
computeNullabilityOfFutureOr(sFutureOr, types.hierarchy.futureOrClass);
743+
Nullability tNullability =
744+
computeNullabilityOfFutureOr(tFutureOr, types.hierarchy.futureOrClass);
745+
// The following is an optimized is-subtype-of test for the case where
746+
// both LHS and RHS are FutureOrs. It's based on the following:
747+
// FutureOr<X> <: FutureOr<Y> iff X <: Y OR (X <: Future<Y> AND
748+
// Future<X> <: Y).
749+
//
750+
// The correctness of that can be shown as follows:
751+
// 1. FutureOr<X> <: Y iff X <: Y AND Future<X> <: Y
752+
// 2. X <: FutureOr<Y> iff X <: Y OR X <: Future<Y>
753+
// 3. 1,2 => FutureOr<X> <: FutureOr<Y> iff
754+
// (X <: Y OR X <: Future<Y>) AND
755+
// (Future<X> <: Y OR Future<X> <: Future<Y>)
756+
// 4. X <: Y iff Future<X> <: Future<Y>
757+
// 5. 3,4 => FutureOr<X> <: FutureOr<Y> iff
758+
// (X <: Y OR X <: Future<Y>) AND
759+
// (X <: Y OR Future<X> <: Y) iff
760+
// X <: Y OR (X <: Future<Y> AND Future<X> <: Y)
761+
return types
762+
.performNullabilityAwareSubtypeCheck(sArgument, tArgument)
763+
.or(types
764+
.performNullabilityAwareSubtypeCheck(sArgument, tFutureOfArgument)
765+
.andSubtypeCheckFor(sFutureOfArgument, tArgument, types))
766+
.and(new IsSubtypeOf.basedSolelyOnNullabilities(
767+
sFutureOr.withNullability(sNullability),
768+
tFutureOr.withNullability(tNullability)));
738769
}
739770

740771
@override

‎pkg/front_end/test/fasta/types/shared_type_tests.dart‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ abstract class SubtypeTest<T, E> {
1414
T supertype = toType(supertypeString, environment);
1515
Expect.isTrue(
1616
isSubtypeImpl(subtype, supertype).isSubtypeWhenUsingNullabilities(),
17-
"$subtypeString should always be a subtype of $supertypeString.");
17+
"$subtypeString should be a subtype of ${supertypeString}, "
18+
"regardless of whether the nullability modifiers are ignored or not.");
1819
}
1920

2021
void isNotSubtype(String subtypeString, String supertypeString,
@@ -24,7 +25,8 @@ abstract class SubtypeTest<T, E> {
2425
T supertype = toType(supertypeString, environment);
2526
Expect.isFalse(
2627
isSubtypeImpl(subtype, supertype).isSubtypeWhenIgnoringNullabilities(),
27-
"$subtypeString should never be a subtype of $supertypeString.");
28+
"$subtypeString shouldn't be a subtype of $supertypeString, "
29+
"regardless of whether the nullability modifiers are ignored or not.");
2830
}
2931

3032
/// Checks if a type is a subtype of the other ignoring nullability modifiers.
@@ -257,7 +259,11 @@ abstract class SubtypeTest<T, E> {
257259
isSubtype('Future<num>', 'FutureOr<num?>?');
258260
isSubtype('Future<num?>', 'FutureOr<num?>');
259261
isObliviousSubtype('Future<num?>', 'FutureOr<num>?');
262+
isObliviousSubtype('FutureOr<int>?', 'FutureOr<int>');
263+
isObliviousSubtype('Future<int>?', 'FutureOr<int>');
260264
isSubtype('Future<num?>', 'FutureOr<num?>?');
265+
isSubtype('FutureOr<X>', 'FutureOr<Future<X>>',
266+
typeParameters: 'X extends Future<Future<X>>');
261267

262268
isSubtype('num?', 'FutureOr<FutureOr<FutureOr<num>>?>');
263269
isSubtype('Future<num>?', 'FutureOr<FutureOr<FutureOr<num>>?>');

‎pkg/front_end/test/spell_checking_list_code.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -679,6 +679,7 @@ orders
679679
ordinal
680680
org
681681
orphans
682+
ors
682683
os
683684
overlap
684685
overloader

‎pkg/front_end/test/spell_checking_list_common.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1969,6 +1969,7 @@ opted
19691969
optimistically
19701970
optimization
19711971
optimize
1972+
optimized
19721973
option
19731974
optional
19741975
optionally

‎pkg/kernel/lib/type_environment.dart‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -471,9 +471,37 @@ abstract class SubtypeTester {
471471
var subtypeArg = subtype.typeArguments[0];
472472
if (supertype is InterfaceType &&
473473
identical(supertype.classNode, futureOrClass)) {
474-
var supertypeArg = supertype.typeArguments[0];
475-
// FutureOr<A> <: FutureOr<B> iff A <: B
476-
return performNullabilityAwareSubtypeCheck(subtypeArg, supertypeArg);
474+
DartType supertypeArg = supertype.typeArguments[0];
475+
Nullability subtypeNullability =
476+
computeNullabilityOfFutureOr(subtype, futureOrClass);
477+
Nullability supertypeNullability =
478+
computeNullabilityOfFutureOr(supertype, futureOrClass);
479+
// The following is an optimized is-subtype-of test for the case where
480+
// both LHS and RHS are FutureOrs. It's based on the following:
481+
// FutureOr<X> <: FutureOr<Y> iff X <: Y OR (X <: Future<Y> AND
482+
// Future<X> <: Y).
483+
//
484+
// The correctness of that can be shown as follows:
485+
// 1. FutureOr<X> <: Y iff X <: Y AND Future<X> <: Y
486+
// 2. X <: FutureOr<Y> iff X <: Y OR X <: Future<Y>
487+
// 3. 1,2 => FutureOr<X> <: FutureOr<Y> iff
488+
// (X <: Y OR X <: Future<Y>) AND
489+
// (Future<X> <: Y OR Future<X> <: Future<Y>)
490+
// 4. X <: Y iff Future<X> <: Future<Y>
491+
// 5. 3,4 => FutureOr<X> <: FutureOr<Y> iff
492+
// (X <: Y OR X <: Future<Y>) AND
493+
// (X <: Y OR Future<X> <: Y) iff
494+
// X <: Y OR (X <: Future<Y> AND Future<X> <: Y)
495+
return performNullabilityAwareSubtypeCheck(subtypeArg, supertypeArg)
496+
.or(performNullabilityAwareSubtypeCheck(subtypeArg,
497+
futureType(supertypeArg, Nullability.nonNullable))
498+
.andSubtypeCheckFor(
499+
futureType(subtypeArg, Nullability.nonNullable),
500+
supertypeArg,
501+
this))
502+
.and(new IsSubtypeOf.basedSolelyOnNullabilities(
503+
subtype.withNullability(subtypeNullability),
504+
supertype.withNullability(supertypeNullability)));
477505
}
478506

479507
// given t1 is Future<A> | A, then:
@@ -854,8 +882,8 @@ class _FlatStatefulStaticTypeContext extends StatefulStaticTypeContext {
854882
}
855883
}
856884

857-
/// Implementation of [StatefulStaticTypeContext] that use a stack to change state
858-
/// when entering and leaving libraries and members.
885+
/// Implementation of [StatefulStaticTypeContext] that use a stack to change
886+
/// state when entering and leaving libraries and members.
859887
class _StackedStatefulStaticTypeContext extends StatefulStaticTypeContext {
860888
final List<_StaticTypeContextState> _contextStack =
861889
<_StaticTypeContextState>[];

0 commit comments

Comments
 (0)