Skip to content

Commit 459b2bf

Browse files
stereotype441Commit Queue
authored and
Commit Queue
committed
Flow analysis: implement type promotions for or-patterns.
There are three ways type promotion can occur in an or-pattern: (1) the scrutinee variable (if any) might be promoted, e.g.: f(Object x) { if (x case int _ && < 0 || int _ && > 10) { // `x` is promoted to `int` because both sides of the `||` // promote the scrutinee variable to `int`. } } (2) the implicit temporary variable that holds the matched value might be promoted, e.g.: f(Object Function() g) { if (g() case (int _ && < 0 || int _ && > 10) && (var x)) { // `x` has type `int` because both sides of the `||` promote // the matched value to `int`. } } For this sort of promotion to work, we need to (3) explicitly matched variables might be promoted at the time of the match, e.g.: f<T>(T t) { if (t is int) { if (t case var x && < 0 || var x && > 10) { // `x` has type `T` but is promoted to `T&int`, because both // declarations of `x` are in a context where the matched // value has type `T&int`. } } } The existing flow analysis logic handles cases (1) and (2) without any extra work, because those promotions are joined as a natural consequence of the flow control join at the end of matching the logical-or pattern. However, flow analysis has to do some extra work for case (3), because the two copies of variable `x` are associated with different variable declarations (and hence have different promotion keys). To ensure that the promotions are joined in this case, we need to copy the flow model for the two copies of `x` into a common promotion key prior to doing the flow control join. The bookkeeping necessary to figure out a common promotion key is similar to the bookkeeping necessary to track the association between the individual declared variable patterns and the joined pattern variable (and this is bookkeeping that flow analysis is already doing). So as part of this change I went ahead and removed the `getJoinedVariableComponents` method (which was previously used by flow analysis to query this association). This reduces the constraints on the analyzer and CFE implementations by not requiring them to do this bookkeeping themselves. In the process I've made two additional small changes: - I modified the logic for assigning types and finality to joined variables so that if there is a finality conflict but no type conflict, the common type is used; conversely, if there is a type conflict but no finality conflict, the common finality is used. This should help reduce follow-on errors. - I added logic to ensure that if a variable is only declared on one side or the other of a logical-or, flow analysis still considers that variable to be definitely assigned. This should help reduce follow-on errors. Change-Id: I62f17adb6a51a583707c216ed48d941d1c621eea Bug: #50419 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/279756 Commit-Queue: Paul Berry <[email protected]> Reviewed-by: Johnni Winther <[email protected]>
1 parent da7520e commit 459b2bf

20 files changed

+398
-138
lines changed

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

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,14 @@ abstract class FlowAnalysis<Node extends Object, Statement extends Node,
196196

197197
void constantPattern_end(Expression expression);
198198

199+
/// Copy promotion data associated with one promotion key to another. This
200+
/// is used after analyzing a branch of a logical-or pattern, to move the
201+
/// promotion data associated with the result of a pattern match on the left
202+
/// hand and right hand sides of the logical-or into a common promotion key,
203+
/// so that promotions will be properly unified when the control flow paths
204+
/// are joined.
205+
void copyPromotionData({required int sourceKey, required int destinationKey});
206+
199207
/// Register a declaration of the [variable] in the current state.
200208
/// Should also be called for function parameters.
201209
///
@@ -1092,6 +1100,16 @@ class FlowAnalysisDebug<Node extends Object, Statement extends Node,
10921100
() => _wrapped.constantPattern_end(expression));
10931101
}
10941102

