Skip to content

Commit 6c665c4

Browse files
stereotype441commit-bot@chromium.org
authored andcommitted
Flow analysis: implement "why not promoted" logic for most binary operators.
`&&` and `||` will be addressed in a follow-up CL go through a different code path in the analyzer. Bug: #44898 Change-Id: I20732ceceb113017a0b1e77134b1e28c4c924fbb Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/192607 Reviewed-by: Konstantin Shcheglov <[email protected]>
1 parent 6a585a2 commit 6c665c4

File tree

5 files changed

+66
-10
lines changed

5 files changed

+66
-10
lines changed

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

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4184,10 +4184,13 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
41844184

41854185
@override
41864186
Map<Type, NonPromotionReason> Function() whyNotPromoted(Expression target) {
4187-
ReferenceWithType<Variable, Type>? referenceWithType = _expressionReference;
4188-
if (referenceWithType != null) {
4189-
return referenceWithType.reference.getNonPromotionReasons(
4190-
_current.variableInfo, referenceWithType.type, typeOperations);
4187+
if (identical(target, _expressionWithReference)) {
4188+
ReferenceWithType<Variable, Type>? referenceWithType =
4189+
_expressionReference;
4190+
if (referenceWithType != null) {
4191+
return referenceWithType.reference.getNonPromotionReasons(
4192+
_current.variableInfo, referenceWithType.type, typeOperations);
4193+
}
41914194
}
41924195
return () => {};
41934196
}

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,3 +89,46 @@ C7 constructor_with_explicit_new(C7 c) {
8989
/*analyzer.notPromoted(propertyNotPromoted(target: member:C7.bad, type: int?))*/ c
9090
. /*cfe.notPromoted(propertyNotPromoted(target: member:C7.bad, type: int?))*/ bad);
9191
}
92+
93+
class C8 {
94+
int? bad;
95+
}
96+
97+
userDefinableBinaryOpRhs(C8 c) {
98+
if (c.bad == null) return;
99+
1 +
100+
/*analyzer.notPromoted(propertyNotPromoted(target: member:C8.bad, type: int?))*/ c
101+
. /*cfe.notPromoted(propertyNotPromoted(target: member:C8.bad, type: int?))*/ bad;
102+
}
103+
104+
class C9 {
105+
int? bad;
106+
f(int i) {}
107+
}
108+
109+
questionQuestionRhs(C9 c, int? i) {
110+
// Note: "why not supported" functionality is currently not supported for the
111+
// RHS of `??` because it requires more clever reasoning than we currently do:
112+
// we would have to understand that the reason `i ?? c.bad` has a type of
113+
// `int?` rather than `int` is because `c.bad` was not promoted. We currently
114+
// only support detecting non-promotion when the expression that had the wrong
115+
// type *is* the expression that wasn't promoted.
116+
if (c.bad == null) return;
117+
c.f(i ?? c.bad);
118+
}
119+
120+
class C10 {
121+
D10? bad;
122+
f(bool b) {}
123+
}
124+
125+
class D10 {
126+
bool operator ==(covariant D10 other) => true;
127+
}
128+
129+
equalRhs(C10 c, D10 d) {
130+
if (c.bad == null) return;
131+
// Note: we don't report an error here because `==` always accepts `null`.
132+
c.f(d == c.bad);
133+
c.f(d != c.bad);
134+
}

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ class BinaryExpressionResolver {
108108
var right = node.rightOperand;
109109
right.accept(_resolver);
110110
right = node.rightOperand;
111+
var whyNotPromotedInfo =
112+
_resolver.flowAnalysis?.flow?.whyNotPromoted(right);
111113

112114
if (!leftExtensionOverride) {
113115
flow?.equalityOp_end(node, right, right.typeOrThrow, notEqual: notEqual);
@@ -119,6 +121,9 @@ class BinaryExpressionResolver {
119121
promoteLeftTypeToNonNull: true,
120122
);
121123
_resolveUserDefinableType(node);
124+
_resolver.checkForArgumentTypeNotAssignableForArgument(node.rightOperand,
125+
promoteParameterToNullable: true,
126+
whyNotPromotedInfo: whyNotPromotedInfo);
122127
}
123128

124129
void _resolveIfNull(BinaryExpressionImpl node) {
@@ -154,6 +159,7 @@ class BinaryExpressionResolver {
154159
} else {
155160
_analyzeLeastUpperBoundTypes(node, leftType, rightType);
156161
}
162+
_resolver.checkForArgumentTypeNotAssignableForArgument(right);
157163
}
158164

159165
void _resolveLogicalAnd(BinaryExpressionImpl node) {
@@ -259,8 +265,13 @@ class BinaryExpressionResolver {
259265
}
260266

261267
right.accept(_resolver);
268+
right = node.rightOperand;
269+
var whyNotPromotedInfo =
270+
_resolver.flowAnalysis?.flow?.whyNotPromoted(right);
262271

263272
_resolveUserDefinableType(node);
273+
_resolver.checkForArgumentTypeNotAssignableForArgument(right,
274+
whyNotPromotedInfo: whyNotPromotedInfo);
264275
}
265276

266277
void _resolveUserDefinableElement(

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

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -388,13 +388,8 @@ class ErrorVerifier extends RecursiveAstVisitor<void>
388388
TokenType type = operator.type;
389389
if (type == TokenType.AMPERSAND_AMPERSAND || type == TokenType.BAR_BAR) {
390390
checkForUseOfVoidResult(node.rightOperand);
391-
} else if (type == TokenType.EQ_EQ || type == TokenType.BANG_EQ) {
392-
checkForArgumentTypeNotAssignableForArgument(node.rightOperand,
393-
promoteParameterToNullable: true);
394-
} else if (type != TokenType.QUESTION_QUESTION) {
395-
checkForArgumentTypeNotAssignableForArgument(node.rightOperand);
396391
} else {
397-
checkForArgumentTypeNotAssignableForArgument(node.rightOperand);
392+
// Assignability checking is done by the resolver.
398393
}
399394

400395
if (type == TokenType.QUESTION_QUESTION) {

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1837,6 +1837,10 @@ class ResolverVisitor extends ScopedVisitor with ErrorDetectionHelpers {
18371837
void visitNamedExpression(NamedExpression node) {
18381838
InferenceContext.setTypeFromNode(node.expression, node);
18391839
super.visitNamedExpression(node);
1840+
// Any "why not promoted" information that flow analysis had associated with
1841+
// `node.expression` now needs to be forwarded to `node`, so that when
1842+
// `visitArgumentList` iterates through the arguments, it will find it.
1843+
flowAnalysis?.flow?.forwardExpression(node, node.expression);
18401844
}
18411845

18421846
@override

0 commit comments

Comments
 (0)