Skip to content

Commit 20d9c6e

Browse files
stereotype441commit-bot@chromium.org
authored andcommitted
Flow analysis: implement "why not promoted" logic for boolean conditions
Bug: #44898 Change-Id: Ia8f3a5ad2971dfb6baeefb0d2f9c6998af0d897e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/192611 Reviewed-by: Konstantin Shcheglov <[email protected]>
1 parent eb37227 commit 20d9c6e

File tree

5 files changed

+158
-10
lines changed

5 files changed

+158
-10
lines changed

pkg/_fe_analyzer_shared/test/flow_analysis/why_not_promoted/data/argument_type_not_assignable_nullability_error.dart

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,3 +164,129 @@ orOperand(C12 c, bool b) {
164164
/*analyzer.notPromoted(propertyNotPromoted(target: member:C12.bad, type: bool?))*/ c
165165
. /*cfe.notPromoted(propertyNotPromoted(target: member:C12.bad, type: bool?))*/ bad);
166166
}
167+
168+
class C13 {
169+
bool? bad;
170+
}
171+
172+
assertStatementCondition(C13 c) {
173+
if (c.bad == null) return;
174+
assert(
175+
/*analyzer.notPromoted(propertyNotPromoted(target: member:C13.bad, type: bool?))*/ c
176+
. /*cfe.notPromoted(propertyNotPromoted(target: member:C13.bad, type: bool?))*/ bad);
177+
}
178+
179+
class C14 {
180+
bool? bad;
181+
C14.assertInitializerCondition(C14 c)
182+
: bad = c.bad!,
183+
assert(
184+
/*analyzer.notPromoted(propertyNotPromoted(target: member:C14.bad, type: bool?))*/ c
185+
. /*cfe.notPromoted(propertyNotPromoted(target: member:C14.bad, type: bool?))*/ bad);
186+
}
187+
188+
class C15 {
189+
bool? bad;
190+
f(bool b) {}
191+
}
192+
193+
notOperand(C15 c) {
194+
if (c.bad == null) return;
195+
c.f(!
196+
/*analyzer.notPromoted(propertyNotPromoted(target: member:C15.bad, type: bool?))*/ c
197+
. /*cfe.notPromoted(propertyNotPromoted(target: member:C15.bad, type: bool?))*/ bad);
198+
}
199+
200+
class C16 {
201+
bool? bad;
202+
f(bool b) {}
203+
}
204+
205+
forLoopCondition(C16 c) {
206+
if (c.bad == null) return;
207+
for (;
208+
/*analyzer.notPromoted(propertyNotPromoted(target: member:C16.bad, type: bool?))*/ c
209+
. /*cfe.notPromoted(propertyNotPromoted(target: member:C16.bad, type: bool?))*/ bad;) {}
210+
[
211+
for (;
212+
/*analyzer.notPromoted(propertyNotPromoted(target: member:C16.bad, type: bool?))*/ c
213+
. /*cfe.notPromoted(propertyNotPromoted(target: member:C16.bad, type: bool?))*/ bad;)
214+
null
215+
];
216+
({
217+
for (;
218+
/*analyzer.notPromoted(propertyNotPromoted(target: member:C16.bad, type: bool?))*/ c
219+
. /*cfe.notPromoted(propertyNotPromoted(target: member:C16.bad, type: bool?))*/ bad;)
220+
null
221+
});
222+
({
223+
for (;
224+
/*analyzer.notPromoted(propertyNotPromoted(target: member:C16.bad, type: bool?))*/ c
225+
. /*cfe.notPromoted(propertyNotPromoted(target: member:C16.bad, type: bool?))*/ bad;)
226+
null: null
227+
});
228+
}
229+
230+
class C17 {
231+
bool? bad;
232+
f(int i) {}
233+
}
234+
235+
conditionalExpressionCondition(C17 c) {
236+
if (c.bad == null) return;
237+
c.f(
238+
/*analyzer.notPromoted(propertyNotPromoted(target: member:C17.bad, type: bool?))*/ c
239+
. /*cfe.notPromoted(propertyNotPromoted(target: member:C17.bad, type: bool?))*/ bad
240+
? 1
241+
: 2);
242+
}
243+
244+
class C18 {
245+
bool? bad;
246+
}
247+
248+
doLoopCondition(C18 c) {
249+
if (c.bad == null) return;
250+
do {} while (
251+
/*analyzer.notPromoted(propertyNotPromoted(target: member:C18.bad, type: bool?))*/ c
252+
. /*cfe.notPromoted(propertyNotPromoted(target: member:C18.bad, type: bool?))*/ bad);
253+
}
254+
255+
class C19 {
256+
bool? bad;
257+
}
258+
259+
ifCondition(C19 c) {
260+
if (c.bad == null) return;
261+
if (
262+
/*analyzer.notPromoted(propertyNotPromoted(target: member:C19.bad, type: bool?))*/ c
263+
. /*cfe.notPromoted(propertyNotPromoted(target: member:C19.bad, type: bool?))*/ bad) {}
264+
[
265+
if (
266+
/*analyzer.notPromoted(propertyNotPromoted(target: member:C19.bad, type: bool?))*/ c
267+
. /*cfe.notPromoted(propertyNotPromoted(target: member:C19.bad, type: bool?))*/ bad)
268+
null
269+
];
270+
({
271+
if (
272+
/*analyzer.notPromoted(propertyNotPromoted(target: member:C19.bad, type: bool?))*/ c
273+
. /*cfe.notPromoted(propertyNotPromoted(target: member:C19.bad, type: bool?))*/ bad)
274+
null
275+
});
276+
({
277+
if (
278+
/*analyzer.notPromoted(propertyNotPromoted(target: member:C19.bad, type: bool?))*/ c
279+
. /*cfe.notPromoted(propertyNotPromoted(target: member:C19.bad, type: bool?))*/ bad)
280+
null: null
281+
});
282+
}
283+
284+
class C20 {
285+
bool? bad;
286+
}
287+
288+
whileCondition(C20 c) {
289+
if (c.bad == null) return;
290+
while (/*analyzer.notPromoted(propertyNotPromoted(target: member:C20.bad, type: bool?))*/ c
291+
. /*cfe.notPromoted(propertyNotPromoted(target: member:C20.bad, type: bool?))*/ bad) {}
292+
}

