Skip to content

Commit cdd2aef

Browse files
stereotype441dart-scoped@luci-project-accounts.iam.gserviceaccount.com
stereotype441
authored and
dart-scoped@luci-project-accounts.iam.gserviceaccount.com
committed
Fix bug in this promotion with switches and if/case.
Promotion of the scrutinees of switch statements, switch expressions, and if/case constructs goes through a slightly different control flow path than other type promotion, to ensure soundness in the case where the scrutinee is a variable that gets modified partway through execution of the switch. When I fisrt implemented `this` promotion, I neglected to update this control flow path. This CL makes the necessary update and adds additional tests to make sure the behavior is correct. Change-Id: I54acad1f40f36304d1448bf1eaa0362b6a6a6964 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/515100 Reviewed-by: Erik Ernst <eernst@google.com> Commit-Queue: Paul Berry <paulberry@google.com> Reviewed-by: Konstantin Shcheglov <scheglov@google.com>
1 parent 83626c2 commit cdd2aef

4 files changed

Lines changed: 261 additions & 0 deletions

File tree

‎pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6475,8 +6475,12 @@ class _FlowAnalysisImpl<
64756475
// even if the underlying variable whose property is being referenced has
64766476
// changed, because the next time the property is accessed, it will be
64776477
// accessed through a new SSA node, and thus a new promotion key).
6478+
//
6479+
// If the scrutinee is `this`, promote it too.
64786480
if (scrutineeReference != null &&
64796481
(scrutineeReference is _PropertyReference ||
6482+
(scrutineeReference.isThisOrSuper &&
6483+
typeAnalyzerOptions.thisPromotionEnabled) ||
64806484
_current.promotionInfo
64816485
?.get(this, matchedValueReference.promotionKey)!
64826486
.ssaNode ==

‎pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5749,6 +5749,77 @@ main() {
57495749
}),
57505750
]);
57515751
});
5752+
5753+
test('switchStatement this promotes', () {
5754+
h.thisType = 'C';
5755+
h.addExhaustiveness('C', false);
5756+
h.addSuperInterfaces('D', (_) => [Type('C'), Type('Object')]);
5757+
h.addSuperInterfaces('C', (_) => [Type('Object')]);
5758+
h.run([
5759+
switch_(this_, [
5760+
wildcard(type: 'D').then([checkPromoted(this_, 'D')]),
5761+
]),
5762+
]);
5763+
});
5764+
5765+
test('switchStatement this does not promote when disabled', () {
5766+
h.disableThisPromotion();
5767+
h.thisType = 'C';
5768+
h.addExhaustiveness('C', false);
5769+
h.addSuperInterfaces('D', (_) => [Type('C'), Type('Object')]);
5770+
h.addSuperInterfaces('C', (_) => [Type('Object')]);
5771+
h.run([
5772+
switch_(this_, [
5773+
wildcard(type: 'D').then([checkNotPromoted(this_)]),
5774+
]),
5775+
]);
5776+
});
5777+
5778+
test('switchExpression this promotes', () {
5779+
h.thisType = 'C';
5780+
h.addExhaustiveness('C', false);
5781+
h.addSuperInterfaces('D', (_) => [Type('C'), Type('Object')]);
5782+
h.addSuperInterfaces('C', (_) => [Type('Object')]);
5783+
h.run([
5784+
switchExpr(this_, [
5785+
wildcard(type: 'D').thenExpr(checkPromoted(this_, 'D')),
5786+
]),
5787+
]);
5788+
});
5789+
5790+
test('switchExpression this does not promote when disabled', () {
5791+
h.disableThisPromotion();
5792+
h.thisType = 'C';
5793+
h.addExhaustiveness('C', false);
5794+
h.addSuperInterfaces('D', (_) => [Type('C'), Type('Object')]);
5795+
h.addSuperInterfaces('C', (_) => [Type('Object')]);
5796+
h.run([
5797+
switchExpr(this_, [
5798+
wildcard(type: 'D').thenExpr(checkNotPromoted(this_)),
5799+
]),
5800+
]);
5801+
});
5802+
5803+
test('ifCase this promotes', () {
5804+
h.thisType = 'C';
5805+
h.addExhaustiveness('C', false);
5806+
h.addSuperInterfaces('D', (_) => [Type('C'), Type('Object')]);
5807+
h.addSuperInterfaces('C', (_) => [Type('Object')]);
5808+
h.run([
5809+
ifCase(this_, wildcard(type: 'D'), [checkPromoted(this_, 'D')]),
5810+
]);
5811+
});
5812+
5813+
test('ifCase this does not promote when disabled', () {
5814+
h.disableThisPromotion();
5815+
h.thisType = 'C';
5816+
h.addExhaustiveness('C', false);
5817+
h.addSuperInterfaces('D', (_) => [Type('C'), Type('Object')]);
5818+
h.addSuperInterfaces('C', (_) => [Type('Object')]);
5819+
h.run([
5820+
ifCase(this_, wildcard(type: 'D'), [checkNotPromoted(this_)]),
5821+
]);
5822+
});
57525823
});
57535824
});
57545825

