Split LocalsHandler.mergeDiamondFlow into mergeFlow and mergeDiamondFlow.

Change-Id: Ic3e512a2abc67bf3c79c4c2f36f5e0b5e298c4aa
Reviewed-on: https://dart-review.googlesource.com/c/85285
Commit-Queue: Johnni Winther <johnniwinther@google.com>
Auto-Submit: Johnni Winther <johnniwinther@google.com>
Reviewed-by: Sigmund Cherem <sigmund@google.com>
diff --git a/pkg/compiler/lib/src/inferrer/builder_kernel.dart b/pkg/compiler/lib/src/inferrer/builder_kernel.dart
index 447e6a2..3009a68 100644
--- a/pkg/compiler/lib/src/inferrer/builder_kernel.dart
+++ b/pkg/compiler/lib/src/inferrer/builder_kernel.dart
@@ -49,7 +49,7 @@
   final bool _inGenerativeConstructor;
 
   LocalsHandler _locals;
-  FieldInitializationScope _fields;
+  FieldInitializationScope _fieldScope;
   final SideEffectsBuilder _sideEffectsBuilder;
   final Map<JumpTarget, List<LocalsHandler>> _breaksFor =
       <JumpTarget, List<LocalsHandler>>{};
@@ -71,7 +71,9 @@
 
   KernelTypeGraphBuilder(this._options, this._closedWorld, this._inferrer,
       this._analyzedMember, this._analyzedNode, this._localsMap,
-      [this._locals, this._fields, Map<Local, FieldEntity> capturedAndBoxed])
+      [this._locals,
+      this._fieldScope,
+      Map<Local, FieldEntity> capturedAndBoxed])
       : this._types = _inferrer.types,
         this._memberData = _inferrer.dataOfMember(_analyzedMember),
         // TODO(johnniwinther): Should side effects also be tracked for field
@@ -86,7 +88,8 @@
             : <Local, FieldEntity>{} {
     if (_locals != null) return;
 
-    _fields = _inGenerativeConstructor ? new FieldInitializationScope() : null;
+    _fieldScope =
+        _inGenerativeConstructor ? new FieldInitializationScope() : null;
     _locals = new LocalsHandler(_analyzedNode);
   }
 
@@ -99,12 +102,12 @@
   bool get inLoop => _loopLevel > 0;
 
   bool get _isThisExposed {
-    return _inGenerativeConstructor ? _fields.isThisExposed : true;
+    return _inGenerativeConstructor ? _fieldScope.isThisExposed : true;
   }
 
   void _markThisAsExposed() {
     if (_inGenerativeConstructor) {
-      _fields.isThisExposed = true;
+      _fieldScope.isThisExposed = true;
     }
   }
 
