Flow analysis: Replace `merge` with `join` followed by `unsplit`. The `merge` operation is equivalent to a `join` operation followed by an `unsplit` operation, so there is no real benefit to having both `merge` and `join` as separate methods. Removing `merge` will make it easier to refactor the behavior of `join` in follow-up CLs. Change-Id: I4a42cdd1cb2e795dcfeae86703fe5d3356c131ce Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/322140 Commit-Queue: Paul Berry <paulberry@google.com> Reviewed-by: Phil Quitslund <pquitslund@google.com>
diff --git a/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart b/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart index 35a967c..4a0316f 100644 --- a/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart +++ b/pkg/_fe_analyzer_shared/lib/src/flow_analysis/flow_analysis.dart
@@ -2674,40 +2674,6 @@ return newPromotionInfo; } - /// Models the result of joining the flow models [first] and [second] at the - /// merge of two control flow paths. - static FlowModel<Type> merge<Type extends Object>( - FlowModelHelper<Type> helper, - FlowModel<Type>? first, - FlowModel<Type>? second, - ) { - if (first == null) return second!.unsplit(); - if (second == null) return first.unsplit(); - - assert(identical(first.reachable.parent, second.reachable.parent)); - if (first.reachable.locallyReachable && - !second.reachable.locallyReachable) { - return first.unsplit(); - } - if (!first.reachable.locallyReachable && - second.reachable.locallyReachable) { - return second.unsplit(); - } - - // first.reachable and second.reachable are equivalent, so we don't need to - // join reachabilities. - assert( - first.reachable.locallyReachable == second.reachable.locallyReachable); - assert(first.reachable.parent == second.reachable.parent); - Reachability newReachable = first.reachable.unsplit(); - Map<int, PromotionModel<Type>> newPromotionInfo = - FlowModel.joinPromotionInfo( - helper, first.promotionInfo, second.promotionInfo); - - return FlowModel._identicalOrNew( - first, second, newReachable, newPromotionInfo); - } - /// Creates a new [FlowModel] object, unless it is equivalent to either /// [first] or [second], in which case one of those objects is re-used. static FlowModel<Type> _identicalOrNew<Type extends Object>( @@ -4139,7 +4105,8 @@ @override void assert_end() { _AssertContext<Type> context = _stack.removeLast() as _AssertContext<Type>; - _current = _merge(context._previous, context._conditionInfo!.ifTrue); + _current = + _join(context._previous, context._conditionInfo!.ifTrue).unsplit(); } @override @@ -4255,9 +4222,9 @@ conditionalExpression, new ExpressionInfo( type: conditionalExpressionType, - after: _merge(thenInfo.after, elseInfo.after), - ifTrue: _merge(thenInfo.ifTrue, elseInfo.ifTrue), - ifFalse: _merge(thenInfo.ifFalse, elseInfo.ifFalse))); + after: _join(thenInfo.after, elseInfo.after).unsplit(), + ifTrue: _join(thenInfo.ifTrue, elseInfo.ifTrue).unsplit(), + ifFalse: _join(thenInfo.ifFalse, elseInfo.ifFalse).unsplit())); } @override @@ -4345,8 +4312,9 @@ void doStatement_end(Expression condition) { _BranchTargetContext<Type> context = _stack.removeLast() as _BranchTargetContext<Type>; - _current = _merge( - _expressionEnd(condition, boolType).ifFalse, context._breakModel); + _current = + _join(_expressionEnd(condition, boolType).ifFalse, context._breakModel) + .unsplit(); } @override @@ -4448,8 +4416,9 @@ FlowModel<Type>? breakState = context._breakModel; FlowModel<Type> falseCondition = context._conditionInfo.ifFalse; - _current = - _merge(falseCondition, breakState).inheritTested(operations, _current); + _current = _join(falseCondition, breakState) + .inheritTested(operations, _current) + .unsplit(); } @override @@ -4471,7 +4440,7 @@ void forEach_end() { _SimpleStatementContext<Type> context = _stack.removeLast() as _SimpleStatementContext<Type>; - _current = _merge(_current, context._previous); + _current = _join(_current, context._previous).unsplit(); } @override @@ -4557,7 +4526,7 @@ void ifNullExpression_end() { _IfNullExpressionContext<Type> context = _stack.removeLast() as _IfNullExpressionContext<Type>; - _current = _merge(_current, context._shortcutState); + _current = _join(_current, context._shortcutState).unsplit(); } @override @@ -4616,7 +4585,7 @@ afterThen = _current; // no `else`, so `then` is still current afterElse = context._branchModel; } - _current = _merge(afterThen, afterElse); + _current = _join(afterThen, afterElse).unsplit(); } @override @@ -4686,7 +4655,7 @@ void labeledStatement_end() { _BranchTargetContext<Type> context = _stack.removeLast() as _BranchTargetContext<Type>; - _current = _merge(_current, context._breakModel); + _current = _join(_current, context._breakModel).unsplit(); } @override @@ -4733,7 +4702,7 @@ wholeExpression, new ExpressionInfo( type: boolType, - after: _merge(trueResult, falseResult), + after: _join(trueResult, falseResult).unsplit(), ifTrue: trueResult.unsplit(), ifFalse: falseResult.unsplit())); } @@ -4813,7 +4782,7 @@ void nullAwareAccess_end() { _NullAwareAccessContext<Type> context = _stack.removeLast() as _NullAwareAccessContext<Type>; - _current = _merge(_current, context._previous); + _current = _join(_current, context._previous).unsplit(); } @override @@ -5242,7 +5211,8 @@ @override void whileStatement_end() { _WhileContext<Type> context = _stack.removeLast() as _WhileContext<Type>; - _current = _merge(context._conditionInfo.ifFalse, context._breakModel) + _current = _join(context._conditionInfo.ifFalse, context._breakModel) + .unsplit() .inheritTested(operations, _current); } @@ -5651,9 +5621,6 @@ ssaNode: ssaNode); } - FlowModel<Type> _merge(FlowModel<Type> first, FlowModel<Type>? second) => - FlowModel.merge(this, first, second); - /// Computes an updated flow model representing the result of a null check /// performed by a pattern. The returned flow model represents what is known /// about the program state if the matched value is determined to be not equal
diff --git a/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart b/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart index 394c710..795b7cd 100644 --- a/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart +++ b/pkg/_fe_analyzer_shared/test/flow_analysis/flow_analysis_test.dart
@@ -4752,97 +4752,6 @@ }); }); - group('merge', () { - late int x; - var intType = Type('int'); - var stringType = Type('String'); - const emptyMap = const <int, PromotionModel<Type>>{}; - - setUp(() { - x = h.promotionKeyStore.keyForVariable(Var('x')..type = Type('Object?')); - }); - - PromotionModel<Type> varModel(List<Type>? promotionChain, - {bool assigned = false}) => - PromotionModel<Type>( - promotedTypes: promotionChain, - tested: promotionChain ?? [], - assigned: assigned, - unassigned: !assigned, - ssaNode: new SsaNode<Type>(null)); - - test('first is null', () { - var s1 = FlowModel.withInfo(Reachability.initial.split(), emptyMap); - var result = FlowModel.merge(h, null, s1); - expect(result.reachable, same(Reachability.initial)); - }); - - test('second is null', () { - var splitPoint = Reachability.initial.split(); - var afterSplit = splitPoint.split(); - var s1 = FlowModel.withInfo(afterSplit, emptyMap); - var result = FlowModel.merge(h, s1, null); - expect(result.reachable, same(splitPoint)); - }); - - test('both are reachable', () { - var splitPoint = Reachability.initial.split(); - var afterSplit = splitPoint.split(); - var s1 = FlowModel.withInfo(afterSplit, { - x: varModel([intType]) - }); - var s2 = FlowModel.withInfo(afterSplit, { - x: varModel([stringType]) - }); - var result = FlowModel.merge(h, s1, s2); - expect(result.reachable, same(splitPoint)); - expect(result.promotionInfo[x]!.promotedTypes, isNull); - }); - - test('first is unreachable', () { - var splitPoint = Reachability.initial.split(); - var afterSplit = splitPoint.split(); - var s1 = FlowModel.withInfo(afterSplit.setUnreachable(), { - x: varModel([intType]) - }); - var s2 = FlowModel.withInfo(afterSplit, { - x: varModel([stringType]) - }); - var result = FlowModel.merge(h, s1, s2); - expect(result.reachable, same(splitPoint)); - expect(result.promotionInfo, same(s2.promotionInfo)); - }); - - test('second is unreachable', () { - var splitPoint = Reachability.initial.split(); - var afterSplit = splitPoint.split(); - var s1 = FlowModel.withInfo(afterSplit, { - x: varModel([intType]) - }); - var s2 = FlowModel.withInfo(afterSplit.setUnreachable(), { - x: varModel([stringType]) - }); - var result = FlowModel.merge(h, s1, s2); - expect(result.reachable, same(splitPoint)); - expect(result.promotionInfo, same(s1.promotionInfo)); - }); - - test('both are unreachable', () { - var splitPoint = Reachability.initial.split(); - var afterSplit = splitPoint.split(); - var s1 = FlowModel.withInfo(afterSplit.setUnreachable(), { - x: varModel([intType]) - }); - var s2 = FlowModel.withInfo(afterSplit.setUnreachable(), { - x: varModel([stringType]) - }); - var result = FlowModel.merge(h, s1, s2); - expect(result.reachable.locallyReachable, false); - expect(result.reachable.parent, same(splitPoint.parent)); - expect(result.promotionInfo[x]!.promotedTypes, isNull); - }); - }); - group('inheritTested', () { late int x; var intType = Type('int');