Skip to content

Commit 1026367

Browse files
osa1Commit Queue
osa1
authored and
Commit Queue
committed
[dart2wasm] Fix switch-case comparisons with dynamic scrutinee
Fixes #61098. Issue: #61098 Change-Id: Iabd1b3ffde6e4eafbdf98a2bb04f5949b799fcf6 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/439940 Commit-Queue: Ömer Ağacan <[email protected]> Reviewed-by: Slava Egorov <[email protected]>
1 parent f82c315 commit 1026367

File tree

3 files changed

+273
-9
lines changed

3 files changed

+273
-9
lines changed

‎pkg/dart2wasm/lib/code_generator.dart‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4335,10 +4335,11 @@ class SwitchInfo {
43354335
e is L ||
43364336
e is NullLiteral ||
43374337
(e is ConstantExpression &&
4338-
(e.constant is C || e.constant is NullConstant) &&
4339-
(translator.hierarchy.isSubInterfaceOf(
4340-
translator.classForType(codeGen.dartTypeOf(e)),
4341-
switchExprClass))));
4338+
((e.constant is C &&
4339+
(translator.hierarchy.isSubInterfaceOf(
4340+
translator.classForType(codeGen.dartTypeOf(e)),
4341+
switchExprClass))) ||
4342+
e.constant is NullConstant)));
43424343