‎tests/language/this_promotion/this_promotion_switch_disabled_test.dart‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
// Copyright (c) 2026, 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+
// @dart=3.12
6+
7+
// Confirm that `this` does not get promoted when the feature isn't
8+
// enabled. This test specifically covers switch statements, switch expressions,
9+
// and the "if case" construct.
10+
11+
import 'package:expect/static_type_helper.dart';
12+
13+
class A {
14+
void aOnly() {}
15+
}
16+
17+
class ClassTest extends A {
18+
void testClass() {
19+
// 1. Switch statement
20+
switch (this) {
21+
case B():
22+
this.expectStaticType<Exactly<ClassTest>>();
23+
}
24+
25+
// 2. Switch expression
26+
var _ = switch (this) {
27+
B() => <void>[this.expectStaticType<Exactly<ClassTest>>()],
28+
_ => null,
29+
};
30+
31+
// 3. If-case statement
32+
if (this case B()) {
33+
this.expectStaticType<Exactly<ClassTest>>();
34+
}
35+
}
36+
}
37+
38+
mixin M on A {
39+
void testMixin() {
40+
// 1. Switch statement
41+
switch (this) {
42+
case B():
43+
this.expectStaticType<Exactly<M>>();
44+
}
45+
46+
// 2. Switch expression
47+
var _ = switch (this) {
48+
B() => <void>[this.expectStaticType<Exactly<M>>()],
49+
_ => null,
50+
};
51+
52+
// 3. If-case statement
53+
if (this case B()) {
54+
this.expectStaticType<Exactly<M>>();
55+
}
56+
}
57+
}
58+
59+
class B extends ClassTest with M {
60+
void bOnly() {}
61+
}
62+
63+
extension Ext on A {
64+
void testExtension() {
65+
// 1. Switch statement
66+
switch (this) {
67+
case B():
68+
this.expectStaticType<Exactly<A>>();
69+
}
70+
71+
// 2. Switch expression
72+
var _ = switch (this) {
73+
B() => <void>[this.expectStaticType<Exactly<A>>()],
74+
_ => null,
75+
};
76+
77+
// 3. If-case statement
78+
if (this case B()) {
79+
this.expectStaticType<Exactly<A>>();
80+
}
81+
}
82+
}
83+
84+
void main() {
85+
var obj = B();
86+
obj.testClass();
87+
obj.testMixin();
88+
obj.testExtension();
89+
}

‎tests/language/this_promotion/this_promotion_switch_test.dart‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
// Copyright (c) 2026, 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+
// SharedOptions=--enable-experiment=this-promotion
6+
7+
import 'package:expect/static_type_helper.dart';
8+
9+
class A {
10+
void aOnly() {}
11+
}
12+
13+
class ClassTest extends A {
14+
void testClass() {
15+
// 1. Switch statement
16+
switch (this) {
17+
case B():
18+
this.bOnly();
19+
bOnly();
20+
this.expectStaticType<Exactly<B>>();
21+
}
22+
23+
// 2. Switch expression
24+
var _ = switch (this) {
25+
B() => <void>[this.expectStaticType<Exactly<B>>(), bOnly(), this.bOnly()],
26+
_ => null,
27+
};
28+
29+
// 3. If-case statement
30+
if (this case B()) {
31+
this.bOnly();
32+
bOnly();
33+
this.expectStaticType<Exactly<B>>();
34+
}
35+
}
36+
}
37+
38+
mixin M on A {
39+
void testMixin() {
40+
// 1. Switch statement
41+
switch (this) {
42+
case B():
43+
this.bOnly();
44+
bOnly();
45+
this.expectStaticType<Exactly<B>>();
46+
}
47+
48+
// 2. Switch expression
49+
var _ = switch (this) {
50+
B() => <void>[this.expectStaticType<Exactly<B>>(), bOnly(), this.bOnly()],
51+
_ => null,
52+
};
53+
54+
// 3. If-case statement
55+
if (this case B()) {
56+
this.bOnly();
57+
bOnly();
58+
this.expectStaticType<Exactly<B>>();
59+
}
60+
}
61+
}
62+
63+
class B extends ClassTest with M {
64+
void bOnly() {}
65+
}
66+
67+
extension Ext on A {
68+
void testExtension() {
69+
// 1. Switch statement
70+
switch (this) {
71+
case B():
72+
this.bOnly();
73+
bOnly();
74+
this.expectStaticType<Exactly<B>>();
75+
}
76+
77+
// 2. Switch expression
78+
var _ = switch (this) {
79+
B() => <void>[this.expectStaticType<Exactly<B>>(), bOnly(), this.bOnly()],
80+
_ => null,
81+
};
82+
83+
// 3. If-case statement
84+
if (this case B()) {
85+
this.bOnly();
86+
bOnly();
87+
this.expectStaticType<Exactly<B>>();
88+
}
89+
}
90+
}
91+
92+
void main() {
93+
var obj = B();
94+
obj.testClass();
95+
obj.testMixin();
96+
obj.testExtension();
97+
}

0 commit comments

Comments
 (0)