@@ -141,7 +144,7 @@
           if (!selector.isSetter &&
               _isInClassOrSubclass(field) &&
               field.isAssignable &&
-              _fields.readField(field) == null &&
+              _fieldScope.readField(field) == null &&
               getFieldInitializer(_elementMap, field) == null) {
             // If the field is being used before this constructor
             // actually had a chance to initialize it, say it can be
@@ -198,7 +201,7 @@
 
   void initializationIsIndefinite() {
     if (_inGenerativeConstructor) {
-      _fields.isIndefinite = true;
+      _fieldScope.isIndefinite = true;
     }
   }
 
@@ -257,7 +260,7 @@
       _elementMap.elementEnvironment.forEachLocalClassMember(cls,
           (MemberEntity member) {
         if (member.isField && member.isInstanceMember && member.isAssignable) {
-          TypeInformation type = _fields.readField(member);
+          TypeInformation type = _fieldScope.readField(member);
           MemberDefinition definition = _elementMap.getMemberDefinition(member);
           assert(definition.kind == MemberKind.regular);
           ir.Field node = definition.node;
@@ -291,7 +294,7 @@
   visitFieldInitializer(ir.FieldInitializer node) {
     TypeInformation rhsType = visit(node.value);
     FieldEntity field = _elementMap.getField(node.field);
-    _fields.updateField(field, rhsType);
+    _fieldScope.updateField(field, rhsType);
     _inferrer.recordTypeOfField(field, rhsType);
     return null;
   }
@@ -456,24 +459,24 @@
     bool simpleCondition =
         handleCondition(node.condition, positiveTests, negativeTests);
     LocalsHandler localsBefore = _locals;
-    FieldInitializationScope fieldScopeBefore = _fields;
+    FieldInitializationScope fieldScopeBefore = _fieldScope;
 
     _locals = new LocalsHandler.from(localsBefore, node);
     _updateIsChecks(positiveTests, negativeTests);
     LocalsHandler localsAfterCondition = _locals;
-    FieldInitializationScope fieldScopeAfterThen = _fields;
+    FieldInitializationScope fieldScopeAfterThen = _fieldScope;
 
     _locals = new LocalsHandler.from(localsBefore, node);
-    _fields = new FieldInitializationScope.from(fieldScopeBefore);
+    _fieldScope = new FieldInitializationScope.from(fieldScopeBefore);
     if (simpleCondition) _updateIsChecks(negativeTests, positiveTests);
     visit(node.message);
 
     LocalsHandler localsAfterMessage = _locals;
-    FieldInitializationScope fieldScopeAfterMessage = _fields;
+    FieldInitializationScope fieldScopeAfterMessage = _fieldScope;
     localsAfterMessage.seenReturnOrThrow = true;
     _locals = localsBefore.mergeDiamondFlow(
         _inferrer, localsAfterCondition, localsAfterMessage);
-    _fields = fieldScopeBefore?.mergeDiamondFlow(
+    _fieldScope = fieldScopeBefore?.mergeDiamondFlow(
         _inferrer, fieldScopeAfterThen, fieldScopeAfterMessage);
     return null;
   }
@@ -535,15 +538,15 @@
         changed = false;
         for (ir.SwitchCase switchCase in node.cases) {
           LocalsHandler localsBeforeCase = _locals;
-          FieldInitializationScope fieldScopeBeforeCase = _fields;
+          FieldInitializationScope fieldScopeBeforeCase = _fieldScope;
           _locals = new LocalsHandler.from(localsBeforeCase, switchCase);
-          _fields = new FieldInitializationScope.from(fieldScopeBeforeCase);
+          _fieldScope = new FieldInitializationScope.from(fieldScopeBeforeCase);
           visit(switchCase);
           LocalsHandler localsAfterCase = _locals;
           changed = localsBeforeCase.mergeAll(_inferrer, [localsAfterCase]) ||
               changed;
           _locals = localsBeforeCase;
-          _fields = fieldScopeBeforeCase;
+          _fieldScope = fieldScopeBeforeCase;
         }
       } while (changed);
       _locals.endLoop(_inferrer, node);
@@ -551,7 +554,7 @@
       continueTargets.forEach(_clearBreaksAndContinues);
     } else {
       LocalsHandler localsBeforeCase = _locals;
-      FieldInitializationScope fieldScopeBeforeCase = _fields;
+      FieldInitializationScope fieldScopeBeforeCase = _fieldScope;
       List<LocalsHandler> localsToMerge = <LocalsHandler>[];
       bool hasDefaultCase = false;
 
@@ -566,7 +569,7 @@
       localsBeforeCase.mergeAfterBreaks(_inferrer, localsToMerge,
           keepOwnLocals: !hasDefaultCase);
       _locals = localsBeforeCase;
-      _fields = fieldScopeBeforeCase;
+      _fieldScope = fieldScopeBeforeCase;
     }
     _clearBreaksAndContinues(jumpTarget);
     return null;
@@ -1029,14 +1032,14 @@
     _loopLevel++;
     bool changed = false;
     LocalsHandler localsBefore = _locals;
-    FieldInitializationScope fieldScopeBefore = _fields;
+    FieldInitializationScope fieldScopeBefore = _fieldScope;
     localsBefore.startLoop(_inferrer, node);
     do {
       // Setup (and clear in case of multiple iterations of the loop)
       // the lists of breaks and continues seen in the loop.
       _setupBreaksAndContinues(target);
       _locals = new LocalsHandler.from(localsBefore, node);
-      _fields = new FieldInitializationScope.from(fieldScopeBefore);
+      _fieldScope = new FieldInitializationScope.from(fieldScopeBefore);
       logic();
       changed = localsBefore.mergeAll(_inferrer, _getLoopBackEdges(target));
     } while (changed);
@@ -1045,7 +1048,7 @@
     bool keepOwnLocals = node is! ir.DoStatement;
     _locals = localsBefore.mergeAfterBreaks(_inferrer, _getBreaks(target),
         keepOwnLocals: keepOwnLocals);
-    _fields = fieldScopeBefore;
+    _fieldScope = fieldScopeBefore;
     _clearBreaksAndContinues(target);
     return null;
   }
@@ -1311,7 +1314,7 @@
           MemberEntity single = targets.first;
           if (single.isField) {
             FieldEntity field = single;
-            _fields.updateField(field, rhsType);
+            _fieldScope.updateField(field, rhsType);
           }
         }
       }
