During resolution, record the target of break/continue statements.

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

 // Copyright (c) 2014, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library engine.resolver;
 
 import "dart:math" as math;
 import 'dart:collection';
 
 import 'ast.dart';
@@ skipping 7215 matching lines @@
       INIT_IN_INITIALIZERS];
 
   const INIT_STATE(String name, int ordinal) : super(name, ordinal);
 }
 
 /**
  * Instances of the class `LabelScope` represent a scope in which a single label is defined.
  */
 class LabelScope {
   /**
-   * The marker used to look up a label element for an unlabeled `break` or `continue`.
-   */
-  static String EMPTY_LABEL = "";
-
-  /**
-   * The label element returned for scopes that can be the target of an unlabeled `break` or
-   * `continue`.
-   */
-  static SimpleIdentifier _EMPTY_LABEL_IDENTIFIER =
-      new SimpleIdentifier(new sc.StringToken(sc.TokenType.IDENTIFIER, "", 0));
-
-  /**
    * The label scope enclosing this label scope.
    */
   final LabelScope _outerScope;
 
   /**
    * The label defined in this scope.
    */
   final String _label;
 
   /**
    * The element to which the label resolves.
    */
-  final LabelElement _element;
+  final LabelElement element;
 
   /**
-   * Initialize a newly created scope to represent the potential target of an unlabeled
-   * `break` or `continue`.
-   *
-   * @param outerScope the label scope enclosing the new label scope
-   * @param onSwitchStatement `true` if this label is associated with a `switch`
-   *          statement
-   * @param onSwitchMember `true` if this label is associated with a `switch` member
+   * The AST node to which the label resolves.
    */
-  LabelScope.con1(LabelScope outerScope, bool onSwitchStatement,
-      bool onSwitchMember)
-      : this.con2(
-          outerScope,
-          EMPTY_LABEL,
-          new LabelElementImpl(
-              _EMPTY_LABEL_IDENTIFIER,
-              onSwitchStatement,
-              onSwitchMember));
+  final AstNode node;
 
   /**
-   * Initialize a newly created scope to represent the given label.
-   *
-   * @param outerScope the label scope enclosing the new label scope
-   * @param label the label defined in this scope
-   * @param element the element to which the label resolves
+   * Initialize a newly created scope to represent the label [_label].
+   * [_outerScope] is the scope enclosing the new label scope.  [node] is the
+   * AST node the label resolves to.  [element] is the element the label
+   * resolves to.
    */
-  LabelScope.con2(this._outerScope, this._label, this._element);
+  LabelScope(this._outerScope, this._label, this.node, this.element);
 
   /**
-   * Return the label element corresponding to the given label, or `null` if the given label
-   * is not defined in this scope.
-   *
-   * @param targetLabel the label being looked up
-   * @return the label element corresponding to the given label
+   * Return the LabelScope which defines [targetLabel], or `null` if it is not
+   * defined in this scope.
    */
-  LabelElement lookup(String targetLabel) {
+  LabelScope lookup(String targetLabel) {
     if (_label == targetLabel) {
-      return _element;
+      return this;
     } else if (_outerScope != null) {
       return _outerScope.lookup(targetLabel);
     } else {
       return null;
     }
   }
 }
 
 /**
  * Instances of the class `Library` represent the data about a single library during the
@@ skipping 3739 matching lines @@
    * The object keeping track of which elements have had their types overridden.
    */
   TypeOverrideManager _overrideManager = new TypeOverrideManager();
 
   /**
    * The object keeping track of which elements have had their types promoted.
    */
   TypePromotionManager _promoteManager = new TypePromotionManager();
 
   /**
+   * The AST node representing the innermost enclosing statement that can be
+   * target of an unlabeled "break" statement, or null if there is no such

[Brian Wilkerson, 2014/11/24 23:20:53]

"target" --> "the target"
"null" --> "`null`"

(both here and below)

+   * statement.
+   */
+  Statement _unlabeledBreakTarget;

[Brian Wilkerson, 2014/11/24 23:20:53]

I'm not thrilled with this implementation for two reasons. (That isn't to say
that this is wrong, but I'd be interested to talk more about it.)

First, ScopedVisitor was designed to maintain the scope information so that (a)
all of that knowledge would be in one place and (b) it could be used by any
number of subclasses. While we don't currently have any subclasses that depend
on the label scope being constructed correctly, we can't add them in the future
without duplicating this implementation.

Second, it goes against the general design of the scope objects being a stack
that can be used to push and pop scope information by adding a second stack for
unlabeled statements.

+
+  /**
+   * The AST node representing the innermost enclosing statement that can be
+   * target of an unlabeled "continue" statement, or null if there is no such
+   * statement.
+   */
+  Statement _unlabeledContinueTarget;
+
+  /**
    * Initialize a newly created visitor to resolve the nodes in a compilation unit.
    *
    * @param library the library containing the compilation unit being resolved
    * @param source the source representing the compilation unit being visited
    * @param typeProvider the object used to access the types from the core library
    */
   ResolverVisitor.con1(Library library, Source source,
       TypeProvider typeProvider)
       : super.con1(library, source, typeProvider) {
     this._inheritanceManager = library.inheritanceManager;
@@ skipping 162 matching lines @@
     ElementKind kind = element.kind;
     if (kind == ElementKind.LOCAL_VARIABLE) {
       return element as VariableElement;
     }
     if (kind == ElementKind.PARAMETER) {
       return element as VariableElement;
     }
     return null;
   }
 
+  AstNode getUnlabeledBreakOrContinueTarget(bool isContinue) {
+    if (isContinue) {
+      return _unlabeledContinueTarget;
+    } else {
+      return _unlabeledBreakTarget;
+    }
+  }
+
   /**
    * If it is appropriate to do so, override the current type of the static and propagated elements
    * associated with the given expression with the given type. Generally speaking, it is appropriate
    * if the given type is more specific than the current type.
    *
    * @param expression the expression used to access the static and propagated elements whose types
    *          might be overridden
    * @param potentialType the potential type of the elements
    * @param allowPrecisionLoss see @{code overrideVariable} docs
    */
@@ skipping 132 matching lines @@
     }
     node.accept(_elementResolver);
     node.accept(_typeAnalyzer);
     return null;
   }
 
   @override
   Object visitBlockFunctionBody(BlockFunctionBody node) {
     safelyVisit(_commentBeforeFunction);
     _overrideManager.enterScope();
+    Statement previousUnlabeledBreakTarget = _unlabeledBreakTarget;
+    Statement previousUnlabeledContinueTarget = _unlabeledContinueTarget;
     try {
+      _unlabeledBreakTarget = null;
+      _unlabeledContinueTarget = null;
       super.visitBlockFunctionBody(node);
     } finally {
       _overrideManager.exitScope();
+      _unlabeledBreakTarget = previousUnlabeledBreakTarget;
+      _unlabeledContinueTarget = previousUnlabeledContinueTarget;
     }
     return null;
   }
 
   @override
   Object visitBreakStatement(BreakStatement node) {
     //
     // We do not visit the label because it needs to be visited in the context
     // of the statement.
     //
@@ skipping 188 matching lines @@
     // We do not visit the label because it needs to be visited in the context
     // of the statement.
     //
     node.accept(_elementResolver);
     node.accept(_typeAnalyzer);
     return null;
   }
 
   @override
   Object visitDoStatement(DoStatement node) {
+    Statement previousUnlabeledBreakTarget = _unlabeledBreakTarget;
+    Statement previousUnlabeledContinueTarget = _unlabeledContinueTarget;
     _overrideManager.enterScope();
     try {
+      _unlabeledBreakTarget = node;
+      _unlabeledContinueTarget = node;
       super.visitDoStatement(node);
     } finally {
       _overrideManager.exitScope();
+      _unlabeledBreakTarget = previousUnlabeledBreakTarget;
+      _unlabeledContinueTarget = previousUnlabeledContinueTarget;
     }
     // TODO(brianwilkerson) If the loop can only be exited because the condition
     // is false, then propagateFalseState(node.getCondition());
     return null;
   }
 
   @override
   Object visitEmptyFunctionBody(EmptyFunctionBody node) {
     safelyVisit(_commentBeforeFunction);
     return super.visitEmptyFunctionBody(node);
@@ skipping 35 matching lines @@
       Map<VariableElement, DartType> overrides =
           _overrideManager.captureOverrides(node.fields);
       _overrideManager.exitScope();
       _overrideManager.applyOverrides(overrides);
     }
     return null;
   }
 
   @override
   Object visitForEachStatement(ForEachStatement node) {
+    Statement previousUnlabeledBreakTarget = _unlabeledBreakTarget;
+    Statement previousUnlabeledContinueTarget = _unlabeledContinueTarget;
     _overrideManager.enterScope();
     try {
+      _unlabeledBreakTarget = node;
+      _unlabeledContinueTarget = node;
       super.visitForEachStatement(node);
     } finally {
       _overrideManager.exitScope();
+      _unlabeledBreakTarget = previousUnlabeledBreakTarget;
+      _unlabeledContinueTarget = previousUnlabeledContinueTarget;
     }
     return null;
   }
 
   @override
   void visitForEachStatementInScope(ForEachStatement node) {
     //
     // We visit the iterator before the loop variable because the loop variable
     // cannot be in scope while visiting the iterator.
     //
@@ skipping 26 matching lines @@
       } finally {
         _overrideManager.exitScope();
       }
     }
     node.accept(_elementResolver);
     node.accept(_typeAnalyzer);
   }
 
   @override
   Object visitForStatement(ForStatement node) {
+    Statement previousUnlabeledBreakTarget = _unlabeledBreakTarget;
+    Statement previousUnlabeledContinueTarget = _unlabeledContinueTarget;
     _overrideManager.enterScope();
     try {
+      _unlabeledBreakTarget = node;
+      _unlabeledContinueTarget = node;
       super.visitForStatement(node);
     } finally {
       _overrideManager.exitScope();
+      _unlabeledBreakTarget = previousUnlabeledBreakTarget;
+      _unlabeledContinueTarget = previousUnlabeledContinueTarget;
     }
     return null;
   }
 
   @override
   void visitForStatementInScope(ForStatement node) {
     safelyVisit(node.variables);
     safelyVisit(node.initialization);
     safelyVisit(node.condition);
     _overrideManager.enterScope();
@@ skipping 241 matching lines @@
     _overrideManager.enterScope();
     try {
       super.visitSwitchDefault(node);
     } finally {
       _overrideManager.exitScope();
     }
     return null;
   }
 
   @override