1103+
@override
1104+
void copyPromotionData(
1105+
{required int sourceKey, required int destinationKey}) {
1106+
_wrap(
1107+
'copyPromotionData(sourceKey: $sourceKey, '
1108+
'destinationKey: $destinationKey)',
1109+
() => _wrapped.copyPromotionData(
1110+
sourceKey: sourceKey, destinationKey: destinationKey));
1111+
}
1112+
10951113
@override
10961114
void declare(Variable variable, Type staticType,
10971115
{required bool initialized, bool skipDuplicateCheck = false}) {
@@ -3062,6 +3080,18 @@ class VariableModel<Type extends Object> {
30623080
return new _DemotionResult<Type>(newPromotedTypes, newNonPromotionHistory);
30633081
}
30643082

3083+
/// Returns a variable model that is the same as this one, but with the
3084+
/// variable definitely assigned.
3085+
VariableModel<Type> _setAssigned() => assigned
3086+
? this
3087+
: new VariableModel(
3088+
promotedTypes: promotedTypes,
3089+
tested: tested,
3090+
assigned: true,
3091+
unassigned: false,
3092+
ssaNode: ssaNode ?? new SsaNode(null),
3093+
nonPromotionHistory: nonPromotionHistory);
3094+
30653095
/// Determines whether a variable with the given [promotedTypes] should be
30663096
/// promoted to [writtenType] based on types of interest. If it should,
30673097
/// returns an updated promotion chain; otherwise returns [promotedTypes]
@@ -3630,8 +3660,15 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
36303660
@override
36313661
int assignMatchedPatternVariable(Variable variable, int promotionKey) {
36323662
int mergedKey = promotionKeyStore.keyForVariable(variable);
3633-
_current =
3634-
_current._updateVariableInfo(mergedKey, _current.infoFor(promotionKey));
3663+
VariableModel<Type> info = _current.infoFor(promotionKey);
3664+
// Normally flow analysis is responsible for tracking whether variables are
3665+
// definitely assigned; however for variables appearing in patterns we
3666+
// have other logic to make sure that a value is definitely assigned (e.g.
3667+
// the rule that a variable appearing on one side of an `||` must also
3668+
// appear on the other side). So to avoid reporting redundant errors, we
3669+
// pretend that the variable is definitely assigned, even if it isn't.
3670+
info = info._setAssigned();
3671+
_current = _current._updateVariableInfo(mergedKey, info);
36353672
return mergedKey;
36363673
}
36373674

@@ -3691,6 +3728,13 @@ class _FlowAnalysisImpl<Node extends Object, Statement extends Node,
36913728
_unmatched = _join(_unmatched!, _current);
36923729
}
36933730

3731+
@override
3732+
void copyPromotionData(
3733+
{required int sourceKey, required int destinationKey}) {
3734+
_current = _current._updateVariableInfo(
3735+
destinationKey, _current.infoFor(sourceKey));
3736+
}
3737+
36943738
@override
36953739
void declare(Variable variable, Type staticType,
36963740
{required bool initialized, bool skipDuplicateCheck = false}) {
@@ -5172,6 +5216,10 @@ class _LegacyTypePromotion<Node extends Object, Statement extends Node,
51725216
@override
51735217
void constantPattern_end(Expression expression) {}
51745218

5219+
@override
5220+
void copyPromotionData(
5221+
{required int sourceKey, required int destinationKey}) {}
5222+
51755223
@override
51765224
void declare(Variable variable, Type staticType,
51775225
{required bool initialized, bool skipDuplicateCheck = false}) {}

pkg/_fe_analyzer_shared/lib/src/type_inference/type_analysis_result.dart

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ class MatchContext<Node extends Object, Expression extends Node,
7272
/// in a logical-or pattern.
7373
final Map<String, List<Variable>> componentVariables;
7474

75+
/// For each variable name in the pattern, the promotion key holding the value
76+
/// captured by that variable.
77+
final Map<String, int> patternVariablePromotionKeys;
78+
7579
MatchContext({
7680
Expression? initializer,
7781
this.irrefutableContext,
@@ -81,6 +85,7 @@ class MatchContext<Node extends Object, Expression extends Node,
8185
required this.topPattern,
8286
this.assignedVariables,
8387
required this.componentVariables,
88+
required this.patternVariablePromotionKeys,
8489
}) : _initializer = initializer,
8590
_switchScrutinee = switchScrutinee;
8691

@@ -107,7 +112,23 @@ class MatchContext<Node extends Object, Expression extends Node,
107112
switchScrutinee: _switchScrutinee,
108113
topPattern: topPattern,
109114
componentVariables: componentVariables,
115+
patternVariablePromotionKeys: patternVariablePromotionKeys,
110116
);
117+
118+
/// Returns a modified version of `this`, with a new value of
119+
/// [patternVariablePromotionKeys].
120+
MatchContext<Node, Expression, Pattern, Type, Variable> withPromotionKeys(
121+
Map<String, int> patternVariablePromotionKeys) =>
122+
new MatchContext(
123+
initializer: _initializer,
124+
irrefutableContext: irrefutableContext,
125+
isFinal: isFinal,
126+
isLate: isLate,
127+
switchScrutinee: _switchScrutinee,
128+
topPattern: topPattern,
129+
componentVariables: componentVariables,
130+
patternVariablePromotionKeys: patternVariablePromotionKeys,
131+
);
111132
}
112133

113134
/// Container for the result of running type analysis on an expression that does

0 commit comments

Comments
 (0)