Remove FieldInitializerScope from LocalsHandler

Change-Id: I14636228a2bc51f863f73be21913a9cc26b42257
Reviewed-on: https://dart-review.googlesource.com/c/85261
Reviewed-by: Sigmund Cherem <sigmund@google.com>
Auto-Submit: Johnni Winther <johnniwinther@google.com>
diff --git a/pkg/compiler/lib/src/inferrer/builder_kernel.dart b/pkg/compiler/lib/src/inferrer/builder_kernel.dart
index 7c2d111..6a9a1cd 100644
--- a/pkg/compiler/lib/src/inferrer/builder_kernel.dart
+++ b/pkg/compiler/lib/src/inferrer/builder_kernel.dart
@@ -49,6 +49,7 @@
   final bool _inGenerativeConstructor;
 
   LocalsHandler _locals;
+  FieldInitializationScope _fields;
   final SideEffectsBuilder _sideEffectsBuilder;
   final Map<JumpTarget, List<LocalsHandler>> _breaksFor =
       <JumpTarget, List<LocalsHandler>>{};
@@ -70,7 +71,7 @@
 
   KernelTypeGraphBuilder(this._options, this._closedWorld, this._inferrer,
       this._analyzedMember, this._analyzedNode, this._localsMap,
-      [this._locals, Map<Local, FieldEntity> capturedAndBoxed])
+      [this._locals, this._fields, Map<Local, FieldEntity> capturedAndBoxed])
       : this._types = _inferrer.types,
         this._memberData = _inferrer.dataOfMember(_analyzedMember),
         // TODO(johnniwinther): Should side effects also be tracked for field
@@ -85,9 +86,8 @@
             : <Local, FieldEntity>{} {
     if (_locals != null) return;
 
-    FieldInitializationScope fieldScope =
-        _inGenerativeConstructor ? new FieldInitializationScope() : null;
-    _locals = new LocalsHandler(_analyzedNode, fieldScope);
+    _fields = _inGenerativeConstructor ? new FieldInitializationScope() : null;
+    _locals = new LocalsHandler(_analyzedNode);
   }
 
   JsToElementMap get _elementMap => _closedWorld.elementMap;
@@ -99,12 +99,12 @@
   bool get inLoop => _loopLevel > 0;
 
   bool get _isThisExposed {
-    return _inGenerativeConstructor ? _locals.fieldScope.isThisExposed : true;
+    return _inGenerativeConstructor ? _fields.isThisExposed : true;
   }
 
   void _markThisAsExposed() {
     if (_inGenerativeConstructor) {
-      _locals.fieldScope.isThisExposed = true;
+      _fields.isThisExposed = true;
     }
   }
 