pkg/analyzer/lib/src/dart/resolver/for_resolver.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,10 @@ class ForResolver {
159159
InferenceContext.setType(condition, _resolver.typeProvider.boolType);
160160
condition.accept(_resolver);
161161
condition = forParts.condition!;
162-
_resolver.boolExpressionVerifier.checkForNonBoolCondition(condition);
162+
var whyNotPromoted =
163+
_resolver.flowAnalysis?.flow?.whyNotPromoted(condition);
164+
_resolver.boolExpressionVerifier
165+
.checkForNonBoolCondition(condition, whyNotPromoted: whyNotPromoted);
163166
}
164167

165168
_resolver.flowAnalysis?.for_bodyBegin(node, condition);

pkg/analyzer/lib/src/dart/resolver/prefix_expression_resolver.dart

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,8 +226,10 @@ class PrefixExpressionResolver {
226226

227227
operand.accept(_resolver);
228228
operand = node.operand;
229+
var whyNotPromoted = _resolver.flowAnalysis?.flow?.whyNotPromoted(operand);
229230

230-
_resolver.boolExpressionVerifier.checkForNonBoolNegationExpression(operand);
231+
_resolver.boolExpressionVerifier.checkForNonBoolNegationExpression(operand,
232+
whyNotPromoted: whyNotPromoted);
231233

232234
_recordStaticType(node, _typeProvider.boolType);
233235

pkg/analyzer/lib/src/error/bool_expression_verifier.dart

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,10 +34,12 @@ class BoolExpressionVerifier {
3434
/// error is reported on the expression.
3535
///
3636
/// See [CompileTimeErrorCode.NON_BOOL_CONDITION].
37-
void checkForNonBoolCondition(Expression condition) {
37+
void checkForNonBoolCondition(Expression condition,
38+
{required Map<DartType, NonPromotionReason> Function()? whyNotPromoted}) {
3839
checkForNonBoolExpression(
3940
condition,
4041
errorCode: CompileTimeErrorCode.NON_BOOL_CONDITION,
42+
whyNotPromoted: whyNotPromoted,
4143
);
4244
}
4345

@@ -46,7 +48,7 @@ class BoolExpressionVerifier {
4648
void checkForNonBoolExpression(Expression expression,
4749
{required ErrorCode errorCode,
4850
List<Object>? arguments,
49-
Map<DartType, NonPromotionReason> Function()? whyNotPromoted}) {
51+
required Map<DartType, NonPromotionReason> Function()? whyNotPromoted}) {
5052
var type = expression.typeOrThrow;
5153
if (!_checkForUseOfVoidResult(expression) &&
5254
!_resolver.typeSystem.isAssignableTo(type, _boolType)) {
@@ -63,10 +65,12 @@ class BoolExpressionVerifier {
6365
}
6466

6567
/// Checks to ensure that the given [expression] is assignable to bool.
66-
void checkForNonBoolNegationExpression(Expression expression) {
68+
void checkForNonBoolNegationExpression(Expression expression,
69+
{required Map<DartType, NonPromotionReason> Function()? whyNotPromoted}) {
6770
checkForNonBoolExpression(
6871
expression,
6972
errorCode: CompileTimeErrorCode.NON_BOOL_NEGATION_EXPRESSION,
73+
whyNotPromoted: whyNotPromoted,
7074
);
7175
}
7276

pkg/analyzer/lib/src/generated/resolver.dart

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -993,6 +993,7 @@ class ResolverVisitor extends ScopedVisitor with ErrorDetectionHelpers {
993993
boolExpressionVerifier.checkForNonBoolExpression(
994994
node.condition,
995995
errorCode: CompileTimeErrorCode.NON_BOOL_EXPRESSION,
996+
whyNotPromoted: flowAnalysis?.flow?.whyNotPromoted(node.condition),
996997
);
997998
flowAnalysis?.flow?.assert_afterCondition(node.condition);
998999
node.message?.accept(this);
@@ -1007,6 +1008,7 @@ class ResolverVisitor extends ScopedVisitor with ErrorDetectionHelpers {
10071008
boolExpressionVerifier.checkForNonBoolExpression(
10081009
node.condition,
10091010
errorCode: CompileTimeErrorCode.NON_BOOL_EXPRESSION,
1011+
whyNotPromoted: flowAnalysis?.flow?.whyNotPromoted(node.condition),
10101012
);
10111013
flowAnalysis?.flow?.assert_afterCondition(node.condition);
10121014
node.message?.accept(this);
@@ -1187,7 +1189,9 @@ class ResolverVisitor extends ScopedVisitor with ErrorDetectionHelpers {
11871189
// TODO(scheglov) Do we need these checks for null?
11881190
condition.accept(this);
11891191
condition = node.condition;
1190-
boolExpressionVerifier.checkForNonBoolCondition(condition);
1192+
var whyNotPromoted = flowAnalysis?.flow?.whyNotPromoted(condition);
1193+
boolExpressionVerifier.checkForNonBoolCondition(condition,
1194+
whyNotPromoted: whyNotPromoted);
11911195

11921196
Expression thenExpression = node.thenExpression;
11931197
InferenceContext.setTypeFromNode(thenExpression, node);
@@ -1349,7 +1353,9 @@ class ResolverVisitor extends ScopedVisitor with ErrorDetectionHelpers {
13491353
InferenceContext.setType(condition, typeProvider.boolType);
13501354
condition.accept(this);
13511355
condition = node.condition;
1352-
boolExpressionVerifier.checkForNonBoolCondition(condition);
1356+
var whyNotPromoted = flowAnalysis?.flow?.whyNotPromoted(condition);
1357+
boolExpressionVerifier.checkForNonBoolCondition(condition,
1358+
whyNotPromoted: whyNotPromoted);
13531359

13541360
flowAnalysis?.flow?.doStatement_end(condition);
13551361
}
@@ -1598,8 +1604,10 @@ class ResolverVisitor extends ScopedVisitor with ErrorDetectionHelpers {
15981604
InferenceContext.setType(condition, typeProvider.boolType);
15991605
condition.accept(this);
16001606
condition = node.condition;
1607+
var whyNotPromoted = flowAnalysis?.flow?.whyNotPromoted(condition);
16011608

1602-
boolExpressionVerifier.checkForNonBoolCondition(condition);
1609+
boolExpressionVerifier.checkForNonBoolCondition(condition,
1610+
whyNotPromoted: whyNotPromoted);
16031611

16041612
CollectionElement thenElement = node.thenElement;
16051613
if (flowAnalysis != null) {
@@ -1637,8 +1645,10 @@ class ResolverVisitor extends ScopedVisitor with ErrorDetectionHelpers {
16371645
InferenceContext.setType(condition, typeProvider.boolType);
16381646
condition.accept(this);
16391647
condition = node.condition;
1648+
var whyNotPromoted = flowAnalysis?.flow?.whyNotPromoted(condition);
16401649

1641-
boolExpressionVerifier.checkForNonBoolCondition(condition);
1650+
boolExpressionVerifier.checkForNonBoolCondition(condition,
1651+
whyNotPromoted: whyNotPromoted);
16421652

16431653
Statement thenStatement = node.thenStatement;
16441654
if (flowAnalysis != null) {
@@ -2172,8 +2182,11 @@ class ResolverVisitor extends ScopedVisitor with ErrorDetectionHelpers {
21722182

21732183
flowAnalysis?.flow?.whileStatement_conditionBegin(node);
21742184
condition.accept(this);
2185+
condition = node.condition;
2186+
var whyNotPromoted = flowAnalysis?.flow?.whyNotPromoted(condition);
21752187

2176-
boolExpressionVerifier.checkForNonBoolCondition(node.condition);
2188+
boolExpressionVerifier.checkForNonBoolCondition(node.condition,
2189+
whyNotPromoted: whyNotPromoted);
21772190

21782191
Statement body = node.body;
21792192
flowAnalysis?.flow?.whileStatement_bodyBegin(node, condition);

0 commit comments

Comments
 (0)