+  Object visitSwitchStatement(SwitchStatement node) {
+    Statement previousUnlabeledBreakTarget = _unlabeledBreakTarget;
+    try {
+      _unlabeledBreakTarget = node;
+      super.visitSwitchStatement(node);
+    } finally {
+      _unlabeledBreakTarget = previousUnlabeledBreakTarget;
+    }
+    return null;
+  }
+
+  @override
   Object visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) {
     _overrideManager.enterScope();
     try {
       super.visitTopLevelVariableDeclaration(node);
     } finally {
       Map<VariableElement, DartType> overrides =
           _overrideManager.captureOverrides(node.variables);
       _overrideManager.exitScope();
       _overrideManager.applyOverrides(overrides);
     }
     return null;
   }
 
   @override
   Object visitTypeName(TypeName node) => null;
 
   @override
   Object visitWhileStatement(WhileStatement node) {
     Expression condition = node.condition;
     safelyVisit(condition);
     Statement body = node.body;
     if (body != null) {
+      Statement previousUnlabeledBreakTarget = _unlabeledBreakTarget;
+      Statement previousUnlabeledContinueTarget = _unlabeledContinueTarget;
       _overrideManager.enterScope();
       try {
+        _unlabeledBreakTarget = node;
+        _unlabeledContinueTarget = node;
         _propagateTrueState(condition);
         visitStatementInScope(body);
       } finally {
         _overrideManager.exitScope();
+        _unlabeledBreakTarget = previousUnlabeledBreakTarget;
+        _unlabeledContinueTarget = previousUnlabeledContinueTarget;
       }
     }
     // TODO(brianwilkerson) If the loop can only be exited because the condition
     // is false, then propagateFalseState(condition);
     node.accept(_elementResolver);
     node.accept(_typeAnalyzer);
     return null;
   }
 
   /**
@@ skipping 895 matching lines @@
     VariableElement element = node.element;
     if (element != null) {
       _nameScope.define(element);
     }
     super.visitDeclaredIdentifier(node);
     return null;
   }
 
   @override
   Object visitDoStatement(DoStatement node) {
-    LabelScope outerLabelScope = _labelScope;
-    try {
-      _labelScope = new LabelScope.con1(_labelScope, false, false);
-      visitStatementInScope(node.body);
-      safelyVisit(node.condition);
-    } finally {
-      _labelScope = outerLabelScope;
-    }
+    visitStatementInScope(node.body);
+    safelyVisit(node.condition);
     return null;
   }
 
   @override
   Object visitForEachStatement(ForEachStatement node) {
     Scope outerNameScope = _nameScope;
-    LabelScope outerLabelScope = _labelScope;
     try {
       _nameScope = new EnclosedScope(_nameScope);
-      _labelScope = new LabelScope.con1(outerLabelScope, false, false);
       visitForEachStatementInScope(node);
     } finally {
-      _labelScope = outerLabelScope;
       _nameScope = outerNameScope;
     }
     return null;
   }
 
   /**
    * Visit the given statement after it's scope has been created. This replaces the normal call to
    * the inherited visit method so that ResolverVisitor can intervene when type propagation is
    * enabled.
    *
@@ skipping 20 matching lines @@
     }
     if (_nameScope is FunctionTypeScope) {
       (_nameScope as FunctionTypeScope).defineParameters();
     }
     return null;
   }
 
   @override
   Object visitForStatement(ForStatement node) {
     Scope outerNameScope = _nameScope;
-    LabelScope outerLabelScope = _labelScope;
     try {
       _nameScope = new EnclosedScope(_nameScope);
-      _labelScope = new LabelScope.con1(outerLabelScope, false, false);
       visitForStatementInScope(node);
     } finally {
-      _labelScope = outerLabelScope;
       _nameScope = outerNameScope;
     }
     return null;
   }
 
   /**
    * Visit the given statement after it's scope has been created. This replaces the normal call to
    * the inherited visit method so that ResolverVisitor can intervene when type propagation is
    * enabled.
    *
@@ skipping 82 matching lines @@
   @override
   Object visitIfStatement(IfStatement node) {
     safelyVisit(node.condition);
     visitStatementInScope(node.thenStatement);
     visitStatementInScope(node.elseStatement);
     return null;
   }
 
   @override
   Object visitLabeledStatement(LabeledStatement node) {
-    LabelScope outerScope = _addScopesFor(node.labels);
+    LabelScope outerScope = _addScopesFor(node.labels, node.unlabeled);
     try {
       super.visitLabeledStatement(node);
     } finally {
       _labelScope = outerScope;
     }
     return null;
   }
 
   @override
   Object visitMethodDeclaration(MethodDeclaration node) {
@@ skipping 55 matching lines @@
     try {
       _nameScope = new EnclosedScope(_nameScope);
       node.statements.accept(this);
     } finally {
       _nameScope = outerNameScope;
     }
     return null;
   }
 
   @override
   Object visitSwitchStatement(SwitchStatement node) {

[Brian Wilkerson, 2014/11/24 23:20:53]

This change looks wrong to me. The purpose of the label scope was to allow the
labels defined on the switch members to be in scope across the entire switch
statement and then to be easily removed when we exited the switch statement. How
do we remove those scopes?

-    LabelScope outerScope = _labelScope;
-    try {
-      _labelScope = new LabelScope.con1(outerScope, true, false);
-      for (SwitchMember member in node.members) {
-        for (Label label in member.labels) {
-          SimpleIdentifier labelName = label.label;
-          LabelElement labelElement = labelName.staticElement as LabelElement;
-          _labelScope =
-              new LabelScope.con2(_labelScope, labelName.name, labelElement);
-        }
+    for (SwitchMember member in node.members) {
+      for (Label label in member.labels) {
+        SimpleIdentifier labelName = label.label;
+        LabelElement labelElement = labelName.staticElement as LabelElement;
+        _labelScope =
+            new LabelScope(_labelScope, labelName.name, member, labelElement);
       }
-      super.visitSwitchStatement(node);
-    } finally {
-      _labelScope = outerScope;
     }
+    super.visitSwitchStatement(node);
     return null;
   }
 
   @override
   Object visitVariableDeclaration(VariableDeclaration node) {
     super.visitVariableDeclaration(node);
     if (node.parent.parent is! TopLevelVariableDeclaration &&
         node.parent.parent is! FieldDeclaration) {
       VariableElement element = node.element;
       if (element != null) {
         _nameScope.define(element);
       }
     }
     return null;
   }
 
   @override
   Object visitWhileStatement(WhileStatement node) {
-    LabelScope outerScope = _labelScope;
-    try {
-      _labelScope = new LabelScope.con1(outerScope, false, false);
-      safelyVisit(node.condition);
-      visitStatementInScope(node.body);
-    } finally {
-      _labelScope = outerScope;
-    }
+    safelyVisit(node.condition);
+    visitStatementInScope(node.body);
     return null;
   }
 
   /**
    * Add scopes for each of the given labels.
    *
    * @param labels the labels for which new scopes are to be added
    * @return the scope that was in effect before the new scopes were added
    */
-  LabelScope _addScopesFor(NodeList<Label> labels) {
+  LabelScope _addScopesFor(NodeList<Label> labels, AstNode node) {
     LabelScope outerScope = _labelScope;
     for (Label label in labels) {
       SimpleIdentifier labelNameNode = label.label;
       String labelName = labelNameNode.name;
       LabelElement labelElement = labelNameNode.staticElement as LabelElement;
-      _labelScope = new LabelScope.con2(_labelScope, labelName, labelElement);
+      _labelScope = new LabelScope(_labelScope, labelName, node, labelElement);
     }
     return outerScope;
   }
 
   /**
    * Marks the local declarations of the given [Block] hidden in the enclosing scope.
    * According to the scoping rules name is hidden if block defines it, but name is defined after
    * its declaration statement.
    */
   void _hideNamesDefinedInBlock(EnclosedScope scope, Block block) {
@@ skipping 2971 matching lines @@
    * library.
    */
   final HashSet<String> members = new HashSet<String>();
 
   /**
    * Names of resolved or unresolved class members that are read in the
    * library.
    */
   final HashSet<String> readMembers = new HashSet<String>();
 }