@@ -141,7 +141,7 @@
           if (!selector.isSetter &&
               _isInClassOrSubclass(field) &&
               field.isAssignable &&
-              _locals.fieldScope.readField(field) == null &&
+              _fields.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 +198,7 @@
 
   void initializationIsIndefinite() {
     if (_inGenerativeConstructor) {
-      _locals.fieldScope.isIndefinite = true;
+      _fields.isIndefinite = true;
     }
   }
 
@@ -257,7 +257,7 @@
       _elementMap.elementEnvironment.forEachLocalClassMember(cls,
           (MemberEntity member) {
         if (member.isField && member.isInstanceMember && member.isAssignable) {
-          TypeInformation type = _locals.fieldScope.readField(member);
+          TypeInformation type = _fields.readField(member);
           MemberDefinition definition = _elementMap.getMemberDefinition(member);
           assert(definition.kind == MemberKind.regular);
           ir.Field node = definition.node;
@@ -291,7 +291,7 @@
   visitFieldInitializer(ir.FieldInitializer node) {
     TypeInformation rhsType = visit(node.value);
     FieldEntity field = _elementMap.getField(node.field);
-    _locals.updateField(field, rhsType);
+    _fields.updateField(field, rhsType);
     _inferrer.recordTypeOfField(field, rhsType);
     return null;
   }
@@ -455,17 +455,26 @@
     List<IsCheck> negativeTests = <IsCheck>[];
     bool simpleCondition =
         handleCondition(node.condition, positiveTests, negativeTests);
-    LocalsHandler saved = _locals;
-    _locals = new LocalsHandler.from(_locals, node);
-    _updateIsChecks(positiveTests, negativeTests);
+    LocalsHandler localsBefore = _locals;
+    FieldInitializationScope fieldScopeBefore = _fields;
 
-    LocalsHandler thenLocals = _locals;
-    _locals = new LocalsHandler.from(saved, node);
+    _locals = new LocalsHandler.from(localsBefore, node);
+    _updateIsChecks(positiveTests, negativeTests);
+    LocalsHandler localsAfterCondition = _locals;
+    FieldInitializationScope fieldScopeAfterThen = _fields;
+
+    _locals = new LocalsHandler.from(localsBefore, node);
+    _fields = new FieldInitializationScope.from(fieldScopeBefore);
     if (simpleCondition) _updateIsChecks(negativeTests, positiveTests);
     visit(node.message);
-    _locals.seenReturnOrThrow = true;
-    saved.mergeDiamondFlow(_inferrer, thenLocals, _locals);
-    _locals = saved;
+
+    LocalsHandler localsAfterMessage = _locals;
+    FieldInitializationScope fieldScopeAfterMessage = _fields;
+    localsAfterMessage.seenReturnOrThrow = true;
+    _locals = localsBefore.mergeDiamondFlow(
+        _inferrer, localsAfterCondition, localsAfterMessage);
+    _fields = fieldScopeBefore?.mergeDiamondFlow(
+        _inferrer, fieldScopeAfterThen, fieldScopeAfterMessage);
     return null;
   }
 
@@ -525,18 +534,24 @@
       do {
         changed = false;
         for (ir.SwitchCase switchCase in node.cases) {
-          LocalsHandler saved = _locals;
-          _locals = new LocalsHandler.from(_locals, switchCase);
+          LocalsHandler localsBeforeCase = _locals;
+          FieldInitializationScope fieldScopeBeforeCase = _fields;
+          _locals = new LocalsHandler.from(localsBeforeCase, switchCase);
+          _fields = new FieldInitializationScope.from(fieldScopeBeforeCase);
           visit(switchCase);
-          changed = saved.mergeAll(_inferrer, [_locals]) || changed;
-          _locals = saved;
+          LocalsHandler localsAfterCase = _locals;
+          changed = localsBeforeCase.mergeAll(_inferrer, [localsAfterCase]) ||
+              changed;
+          _locals = localsBeforeCase;
+          _fields = fieldScopeBeforeCase;
         }
       } while (changed);
       _locals.endLoop(_inferrer, node);
 
       continueTargets.forEach(_clearBreaksAndContinues);
     } else {
-      LocalsHandler saved = _locals;
+      LocalsHandler localsBeforeCase = _locals;
+      FieldInitializationScope fieldScopeBeforeCase = _fields;
       List<LocalsHandler> localsToMerge = <LocalsHandler>[];
       bool hasDefaultCase = false;
 
@@ -544,13 +559,14 @@
         if (switchCase.isDefault) {
           hasDefaultCase = true;
         }
-        _locals = new LocalsHandler.from(saved, switchCase);
+        _locals = new LocalsHandler.from(localsBeforeCase, switchCase);
         visit(switchCase);
         localsToMerge.add(_locals);
       }
-      saved.mergeAfterBreaks(_inferrer, localsToMerge,
+      localsBeforeCase.mergeAfterBreaks(_inferrer, localsToMerge,
           keepOwnLocals: !hasDefaultCase);
-      _locals = saved;
+      _locals = localsBeforeCase;
+      _fields = fieldScopeBeforeCase;
     }
     _clearBreaksAndContinues(jumpTarget);
     return null;
@@ -1012,22 +1028,24 @@
   TypeInformation handleLoop(ir.Node node, JumpTarget target, void logic()) {
     _loopLevel++;
     bool changed = false;
-    LocalsHandler saved = _locals;
-    saved.startLoop(_inferrer, node);
+    LocalsHandler localsBefore = _locals;
+    FieldInitializationScope fieldScopeBefore = _fields;
+    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(saved, node);
+      _locals = new LocalsHandler.from(localsBefore, node);
+      _fields = new FieldInitializationScope.from(fieldScopeBefore);
       logic();
-      changed = saved.mergeAll(_inferrer, _getLoopBackEdges(target));
+      changed = localsBefore.mergeAll(_inferrer, _getLoopBackEdges(target));
     } while (changed);
     _loopLevel--;
-    saved.endLoop(_inferrer, node);
+    localsBefore.endLoop(_inferrer, node);
     bool keepOwnLocals = node is! ir.DoStatement;
-    saved.mergeAfterBreaks(_inferrer, _getBreaks(target),
+    _locals = localsBefore.mergeAfterBreaks(_inferrer, _getBreaks(target),
         keepOwnLocals: keepOwnLocals);
-    _locals = saved;
+    _fields = fieldScopeBefore;
     _clearBreaksAndContinues(target);
     return null;
   }
@@ -1293,7 +1311,7 @@
           MemberEntity single = targets.first;
           if (single.isField) {
             FieldEntity field = single;
-            _locals.updateField(field, rhsType);
+            _fields.updateField(field, rhsType);
           }
         }
       }