@@ -1397,26 +1400,26 @@
     bool simpleCondition =
         handleCondition(node.condition, positiveTests, negativeTests);
     LocalsHandler localsBefore = _locals;
-    FieldInitializationScope fieldScopeBefore = _fields;
+    FieldInitializationScope fieldScopeBefore = _fieldScope;
     _locals = new LocalsHandler.from(localsBefore, node);
-    _fields = new FieldInitializationScope.from(fieldScopeBefore);
+    _fieldScope = new FieldInitializationScope.from(fieldScopeBefore);
     _updateIsChecks(positiveTests, negativeTests);
     visit(node.then);
 
     LocalsHandler localsAfterThen = _locals;
-    FieldInitializationScope fieldScopeAfterThen = _fields;
+    FieldInitializationScope fieldScopeAfterThen = _fieldScope;
     _locals = new LocalsHandler.from(localsBefore, node);
-    _fields = new FieldInitializationScope.from(fieldScopeBefore);
+    _fieldScope = new FieldInitializationScope.from(fieldScopeBefore);
     if (simpleCondition) {
       _updateIsChecks(negativeTests, positiveTests);
     }
     visit(node.otherwise);
     LocalsHandler localsAfterElse = _locals;
-    FieldInitializationScope fieldScopeAfterElse = _fields;
+    FieldInitializationScope fieldScopeAfterElse = _fieldScope;
 
     _locals = localsBefore.mergeDiamondFlow(
         _inferrer, localsAfterThen, localsAfterElse);
-    _fields = fieldScopeBefore?.mergeDiamondFlow(
+    _fieldScope = fieldScopeBefore?.mergeDiamondFlow(
         _inferrer, fieldScopeAfterThen, fieldScopeAfterElse);
     return null;
   }
@@ -1454,9 +1457,9 @@
       }
       visit(node.left, conditionContext: _accumulateIsChecks);
       LocalsHandler localsBefore = _locals;
-      FieldInitializationScope fieldScopeBefore = _fields;
+      FieldInitializationScope fieldScopeBefore = _fieldScope;
       _locals = new LocalsHandler.from(localsBefore, node);
-      _fields = new FieldInitializationScope.from(fieldScopeBefore);
+      _fieldScope = new FieldInitializationScope.from(fieldScopeBefore);
       _updateIsChecks(_positiveIsChecks, _negativeIsChecks);
 
       LocalsHandler narrowed;
@@ -1480,8 +1483,8 @@
         _positiveIsChecks.removeWhere(invalidatedInRightHandSide);
         _negativeIsChecks.removeWhere(invalidatedInRightHandSide);
       }
