Skip to content

Commit 7e8348f

Browse files
fishythefishcommit-bot@chromium.org
fishythefish
authored and
[email protected]
committed
[dart2js] Assume isPotentialSubtype is always true.
The current implementation of this check is incorrect and produces false negatives. Conservatively assuming that one type is always a potential subtype of another is a simple fix to get this working. The main downsides are that we may emit signatures and provide type arguments to functions even when they're not needed because we no longer statically deduce that a type check must fail. This may cause size regressions. I've added a TODO to add proper constraint solving per the local type inference spec, but this is not currently a priority since no regression is observed on large Google apps. Change-Id: Ie5065596331c33a030e06e66481f258ef937e659 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/155544 Reviewed-by: Stephen Adams <[email protected]> Commit-Queue: Mayank Patke <[email protected]>
1 parent 5671730 commit 7e8348f

17 files changed

+89
-87
lines changed

‎pkg/compiler/lib/src/elements/types.dart

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1915,6 +1915,11 @@ abstract class DartTypes {
19151915

19161916
bool _subtypeHelper(DartType s, DartType t,
19171917
{bool allowPotentialSubtypes: false, bool assumeInstantiations: false}) {
1918+
assert(allowPotentialSubtypes || !assumeInstantiations);
1919+
1920+
// TODO(fishythefish): Add constraint solving for potential subtypes.
1921+
if (allowPotentialSubtypes) return true;
1922+
19181923
/// Based on
19191924
/// https://github.com/dart-lang/language/blob/master/resources/type-system/subtyping.md.
19201925
/// See also [_isSubtype] in `dart:_rti`.
@@ -1927,10 +1932,6 @@ abstract class DartTypes {
19271932
env.isAssumed(s, t)) return true;
19281933

19291934
if (s is AnyType) return true;
1930-
if (allowPotentialSubtypes &&
1931-
(s is TypeVariableType || t is TypeVariableType)) return true;
1932-
if (assumeInstantiations &&
1933-
(s is FunctionTypeVariable || t is FunctionTypeVariable)) return true;
19341935

19351936
// Right Top:
19361937
if (isTopType(t)) return true;
@@ -2039,10 +2040,10 @@ abstract class DartTypes {
20392040
List<FunctionTypeVariable> sTypeVariables = s.typeVariables;
20402041
List<FunctionTypeVariable> tTypeVariables = t.typeVariables;
20412042
int length = tTypeVariables.length;
2042-
if (length == sTypeVariables.length) {
2043-
env ??= _Assumptions();
2044-
env.assumePairs(sTypeVariables, tTypeVariables);
2045-
} else if (!assumeInstantiations || length > 0) return false;
2043+
if (length != sTypeVariables.length) return false;
2044+
2045+
env ??= _Assumptions();
2046+
env.assumePairs(sTypeVariables, tTypeVariables);
20462047
try {
20472048
for (int i = 0; i < length; i++) {
20482049
DartType sBound = sTypeVariables[i].bound;

‎pkg/compiler/test/rti/data/async_local_typed.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ main() async {
1313
// `dynamic Function(dynamic, Class<int>)`, is not a potential subtype and
1414
// therefore doesn't need its signature.
1515

16-
/*spec.needsSignature*/
16+
/*needsSignature*/
1717
local(object, Class<int> stacktrace) => null;
1818

1919
return local;

‎pkg/compiler/test/rti/data/closure_generic_unneeded.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@
55
// @dart = 2.7
66

77
/*spec.class: A:direct,explicit=[A.T*],needsArgs*/
8+
/*prod.class: A:needsArgs*/
89
class A<T> {
910
@pragma('dart2js:noInline')
1011
m() {
11-
return (T t, String s) {};
12+
return /*needsSignature*/(T t, String s) {};
1213
}
1314
}
1415

‎pkg/compiler/test/rti/data/closure_unneeded.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,11 @@
55
// @dart = 2.7
66

77
/*spec.class: A:direct,explicit=[A.T*],needsArgs*/
8+
/*prod.class: A:needsArgs*/
89
class A<T> {
910
@pragma('dart2js:noInline')
1011
m() {
11-
return (T t, String s) {};
12+
return /*needsSignature*/(T t, String s) {};
1213
}
1314
}
1415

‎pkg/compiler/test/rti/data/generic_bounds.dart

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,13 @@ class Class4 {}
4141
method10<T extends Class4>() => null;
4242

4343
main() {
44-
/*spec.needsArgs,selectors=[Selector(call, call, arity=0, types=1)]*/
44+
/*needsArgs,needsSignature,selectors=[Selector(call, call, arity=0, types=1)]*/
4545
method7<T extends Class1a>() => null;
4646

47-
/*spec.needsArgs,selectors=[Selector(call, call, arity=0, types=1)]*/
47+
/*needsArgs,needsSignature,selectors=[Selector(call, call, arity=0, types=1)]*/
4848
method8<T extends Class2a<num>>() => null;
4949

50-
method9<T>() => null;
50+
/*needsArgs,needsSignature,selectors=[Selector(call, call, arity=0, types=1)]*/method9<T>() => null;
5151

5252
dynamic f1 = method1;
5353
dynamic f2 = method2;

‎pkg/compiler/test/rti/data/instantiation3.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class B<S> {
1515
F<S> c;
1616

1717
method() {
18-
return () {
18+
return /*spec.needsSignature*/() {
1919
c = f;
2020
};
2121
}

‎pkg/compiler/test/rti/data/instantiation4.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class B<S> {
1515
F<S> c;
1616

1717
method() {
18-
return () {
18+
return /*spec.needsSignature*/() {
1919
c = f;
2020
};
2121
}

‎pkg/compiler/test/rti/data/local_function_map_literal.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import 'package:expect/expect.dart';
1313
/*spec.member: method:implicit=[method.T],indirect,needsArgs*/
1414
/*prod.member: method:needsArgs*/
1515
method<T>() {
16-
return () => <T, int>{};
16+
return /*spec.needsSignature*/() => <T, int>{};
1717
}
1818

1919
@pragma('dart2js:noInline')

‎pkg/compiler/test/rti/data/local_function_signature2.dart

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,17 @@ import 'package:expect/expect.dart';
88

99
class Class1 {
1010
method1() {
11-
num local<T>(num n) => null;
11+
/*needsArgs,needsSignature*/num local<T>(num n) => null;
1212
return local;
1313
}
1414

1515
method2() {
16-
num local<T>(int n) => null;
16+
/*needsArgs,needsSignature*/num local<T>(int n) => null;
1717
return local;
1818
}
1919

2020
method3() {
21-
int local<T>(num n) => null;
21+
/*needsArgs,needsSignature*/int local<T>(num n) => null;
2222
return local;
2323
}
2424
}
@@ -43,9 +43,10 @@ class Class3 {
4343
}
4444

4545
class Class4 {
46+
/*prod.member: Class4.method6:needsArgs,selectors=[Selector(call, method6, arity=0, types=1)]*/
4647
/*spec.member: Class4.method6:direct,explicit=[method6.T*],needsArgs,selectors=[Selector(call, method6, arity=0, types=1)]*/
4748
method6<T>() {
48-
num local(num n, T t) => null;
49+
/*needsSignature*/num local(num n, T t) => null;
4950
return local;
5051
}
5152
}
@@ -66,25 +67,26 @@ method8<T>() {
6667
}
6768

6869
/*spec.member: method9:direct,explicit=[method9.T*],needsArgs*/
70+
/*prod.member: method9:needsArgs*/
6971
method9<T>() {
70-
num local(num n, T t) => null;
72+
/*needsSignature*/num local(num n, T t) => null;
7173
return local;
7274
}
7375

7476
method10() {
75-
/*spec.direct,explicit=[local.T*],needsArgs*/
76-
num local<T>(T n) => null;
77+
/*spec.direct,explicit=[local.T*],needsArgs,needsSignature*/
78+
/*prod.needsArgs,needsSignature*/num local<T>(T n) => null;
7779
return local;
7880
}
7981

8082
method11() {
81-
T local<T>(num n) => null;
83+
/*needsArgs,needsSignature*/T local<T>(num n) => null;
8284
return local;
8385
}
8486

8587
method12() {
86-
/*spec.direct,explicit=[local.T*],needsArgs*/
87-
num local<T>(num n, T t) => null;
88+
/*spec.direct,explicit=[local.T*],needsArgs,needsSignature*/
89+
/*prod.needsArgs,needsSignature*/num local<T>(num n, T t) => null;
8890
return local;
8991
}
9092

‎pkg/compiler/test/rti/data/local_function_signatures.dart

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@ class Class1 {
1414
}
1515

1616
method2() {
17-
num local(int n) => null;
17+
/*needsSignature*/num local(int n) => null;
1818
return local;
1919
}
2020

2121
method3() {
22-
Object local(num n) => null;
22+
/*needsSignature*/Object local(num n) => null;
2323
return local;
2424
}
2525
}
@@ -44,9 +44,10 @@ class Class3<T> {
4444
}
4545

4646
/*spec.class: Class4:direct,explicit=[Class4.T*],needsArgs*/
47+
/*prod.class: Class4:needsArgs*/
4748
class Class4<T> {
4849
method6() {
49-
num local(num n, T t) => null;
50+
/*needsSignature*/num local(num n, T t) => null;
5051
return local;
5152
}
5253
}

‎pkg/compiler/test/rti/data/local_function_signatures_generic.dart

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ class Class1 {
1414
}
1515

1616
method2() {
17-
num local<T>(int n) => null;
17+
/*needsArgs,needsInst=[<dynamic>,<num*>,<num*>],needsSignature*/num local<T>(int n) => null;
1818
return local;
1919
}
2020

@@ -45,9 +45,10 @@ class Class3 {
4545
}
4646

4747
class Class4 {
48+
/*prod.member: Class4.method6:needsArgs,selectors=[Selector(call, method6, arity=0, types=1)]*/
4849
/*spec.member: Class4.method6:direct,explicit=[method6.T*],needsArgs,selectors=[Selector(call, method6, arity=0, types=1)]*/
4950
method6<T>() {
50-
num local(num n, T t) => null;
51+
/*needsSignature*/num local(num n, T t) => null;
5152
return local;
5253
}
5354
}
@@ -68,8 +69,9 @@ method8<T>() {
6869
}
6970

7071
/*spec.member: method9:direct,explicit=[method9.T*],needsArgs*/
72+
/*prod.member: method9:needsArgs*/
7173
method9<T>() {
72-
num local(num n, T t) => null;
74+
/*needsSignature*/num local(num n, T t) => null;
7375
return local;
7476
}
7577

@@ -87,8 +89,8 @@ method11() {
8789
}
8890

8991
method12() {
90-
/*spec.direct,explicit=[local.T*],needsArgs,needsInst=[<dynamic>,<num*>,<num*>]*/
91-
num local<T>(num n, T t) => null;
92+
/*spec.direct,explicit=[local.T*],needsArgs,needsInst=[<dynamic>,<num*>,<num*>],needsSignature*/
93+
/*prod.needsArgs,needsInst=[<dynamic>,<num*>,<num*>],needsSignature*/num local<T>(num n, T t) => null;
9294
return local;
9395
}
9496

‎pkg/compiler/test/rti/data/subtype_named_args.dart

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,19 +53,19 @@ typedef okWithDynamicFunc_2({int x, bool y, List<Map> z, classesFunc v});
5353

5454
main() {
5555
Expect.isTrue(
56-
/*spec.needsSignature*/
56+
/*needsSignature*/
5757
({D a, B b, C c, A d}) {} is classesFunc);
5858
Expect.isTrue(
5959
/*needsSignature*/
6060
({A a, A b, A c, A d}) {} is classesFunc);
6161
Expect.isTrue(
62-
/*spec.needsSignature*/
62+
/*needsSignature*/
6363
({D a, A1 b, A1 c, A1 d}) {} is classesFunc);
6464
Expect.isTrue(
65-
/*spec.needsSignature*/
65+
/*needsSignature*/
6666
({D a, A2 b, A2 c, A2 d}) {} is classesFunc);
6767
Expect.isTrue(
68-
/*spec.needsSignature*/
68+
/*needsSignature*/
6969
({D a, D b, D c, D d}) {} is classesFunc);
7070
Expect.isTrue(
7171
/*needsSignature*/
@@ -79,7 +79,7 @@ main() {
7979
({Map<num, num> m, List<List<A1>> l, G<A, A1, A1, A1> g}) {}
8080
is genericsFunc);
8181
Expect.isTrue(
82-
/*spec.needsSignature*/
82+
/*needsSignature*/
8383
({Map<int, int> m, List<List<D>> l, G<D, D, D, D> g}) {} is genericsFunc);
8484
Expect.isTrue(
8585
/*needsSignature*/
@@ -89,13 +89,13 @@ main() {
8989
({Object m, Object l, Object g}) {} is genericsFunc);
9090

9191
Expect.isTrue(
92-
/*spec.needsSignature*/
92+
/*needsSignature*/
9393
({A x, G y, mixFunc z, var v}) {} is dynamicFunc);
9494
Expect.isTrue(
95-
/*spec.needsSignature*/
95+
/*needsSignature*/
9696
({int x, bool y, List<Map> z, classesFunc v}) {} is dynamicFunc);
9797

98-
Expect.isTrue((
98+
Expect.isTrue(/*needsSignature*/(
9999
{okWithClassesFunc_1 f1,
100100
okWithGenericsFunc_1 f2,
101101
okWithDynamicFunc_1 f3}) {} is funcFunc);

‎pkg/compiler/test/rti/data/subtype_named_args1.dart

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -40,16 +40,16 @@ main() {
4040
Expect.isTrue(/*needsSignature*/ ({A a}) {} is t1);
4141
Expect.isTrue(/*needsSignature*/ ({B a}) {} is t1);
4242
Expect.isTrue(
43-
/*spec.needsSignature*/ ({C a}) {} is t1);
43+
/*needsSignature*/ ({C a}) {} is t1);
4444
Expect.isTrue(
45-
/*spec.needsSignature*/ ({D a}) {} is t1);
45+
/*needsSignature*/ ({D a}) {} is t1);
4646
Expect.isTrue(/*needsSignature*/ ({Object a}) {} is t1);
4747
Expect.isTrue(/*needsSignature*/ ({var a}) {} is t1);
4848

4949
Expect.isTrue(/*needsSignature*/ ({A c}) {} is t2);
5050
Expect.isTrue(/*needsSignature*/ ({B c}) {} is t2);
5151
Expect.isTrue(/*needsSignature*/ ({C c}) {} is t2);
52-
Expect.isTrue(({D c}) {} is t2);
52+
Expect.isTrue(/*needsSignature*/({D c}) {} is t2);
5353
Expect.isTrue(/*needsSignature*/ ({Object c}) {} is t2);
5454
Expect.isTrue(/*needsSignature*/ ({var c}) {} is t2);
5555

@@ -58,49 +58,49 @@ main() {
5858
Expect.isTrue(/*needsSignature*/ ({Object i}) {} is t3);
5959
Expect.isTrue(/*needsSignature*/ ({var i}) {} is t3);
6060

61-
Expect.isTrue(({A v}) {} is t4);
62-
Expect.isTrue(({B v}) {} is t4);
63-
Expect.isTrue(({C v}) {} is t4);
64-
Expect.isTrue(({D v}) {} is t4);
61+
Expect.isTrue(/*needsSignature*/({A v}) {} is t4);
62+
Expect.isTrue(/*needsSignature*/({B v}) {} is t4);
63+
Expect.isTrue(/*needsSignature*/({C v}) {} is t4);
64+
Expect.isTrue(/*needsSignature*/({D v}) {} is t4);
6565
Expect.isTrue(/*needsSignature*/ ({Object v}) {} is t4);
6666
Expect.isTrue(/*needsSignature*/ ({var v}) {} is t4);
67-
Expect.isTrue(({num v}) {} is t4);
68-
Expect.isTrue(({int v}) {} is t4);
69-
Expect.isTrue(({Map v}) {} is t4);
70-
Expect.isTrue(({Map<List<Map<List, List<int>>>, List> v}) {} is t4);
71-
Expect.isTrue(({List v}) {} is t4);
72-
Expect.isTrue(({t8 v}) {} is t4);
73-
Expect.isTrue(({t7 v}) {} is t4);
67+
Expect.isTrue(/*needsSignature*/({num v}) {} is t4);
68+
Expect.isTrue(/*needsSignature*/({int v}) {} is t4);
69+
Expect.isTrue(/*needsSignature*/({Map v}) {} is t4);
70+
Expect.isTrue(/*needsSignature*/({Map<List<Map<List, List<int>>>, List> v}) {} is t4);
71+
Expect.isTrue(/*needsSignature*/({List v}) {} is t4);
72+
Expect.isTrue(/*needsSignature*/({t8 v}) {} is t4);
73+
Expect.isTrue(/*needsSignature*/({t7 v}) {} is t4);
7474

7575
Expect.isTrue(/*needsSignature*/ ({Map m}) {} is t5);
76-
Expect.isTrue(({Map<List, t8> m}) {} is t5);
76+
Expect.isTrue(/*needsSignature*/({Map<List, t8> m}) {} is t5);
7777
Expect.isTrue(/*needsSignature*/ ({Object m}) {} is t5);
7878
Expect.isTrue(/*needsSignature*/ ({var m}) {} is t5);
79-
Expect.isTrue(({Map<List, List> m}) {} is t5);
80-
Expect.isTrue(({Map<int, t8> m}) {} is t5);
79+
Expect.isTrue(/*needsSignature*/({Map<List, List> m}) {} is t5);
80+
Expect.isTrue(/*needsSignature*/({Map<int, t8> m}) {} is t5);
8181

8282
Expect.isTrue(/*needsSignature*/ ({Map<num, num> m}) {} is t6);
83-
Expect.isTrue(({Map<int, int> m}) {} is t6);
83+
Expect.isTrue(/*needsSignature*/({Map<int, int> m}) {} is t6);
8484
Expect.isTrue(/*needsSignature*/ ({Map m}) {} is t6);
8585
Expect.isTrue(/*needsSignature*/ ({Object m}) {} is t6);
8686
Expect.isTrue(/*needsSignature*/ ({var m}) {} is t6);
8787

88-
Expect.isTrue(({okWithT1_1 f}) {} is t7);
88+
Expect.isTrue(/*needsSignature*/({okWithT1_1 f}) {} is t7);
8989
Expect.isTrue(/*needsSignature*/ ({okWithT1_2 f}) {} is t7);
9090
Expect.isTrue(/*needsSignature*/ ({okWithT1_3 f}) {} is t7);
9191
Expect.isTrue(/*needsSignature*/ ({okWithT1_4 f}) {} is t7);
9292

9393
Expect.isTrue(/*needsSignature*/ ({A a}) {} is t8);
9494
Expect.isTrue(/*needsSignature*/ ({B a}) {} is t8);
9595
Expect.isTrue(
96-
/*spec.needsSignature*/ ({C a}) {} is t8);
96+
/*needsSignature*/ ({C a}) {} is t8);
9797
Expect.isTrue(
98-
/*spec.needsSignature*/ ({D a}) {} is t8);
98+
/*needsSignature*/ ({D a}) {} is t8);
9999
Expect.isTrue(/*needsSignature*/ ({Object a}) {} is t8);
100100
Expect.isTrue(/*needsSignature*/ ({var a}) {} is t8);
101-
Expect.isTrue(({num a}) {} is t8);
102-
Expect.isTrue(({int a}) {} is t8);
103-
Expect.isTrue(({Map a}) {} is t8);
104-
Expect.isTrue(({Map<List<Map<List, List<int>>>, List> a}) {} is t8);
105-
Expect.isTrue(({List a}) {} is t8);
101+
Expect.isTrue(/*needsSignature*/({num a}) {} is t8);
102+
Expect.isTrue(/*needsSignature*/({int a}) {} is t8);
103+
Expect.isTrue(/*needsSignature*/({Map a}) {} is t8);
104+
Expect.isTrue(/*needsSignature*/({Map<List<Map<List, List<int>>>, List> a}) {} is t8);
105+
Expect.isTrue(/*needsSignature*/({List a}) {} is t8);
106106
}

0 commit comments

Comments
 (0)