@@ -1378,18 +1396,28 @@
     List<IsCheck> negativeTests = <IsCheck>[];
     bool simpleCondition =
         handleCondition(node.condition, positiveTests, negativeTests);
-    LocalsHandler saved = _locals;
-    _locals = new LocalsHandler.from(_locals, node);
+    LocalsHandler localsBefore = _locals;
+    FieldInitializationScope fieldScopeBefore = _fields;
+    _locals = new LocalsHandler.from(localsBefore, node);
+    _fields = new FieldInitializationScope.from(fieldScopeBefore);
     _updateIsChecks(positiveTests, negativeTests);
     visit(node.then);
-    LocalsHandler thenLocals = _locals;
-    _locals = new LocalsHandler.from(saved, node);
+
+    LocalsHandler localsAfterThen = _locals;
+    FieldInitializationScope fieldScopeAfterThen = _fields;
+    _locals = new LocalsHandler.from(localsBefore, node);
+    _fields = new FieldInitializationScope.from(fieldScopeBefore);
     if (simpleCondition) {
       _updateIsChecks(negativeTests, positiveTests);
     }
     visit(node.otherwise);
-    saved.mergeDiamondFlow(_inferrer, thenLocals, _locals);
-    _locals = saved;
+    LocalsHandler localsAfterElse = _locals;
+    FieldInitializationScope fieldScopeAfterElse = _fields;
+
+    _locals = localsBefore.mergeDiamondFlow(
+        _inferrer, localsAfterThen, localsAfterElse);
+    _fields = fieldScopeBefore?.mergeDiamondFlow(
+        _inferrer, fieldScopeAfterThen, fieldScopeAfterElse);
     return null;
   }
 
@@ -1425,9 +1453,12 @@
         _negativeIsChecks = <IsCheck>[];
       }
       visit(node.left, conditionContext: _accumulateIsChecks);
-      LocalsHandler saved = _locals;
-      _locals = new LocalsHandler.from(_locals, node);
+      LocalsHandler localsBefore = _locals;
+      FieldInitializationScope fieldScopeBefore = _fields;
+      _locals = new LocalsHandler.from(localsBefore, node);
+      _fields = new FieldInitializationScope.from(fieldScopeBefore);
       _updateIsChecks(_positiveIsChecks, _negativeIsChecks);
+
       LocalsHandler narrowed;
       if (oldAccumulateIsChecks) {
         narrowed = new LocalsHandler.topLevelCopyOf(_locals);
@@ -1445,8 +1476,8 @@
         _positiveIsChecks.removeWhere(invalidatedInRightHandSide);
         _negativeIsChecks.removeWhere(invalidatedInRightHandSide);
       }