-      _locals = localsBefore.mergeDiamondFlow(_inferrer, _locals, null);
-      _fields = fieldScopeBefore?.mergeDiamondFlow(_inferrer, _fields, null);
+      _locals = localsBefore.mergeFlow(_inferrer, _locals);
+      _fieldScope = fieldScopeBefore;
       return _types.boolType;
     } else if (node.operator == '||') {
       _conditionIsSimple = false;
@@ -1490,15 +1493,15 @@
       bool isSimple =
           handleCondition(node.left, positiveIsChecks, negativeIsChecks);
       LocalsHandler localsBefore = _locals;
-      FieldInitializationScope fieldScopeBefore = _fields;
+      FieldInitializationScope fieldScopeBefore = _fieldScope;
       _locals = new LocalsHandler.from(_locals, node);
-      _fields = new FieldInitializationScope.from(fieldScopeBefore);
+      _fieldScope = new FieldInitializationScope.from(fieldScopeBefore);
       if (isSimple) {
         _updateIsChecks(negativeIsChecks, positiveIsChecks);
       }
       visit(node.right, conditionContext: false);
-      _locals = localsBefore.mergeDiamondFlow(_inferrer, _locals, null);
-      _fields = fieldScopeBefore?.mergeDiamondFlow(_inferrer, _fields, null);
+      _locals = localsBefore.mergeFlow(_inferrer, _locals);
+      _fieldScope = fieldScopeBefore;
       return _types.boolType;
     }
     failedAt(CURRENT_ELEMENT_SPANNABLE,
@@ -1514,21 +1517,21 @@
         handleCondition(node.condition, positiveTests, negativeTests);
     LocalsHandler localsBefore = _locals;
     _locals = new LocalsHandler.from(localsBefore, node);
-    FieldInitializationScope fieldScopeBefore = _fields;
-    _fields = new FieldInitializationScope.from(fieldScopeBefore);
+    FieldInitializationScope fieldScopeBefore = _fieldScope;
+    _fieldScope = new FieldInitializationScope.from(fieldScopeBefore);
     _updateIsChecks(positiveTests, negativeTests);
     TypeInformation firstType = visit(node.then);
     LocalsHandler localsAfterThen = _locals;
-    FieldInitializationScope fieldScopeAfterThen = _fields;
+    FieldInitializationScope fieldScopeAfterThen = _fieldScope;
     _locals = new LocalsHandler.from(localsBefore, node);
-    _fields = new FieldInitializationScope.from(fieldScopeBefore);
+    _fieldScope = new FieldInitializationScope.from(fieldScopeBefore);
     if (simpleCondition) _updateIsChecks(negativeTests, positiveTests);
     TypeInformation secondType = visit(node.otherwise);
     LocalsHandler localsAfterElse = _locals;
-    FieldInitializationScope fieldScopeAfterElse = _fields;
+    FieldInitializationScope fieldScopeAfterElse = _fieldScope;
     _locals = localsBefore.mergeDiamondFlow(
         _inferrer, localsAfterThen, localsAfterElse);
-    _fields = fieldScopeBefore?.mergeDiamondFlow(
+    _fieldScope = fieldScopeBefore?.mergeDiamondFlow(
         _inferrer, fieldScopeAfterThen, fieldScopeAfterElse);
     return _types.allocateDiamondPhi(firstType, secondType);
   }
@@ -1580,7 +1583,7 @@
     LocalsHandler closureLocals =
         new LocalsHandler.from(_locals, node, useOtherTryBlock: false);
     FieldInitializationScope closureFieldScope =
-        new FieldInitializationScope.from(_fields);
+        new FieldInitializationScope.from(_fieldScope);
     KernelTypeGraphBuilder visitor = new KernelTypeGraphBuilder(
         _options,
         _closedWorld,
@@ -1653,30 +1656,24 @@
   @override
   visitTryCatch(ir.TryCatch node) {
     LocalsHandler localsBefore = _locals;
-    FieldInitializationScope fieldScopeBefore = _fields;
+    FieldInitializationScope fieldScopeBefore = _fieldScope;
     _locals = new LocalsHandler.from(localsBefore, node,
         isTry: true, useOtherTryBlock: false);
-    _fields = new FieldInitializationScope.from(fieldScopeBefore);
+    _fieldScope = new FieldInitializationScope.from(fieldScopeBefore);
     initializationIsIndefinite();
     visit(node.body);
     LocalsHandler localsAfterBody = _locals;
-    FieldInitializationScope fieldScopeAfterBody = _fields;
-
-    _locals = localsBefore.mergeDiamondFlow(_inferrer, localsAfterBody, null);
-    _fields = fieldScopeBefore?.mergeDiamondFlow(
-        _inferrer, fieldScopeAfterBody, null);
+    _locals = localsBefore.mergeFlow(_inferrer, localsAfterBody);
+    _fieldScope = fieldScopeBefore;
     for (ir.Catch catchBlock in node.catches) {
       LocalsHandler localsBeforeCatch = _locals;
-      FieldInitializationScope fieldScopeBeforeCatch = _fields;
+      FieldInitializationScope fieldScopeBeforeCatch = _fieldScope;
       _locals = new LocalsHandler.from(localsBeforeCatch, catchBlock);
-      _fields = new FieldInitializationScope.from(fieldScopeBeforeCatch);
+      _fieldScope = new FieldInitializationScope.from(fieldScopeBeforeCatch);
       visit(catchBlock);
       LocalsHandler localsAfterCatch = _locals;
-      FieldInitializationScope fieldScopeAfterCatch = _fields;
-      _locals =
-          localsBeforeCatch.mergeDiamondFlow(_inferrer, localsAfterCatch, null);
-      _fields = fieldScopeBeforeCatch?.mergeDiamondFlow(
-          _inferrer, fieldScopeAfterCatch, null);
+      _locals = localsBeforeCatch.mergeFlow(_inferrer, localsAfterCatch);
+      _fieldScope = fieldScopeBeforeCatch;
     }
     return null;
   }
@@ -1684,17 +1681,15 @@
   @override
   visitTryFinally(ir.TryFinally node) {
     LocalsHandler localsBefore = _locals;
-    FieldInitializationScope fieldScopeBefore = _fields;
+    FieldInitializationScope fieldScopeBefore = _fieldScope;
     _locals = new LocalsHandler.from(localsBefore, node,
         isTry: true, useOtherTryBlock: false);
-    _fields = new FieldInitializationScope.from(fieldScopeBefore);
+    _fieldScope = new FieldInitializationScope.from(fieldScopeBefore);
     initializationIsIndefinite();
     visit(node.body);
     LocalsHandler localsAfterBody = _locals;
-    FieldInitializationScope fieldScopeAfterBody = _fields;
-    _locals = localsBefore.mergeDiamondFlow(_inferrer, localsAfterBody, null);
-    _fields = fieldScopeBefore?.mergeDiamondFlow(
-        _inferrer, fieldScopeAfterBody, null);
+    _locals = localsBefore.mergeFlow(_inferrer, localsAfterBody);
+    _fieldScope = fieldScopeBefore;
     visit(node.finalizer);
     return null;
   }
diff --git a/pkg/compiler/lib/src/inferrer/locals_handler.dart b/pkg/compiler/lib/src/inferrer/locals_handler.dart
index fca7943..2ebc446 100644
--- a/pkg/compiler/lib/src/inferrer/locals_handler.dart
+++ b/pkg/compiler/lib/src/inferrer/locals_handler.dart
@@ -144,9 +144,11 @@
     fields?.forEach(f);
   }
 
+  /// Returns the join between [thenScope] and [elseScope] which models the
+  /// flow through either [thenScope] or [elseScope].
   FieldInitializationScope mergeDiamondFlow(InferrerEngine inferrer,
       FieldInitializationScope thenScope, FieldInitializationScope elseScope) {
-    if (elseScope == null) return this;
+    assert(elseScope != null);
 
     // Quick bailout check. If [isThisExposed] or [isIndefinite] is true, we
     // know the code following won'TypeInformation do anything.
@@ -154,7 +156,7 @@
     if (isIndefinite) return this;
 
     FieldInitializationScope otherScope =
-        (elseScope == null || elseScope.fields == null) ? this : elseScope;
+        elseScope.fields == null ? this : elseScope;
 
     thenScope.forEach((FieldEntity field, TypeInformation type) {
       TypeInformation otherType = otherScope.readField(field);
@@ -329,17 +331,15 @@
     update(inferrer, capturedAndBoxed, local, newType, node, type);
   }
 
-  LocalsHandler mergeDiamondFlow(InferrerEngine inferrer,
-      LocalsHandler thenBranch, LocalsHandler elseBranch) {
-    seenReturnOrThrow = thenBranch.seenReturnOrThrow &&
-        elseBranch != null &&
-        elseBranch.seenReturnOrThrow;
-    seenBreakOrContinue = thenBranch.seenBreakOrContinue &&
-        elseBranch != null &&
-        elseBranch.seenBreakOrContinue;
-    if (aborts) return this;
+  /// Returns the join between this locals handler and [other] which models the
+  /// flow through either this or [other].
+  LocalsHandler mergeFlow(InferrerEngine inferrer, LocalsHandler other) {
+    seenReturnOrThrow = false;
+    seenBreakOrContinue = false;
 
-    void mergeOneBranch(LocalsHandler other) {
+    if (other.aborts) {
+      return this;
+    } else {
       other._locals.forEachOwnLocal((Local local, TypeInformation type) {
         TypeInformation myType = _locals[local];
         if (myType == null) return; // Variable is only defined in [other].
@@ -347,6 +347,19 @@
         _locals[local] = inferrer.types.allocateDiamondPhi(myType, type);
       });
     }
+    return this;
+  }
+
+  /// Returns the join between [thenBranch] and [elseBranch] which models the
+  /// flow through either [thenBranch] or [elseBranch].
+  LocalsHandler mergeDiamondFlow(InferrerEngine inferrer,
+      LocalsHandler thenBranch, LocalsHandler elseBranch) {
+    assert(elseBranch != null);
+    seenReturnOrThrow =
+        thenBranch.seenReturnOrThrow && elseBranch.seenReturnOrThrow;
+    seenBreakOrContinue =
+        thenBranch.seenBreakOrContinue && elseBranch.seenBreakOrContinue;
+    if (aborts) return this;
 
     void inPlaceUpdateOneBranch(LocalsHandler other) {
       other._locals.forEachOwnLocal((Local local, TypeInformation type) {
@@ -358,10 +371,7 @@
     }
 
     if (thenBranch.aborts) {
-      if (elseBranch == null) return this;
       inPlaceUpdateOneBranch(elseBranch);
-    } else if (elseBranch == null) {
-      mergeOneBranch(thenBranch);
     } else if (elseBranch.aborts) {
       inPlaceUpdateOneBranch(thenBranch);
     } else {