43434344
// Type objects should be compared using `==` rather than identity even
43444345
// though the specification is not very clear about it. In language versions
@@ -4367,7 +4368,7 @@ class SwitchInfo {
43674368
nonNullableType = translator.runtimeTypeType;
43684369
nullableType = translator.runtimeTypeTypeNullable;
43694370
compare = (switchExprLocal, pushCaseExpr) {
4370-
// Virtual call to `Object.==`.
4371+
// Virtual call to `Type.==`.
43714372
codeGen._virtualCall(
43724373
node, translator.coreTypes.objectEquals, _VirtualCallKind.Call,
43734374
(functionType) {
@@ -4377,11 +4378,16 @@ class SwitchInfo {
43774378
}, useUncheckedEntry: false);
43784379
};
43794380
} else if (switchExprType is DynamicType) {
4380-
// Object equality switch
4381+
// Per spec, compare with `<case expr> == <switch expr>`. For performance,
4382+
// if we know that the cases all have the same type, we call the case
4383+
// expression's `==` implementation directly (instead of virtually calling
4384+
// `Object.==`).
4385+
//
4386+
// Note: this could be improved by directly calling a different `==` in
4387+
// each of the cases based on the case value. For now we only directly
4388+
// call a `==` if all of the cases have a compatible type.
43814389
nonNullableType = translator.topTypeNonNullable;
43824390
nullableType = translator.topType;
4383-
4384-
// Per spec, compare with `<case expr> == <switch expr>`.
43854391
final Member equalsMember;
43864392
if (check<BoolLiteral, BoolConstant>()) {
43874393
equalsMember = translator.boxedBoolEquals;
@@ -4390,7 +4396,17 @@ class SwitchInfo {
43904396
} else if (check<StringLiteral, StringConstant>()) {
43914397
equalsMember = translator.jsStringEquals;
43924398
} else {
4393-
equalsMember = translator.coreTypes.identicalProcedure;
4399+
compare = (switchExprLocal, pushCaseExpr) {
4400+
// Virtual call to `Object.==`.
4401+
codeGen._virtualCall(node, codeGen.translator.coreTypes.objectEquals,
4402+
_VirtualCallKind.Call, (functionType) {
4403+
codeGen.b.local_get(switchExprLocal);
4404+
}, (functionType, paramInfo) {
4405+
pushCaseExpr();
4406+
}, useUncheckedEntry: false);
4407+
};
4408+
_initializeSpecialCases(node);
4409+
return;
43944410
}
43954411

43964412
final equalsMemberSignature =
@@ -4462,6 +4478,10 @@ class SwitchInfo {
44624478
};
44634479
}
44644480

4481+
_initializeSpecialCases(node);
4482+
}
4483+
4484+
void _initializeSpecialCases(SwitchStatement node) {
44654485
// Special cases
44664486
defaultCase = node.cases
44674487
.cast<SwitchCase?>()

‎tests/language/switch/switch_comparisons_1_test.dart‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// This is a regression test for the SDK issue #61098, originally reported as
6+
// Flutter issue #171803.
7+
//
8+
// Tests that the right comparison method (`identical` or `operator ==`) is used
9+
// when comparing strings in switch statements and expressions.
10+
//
11+
// This is the smaller version of the original repro. More tests in
12+
// `switch_comparisons_2_test.dart`.
13+
14+
import 'dart:convert';
15+
import 'package:expect/expect.dart';
16+
17+
void main() {
18+
_option = utf8.decoder.convert([
19+
80,
20+
104,
21+
111,
22+
116,
23+
111,
24+
103,
25+
114,
26+
97,
27+
112,
28+
104,
29+
105,
30+
99,
31+
66,
32+
111,
33+
120,
34+
]);
35+
36+
Expect.isTrue(infoSwitchCaseWithNull);
37+
Expect.isTrue(infoSwitchCaseWithoutNull);
38+
Expect.isTrue(infoSwitchExpression);
39+
Expect.isTrue(infoIfCase);
40+
Expect.isTrue(infoIfEquals);
41+
}
42+
43+
const kTypeString = 'PhotographicBox';
44+
45+
String? _option;
46+
47+
String? get type => _option;
48+
49+
bool get infoSwitchCaseWithNull {
50+
switch (type) {
51+
case kTypeString:
52+
return true;
53+
case null:
54+
throw 'Type is null on SWITCH CASE WITH NULL';
55+
default:
56+
throw 'Unexpected type on SWITCH CASE WITH NULL: $type';
57+
}
58+
}
59+
60+
bool get infoSwitchCaseWithoutNull {
61+
switch (type) {
62+
case kTypeString:
63+
return true;
64+
default:
65+
throw 'Unexpected type on SWITCH CASE WITHOUT NULL: $type';
66+
}
67+
}
68+
69+
bool get infoSwitchExpression {
70+
return switch (type) {
71+
kTypeString => true,
72+
null => throw 'Type is null with SWITCH EXPRESSION',
73+
_ => throw 'Unexpected type with SWITCH EXPRESSION: $type',
74+
};
75+
}
76+
77+
bool get infoIfCase {
78+
if (type case kTypeString) {
79+
return true;
80+
} else if (type case null) {
81+
throw 'Type is null with IF CASE';
82+
} else {
83+
throw 'Unexpected type with IF CASE: $type';
84+
}
85+
}
86+
87+
bool get infoIfEquals {
88+
if (type == kTypeString) {
89+
return true;
90+
} else if (type == null) {
91+
throw 'Type is null with IF';
92+
} else {
93+
throw 'Unexpected type with IF: $type';
94+
}
95+
}

‎tests/language/switch/switch_comparisons_2_test.dart‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
2+
// for details. All rights reserved. Use of this source code is governed by a
3+
// BSD-style license that can be found in the LICENSE file.
4+
5+
// Tests equalities used when scrutinee is `dynamic` and cases have the same
6+
// type and different types, with and without nulls.
7+
8+
import 'dart:convert';
9+
import 'package:expect/expect.dart';
10+
11+
void main() {
12+
var s = utf8.decoder.convert([
13+
80,
14+
104,
15+
111,
16+
116,
17+
111,
18+
103,
19+
114,
20+
97,
21+
112,
22+
104,
23+
105,
24+
99,
25+
66,
26+
111,
27+
120,
28+
]);
29+
30+
Expect.isTrue(test1(s));
31+
Expect.isTrue(test2(s));
32+
Expect.isTrue(test3(s));
33+
Expect.isTrue(test4(s));
34+
35+
// Test the same thing, but with another special cased type in dart2wasm
36+
// (int).
37+
var i = int.parse('123');
38+
39+
Expect.isTrue(test5(i));
40+
Expect.isTrue(test6(i));
41+
Expect.isTrue(test7(i));
42+
Expect.isTrue(test8(i));
43+
}
44+
45+
const kTypeString = 'PhotographicBox';
46+
47+
// Dynamic scrutinee with mixed type cases.
48+
Object? test1(dynamic v) {
49+
switch (v) {
50+
case 1:
51+
throw 'int case in switch without null';
52+
case kTypeString:
53+
return true;
54+
default:
55+
throw 'default case in switch without null';
56+
}
57+
}
58+
59+
// Dynamic scrutinee with mixed type cases and null.
60+
Object? test2(dynamic v) {
61+
switch (v) {
62+
case 1:
63+
throw 'int case in switch with null';
64+
case null:
65+
throw 'null case';
66+
case kTypeString:
67+
return true;
68+
default:
69+
throw 'default case in switch with null';
70+
}
71+
}
72+
73+
// Dynamic scrutinee with just string cases.
74+
Object? test3(dynamic v) {
75+
switch (v) {
76+
case 'blah':
77+
throw 'string case';
78+
case kTypeString:
79+
return true;
80+
default:
81+
throw 'default case in string switch';
82+
}
83+
}
84+
85+
// Dynamic scrutinee with just string and null cases.
86+
Object? test4(dynamic v) {
87+
switch (v) {
88+
case 'blah':
89+
throw 'string case';
90+
case null:
91+
throw 'null case';
92+
case kTypeString:
93+
return true;
94+
default:
95+
throw 'default case in string switch';
96+
}
97+
}
98+
99+
// Dynamic scrutinee with mixed type cases.
100+
Object? test5(dynamic v) {
101+
switch (v) {
102+
case kTypeString:
103+
throw 'string case';
104+
case 123:
105+
return true;
106+
default:
107+
throw 'default case in switch without null';
108+
}
109+
}
110+
111+
// Dynamic scrutinee with mixed type cases and null.
112+
Object? test6(dynamic v) {
113+
switch (v) {
114+
case kTypeString:
115+
throw 'string case';
116+
case null:
117+
throw 'null case';
118+
case 123:
119+
return true;
120+
default:
121+
throw 'default case in switch with null';
122+
}
123+
}
124+
125+
// Dynamic scrutinee with just int cases.
126+
Object? test7(dynamic v) {
127+
switch (v) {
128+
case 0:
129+
throw 'wrong int';
130+
case 123:
131+
return true;
132+
default:
133+
throw 'default case in string switch';
134+
}
135+
}
136+
137+
// Dynamic scrutinee with just int and null cases.
138+
Object? test8(dynamic v) {
139+
switch (v) {
140+
case 0:
141+
throw 'wrong int';
142+
case null:
143+
throw 'null case';
144+
case 123:
145+
return true;
146+
default:
147+
throw 'default case in string switch';
148+
}
149+
}

0 commit comments

Comments
 (0)