-      saved.mergeDiamondFlow(_inferrer, _locals, null);
-      _locals = saved;
+      _locals = localsBefore.mergeDiamondFlow(_inferrer, _locals, null);
+      _fields = fieldScopeBefore?.mergeDiamondFlow(_inferrer, _fields, null);
       return _types.boolType;
     } else if (node.operator == '||') {
       _conditionIsSimple = false;
@@ -1454,14 +1485,16 @@
       List<IsCheck> negativeIsChecks = <IsCheck>[];
       bool isSimple =
           handleCondition(node.left, positiveIsChecks, negativeIsChecks);
-      LocalsHandler saved = _locals;
+      LocalsHandler localsBefore = _locals;
+      FieldInitializationScope fieldScopeBefore = _fields;
       _locals = new LocalsHandler.from(_locals, node);
+      _fields = new FieldInitializationScope.from(fieldScopeBefore);
       if (isSimple) {
         _updateIsChecks(negativeIsChecks, positiveIsChecks);
       }
       visit(node.right, conditionContext: false);
-      saved.mergeDiamondFlow(_inferrer, _locals, null);
-      _locals = saved;
+      _locals = localsBefore.mergeDiamondFlow(_inferrer, _locals, null);
+      _fields = fieldScopeBefore?.mergeDiamondFlow(_inferrer, _fields, null);
       return _types.boolType;
     }
     failedAt(CURRENT_ELEMENT_SPANNABLE,
@@ -1475,16 +1508,24 @@
     List<IsCheck> negativeTests = <IsCheck>[];
     bool simpleCondition =
         handleCondition(node.condition, positiveTests, negativeTests);
-    LocalsHandler saved = _locals;
-    _locals = new LocalsHandler.from(_locals, node);
+    LocalsHandler localsBefore = _locals;
+    _locals = new LocalsHandler.from(localsBefore, node);
+    FieldInitializationScope fieldScopeBefore = _fields;
+    _fields = new FieldInitializationScope.from(fieldScopeBefore);
     _updateIsChecks(positiveTests, negativeTests);
     TypeInformation firstType = visit(node.then);
-    LocalsHandler thenLocals = _locals;
-    _locals = new LocalsHandler.from(saved, node);
+    LocalsHandler localsAfterThen = _locals;
+    FieldInitializationScope fieldScopeAfterThen = _fields;
+    _locals = new LocalsHandler.from(localsBefore, node);
+    _fields = new FieldInitializationScope.from(fieldScopeBefore);
     if (simpleCondition) _updateIsChecks(negativeTests, positiveTests);
     TypeInformation secondType = visit(node.otherwise);
-    saved.mergeDiamondFlow(_inferrer, thenLocals, _locals);
-    _locals = saved;
+    LocalsHandler localsAfterElse = _locals;
+    FieldInitializationScope fieldScopeAfterElse = _fields;
+    _locals = localsBefore.mergeDiamondFlow(
+        _inferrer, localsAfterThen, localsAfterElse);
+    _fields = fieldScopeBefore?.mergeDiamondFlow(
+        _inferrer, fieldScopeAfterThen, fieldScopeAfterElse);
     return _types.allocateDiamondPhi(firstType, secondType);
   }
 
@@ -1531,6 +1572,8 @@
     // method, like for example the types of local variables.
     LocalsHandler closureLocals =
         new LocalsHandler.from(_locals, node, useOtherTryBlock: false);
+    FieldInitializationScope closureFieldScope =
+        new FieldInitializationScope.from(_fields);
     KernelTypeGraphBuilder visitor = new KernelTypeGraphBuilder(
         _options,
         _closedWorld,
@@ -1539,6 +1582,7 @@
         functionNode,
         _localsMap,
         closureLocals,
+        closureFieldScope,
         _capturedAndBoxed);
     visitor.run();
     _inferrer.recordReturnType(info.callMethod, visitor._returnType);
@@ -1601,32 +1645,49 @@
 
   @override
   visitTryCatch(ir.TryCatch node) {
-    LocalsHandler saved = _locals;
-    _locals = new LocalsHandler.from(_locals, node,
+    LocalsHandler localsBefore = _locals;
+    FieldInitializationScope fieldScopeBefore = _fields;
+    _locals = new LocalsHandler.from(localsBefore, node,
         isTry: true, useOtherTryBlock: false);
+    _fields = new FieldInitializationScope.from(fieldScopeBefore);
     initializationIsIndefinite();
     visit(node.body);
-    saved.mergeDiamondFlow(_inferrer, _locals, null);
-    _locals = saved;
+    LocalsHandler localsAfterBody = _locals;
+    FieldInitializationScope fieldScopeAfterBody = _fields;
+
+    _locals = localsBefore.mergeDiamondFlow(_inferrer, localsAfterBody, null);
+    _fields = fieldScopeBefore?.mergeDiamondFlow(
+        _inferrer, fieldScopeAfterBody, null);
     for (ir.Catch catchBlock in node.catches) {
-      saved = _locals;
-      _locals = new LocalsHandler.from(_locals, catchBlock);
+      LocalsHandler localsBeforeCatch = _locals;
+      FieldInitializationScope fieldScopeBeforeCatch = _fields;
+      _locals = new LocalsHandler.from(localsBeforeCatch, catchBlock);
+      _fields = new FieldInitializationScope.from(fieldScopeBeforeCatch);
       visit(catchBlock);
-      saved.mergeDiamondFlow(_inferrer, _locals, null);
-      _locals = saved;
+      LocalsHandler localsAfterCatch = _locals;
+      FieldInitializationScope fieldScopeAfterCatch = _fields;
+      _locals =
+          localsBeforeCatch.mergeDiamondFlow(_inferrer, localsAfterCatch, null);
+      _fields = fieldScopeBeforeCatch?.mergeDiamondFlow(
+          _inferrer, fieldScopeAfterCatch, null);
     }
     return null;
   }
 
   @override
   visitTryFinally(ir.TryFinally node) {
-    LocalsHandler saved = _locals;
-    _locals = new LocalsHandler.from(_locals, node,
+    LocalsHandler localsBefore = _locals;
+    FieldInitializationScope fieldScopeBefore = _fields;
+    _locals = new LocalsHandler.from(localsBefore, node,
         isTry: true, useOtherTryBlock: false);
+    _fields = new FieldInitializationScope.from(fieldScopeBefore);
     initializationIsIndefinite();
     visit(node.body);
-    saved.mergeDiamondFlow(_inferrer, _locals, null);
-    _locals = saved;
+    LocalsHandler localsAfterBody = _locals;
+    FieldInitializationScope fieldScopeAfterBody = _fields;
+    _locals = localsBefore.mergeDiamondFlow(_inferrer, localsAfterBody, null);
+    _fields = fieldScopeBefore?.mergeDiamondFlow(
+        _inferrer, fieldScopeAfterBody, null);
     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 5be66e9..8cb9564 100644
--- a/pkg/compiler/lib/src/inferrer/locals_handler.dart
+++ b/pkg/compiler/lib/src/inferrer/locals_handler.dart
@@ -144,12 +144,14 @@
     fields?.forEach(f);
   }
 
-  void mergeDiamondFlow(InferrerEngine inferrer,
+  FieldInitializationScope mergeDiamondFlow(InferrerEngine inferrer,
       FieldInitializationScope thenScope, FieldInitializationScope elseScope) {
+    if (elseScope == null) return this;
+
     // Quick bailout check. If [isThisExposed] or [isIndefinite] is true, we
     // know the code following won'TypeInformation do anything.
-    if (isThisExposed) return;
-    if (isIndefinite) return;
+    if (isThisExposed) return this;
+    if (isIndefinite) return this;
 
     FieldInitializationScope otherScope =
         (elseScope == null || elseScope.fields == null) ? this : elseScope;
@@ -162,6 +164,7 @@
 
     isThisExposed = thenScope.isThisExposed || elseScope.isThisExposed;
     isIndefinite = thenScope.isIndefinite || elseScope.isIndefinite;
+    return this;
   }
 }
 
@@ -246,7 +249,6 @@
  */
 class LocalsHandler {
   final VariableScope locals;
-  final FieldInitializationScope fieldScope;
   LocalsHandler tryBlock;
   bool seenReturnOrThrow = false;
   bool seenBreakOrContinue = false;
@@ -257,28 +259,24 @@
 
   bool get inTryBlock => tryBlock != null;
 
-  LocalsHandler.internal(
-      ir.Node block, this.fieldScope, this.locals, this.tryBlock);
+  LocalsHandler.internal(ir.Node block, this.locals, this.tryBlock);
 
-  LocalsHandler(ir.Node block, [this.fieldScope])
+  LocalsHandler(ir.Node block)
       : locals = new VariableScope(block, isTry: false),
         tryBlock = null;
 
   LocalsHandler.from(LocalsHandler other, ir.Node block,
       {bool isTry: false, bool useOtherTryBlock: true})
-      : locals = new VariableScope(block, isTry: isTry, parent: other.locals),
-        fieldScope = new FieldInitializationScope.from(other.fieldScope) {
+      : locals = new VariableScope(block, isTry: isTry, parent: other.locals) {
     tryBlock = useOtherTryBlock ? other.tryBlock : this;
   }
 
   LocalsHandler.deepCopyOf(LocalsHandler other)
       : locals = new VariableScope.deepCopyOf(other.locals),
-        fieldScope = new FieldInitializationScope.from(other.fieldScope),
         tryBlock = other.tryBlock;
 
   LocalsHandler.topLevelCopyOf(LocalsHandler other)
       : locals = new VariableScope.topLevelCopyOf(other.locals),
-        fieldScope = new FieldInitializationScope.from(other.fieldScope),
         tryBlock = other.tryBlock;
 
   TypeInformation use(InferrerEngine inferrer,
@@ -330,19 +328,15 @@
     update(inferrer, capturedAndBoxed, local, newType, node, type);
   }
 
-  void mergeDiamondFlow(InferrerEngine inferrer, LocalsHandler thenBranch,
-      LocalsHandler elseBranch) {
-    if (fieldScope != null && elseBranch != null) {
-      fieldScope.mergeDiamondFlow(
-          inferrer, thenBranch.fieldScope, elseBranch.fieldScope);
-    }
+  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;
+    if (aborts) return this;
 
     void mergeOneBranch(LocalsHandler other) {
       other.locals.forEachOwnLocal((Local local, TypeInformation type) {
@@ -363,7 +357,7 @@
     }
 
     if (thenBranch.aborts) {
-      if (elseBranch == null) return;
+      if (elseBranch == null) return this;
       inPlaceUpdateOneBranch(elseBranch);
     } else if (elseBranch == null) {
       mergeOneBranch(thenBranch);
@@ -391,6 +385,7 @@
         if (!thenBranch.locals.updates(local)) mergeLocal(local);
       });
     }
+    return this;
   }
 
   /**
@@ -423,7 +418,8 @@
    * where [:this:] is the [LocalsHandler] for the paths through the
    * labeled statement that do not break out.
    */
-  void mergeAfterBreaks(InferrerEngine inferrer, List<LocalsHandler> handlers,
+  LocalsHandler mergeAfterBreaks(
+      InferrerEngine inferrer, List<LocalsHandler> handlers,
       {bool keepOwnLocals: true}) {
     ir.Node level = locals.block;
     // Use a separate locals handler to perform the merge in, so that Phi
@@ -456,6 +452,7 @@
     });
     seenReturnOrThrow =
         allBranchesAbort && (!keepOwnLocals || seenReturnOrThrow);
+    return this;
   }
 
   /**
@@ -520,10 +517,6 @@
     });
   }
 
-  void updateField(FieldEntity element, TypeInformation type) {
-    fieldScope.updateField(element, type);
-  }
-
   String toString() {
     StringBuffer sb = new StringBuffer();
     sb.write('LocalsHandler(');