Switch-implementation.

frog/leg/resolver.dart

Index: frog/leg/resolver.dart
diff --git a/frog/leg/resolver.dart b/frog/leg/resolver.dart
index e3334f6bb2917f09350b28a29562fa8170e86a7b..c306c19f05fd04fa09c056949449e710a586513a 100644
--- a/frog/leg/resolver.dart
+++ b/frog/leg/resolver.dart
@@ -422,6 +422,17 @@ class LabeledStatementLabelScope implements LabelScope {
   }
 }
 
+class SwitchLabelScope implements LabelScope {
+  final LabelScope outer;
+  final Map<String, LabelElement> caseLabels;

[ahe, 2012/03/13 10:17:15]

Add an empty line.

[Lasse Reichstein Nielsen, 2012/03/13 12:09:38]

Done.

+  SwitchLabelScope(this.outer, this.caseLabels);

[ahe, 2012/03/13 10:17:15]

Add an empty line.

[Lasse Reichstein Nielsen, 2012/03/13 12:09:38]

Done.

+  LabelElement lookup(String labelName) {
+    LabelElement result = caseLabels[labelName];
+    if (result !== null) return result;
+    return outer.lookup(labelName);
+  }
+}
+
 class EmptyLabelScope implements LabelScope {
   const EmptyLabelScope();
   LabelElement lookup(String label) => null;
@@ -434,34 +445,57 @@ class StatementScope {
   LabelScope labels;
   Link<StatementElement> breakTargetStack;
   Link<StatementElement> continueTargetStack;
+  int nestingLevel = 0;
 
   StatementScope()
       : labels = const EmptyLabelScope(),
         breakTargetStack = const EmptyLink<StatementElement>(),
         continueTargetStack = const EmptyLink<StatementElement>();
 
-  LabelElement lookupLabel(String label) => labels.lookup(label);
+  LabelElement lookupLabel(String label) =>
+      labels.lookup(label);
+
   StatementElement currentBreakTarget() =>
     breakTargetStack.isEmpty() ? null : breakTargetStack.head;
+
   StatementElement currentContinueTarget() =>
     continueTargetStack.isEmpty() ? null : continueTargetStack.head;
 
   void enterLabelScope(LabelElement element) {
     labels = new LabeledStatementLabelScope(labels, element);
+    nestingLevel++;
   }
+
   void exitLabelScope() {
+    nestingLevel--;
     labels = labels.outer;
   }
+
   void enterLoop(StatementElement element) {
     breakTargetStack = breakTargetStack.prepend(element);
     continueTargetStack = continueTargetStack.prepend(element);
+    nestingLevel++;
   }
+
   void exitLoop() {
+    nestingLevel--;
     breakTargetStack = breakTargetStack.tail;
     continueTargetStack = continueTargetStack.tail;
   }
-}
 
+  void enterSwitch(StatementElement breakElement,
+                   Map<String, LabelElement> continueElements) {
+    breakTargetStack = breakTargetStack.prepend(breakElement);
+    labels = new SwitchLabelScope(labels, continueElements);
+    nestingLevel++;

[ahe, 2012/03/13 10:17:15]

This seems wrong, but perhaps I'm misunderstanding something. Entering a switch
statement does not affect continue, it only affects break. So isn't this
problematic?

[Lasse Reichstein Nielsen, 2012/03/13 12:09:38]

The switch level doesn't need to be exact, as long as it's different for two
break/continue-target statements if one is inside the other.
I'll add a comment.

+  }
+
+  void exitSwitch() {
+    nestingLevel--;
+    breakTargetStack = breakTargetStack.tail;
+    labels = labels.outer;
+  }
+}
 
 class ResolverVisitor extends CommonResolverVisitor<Element> {
   final TreeElementMapping mapping;
@@ -496,9 +530,12 @@ class ResolverVisitor extends CommonResolverVisitor<Element> {
   // Create, or reuse an already created, statement element for a statement.
   StatementElement getOrCreateStatementElement(Node statement) {
     StatementElement element = mapping[statement];
-    if (element !== null) return element;
-    element = new StatementElement(statement, enclosingElement);
-    mapping[statement] = element;
+    if (element === null) {
+      element = new StatementElement(statement,
+                                     statementScope.nestingLevel,
+                                     enclosingElement);
+      mapping[statement] = element;
+    }
     return element;
   }
 
@@ -963,6 +1000,10 @@ class ResolverVisitor extends CommonResolverVisitor<Element> {
         return;
       }
       target = label.target;
+      if (!target.statement.isValidBreakTarget()) {
+        error(node.target, MessageKind.INVALID_BREAK, [labelName]);
+        return;

[ahe, 2012/03/13 10:17:15]

Thank you :-)

+      }
       label.setBreakTarget();
       mapping[node.target] = label;
     }
@@ -1051,20 +1092,66 @@ class ResolverVisitor extends CommonResolverVisitor<Element> {
 
   visitSwitchStatement(SwitchStatement node) {
     node.expression.accept(this);
-    StatementElement element = getOrCreateStatementElement(node);
-    statementScope.enterLoop(element);
+
+    StatementElement breakElement = getOrCreateStatementElement(node);

[ahe, 2012/03/13 10:17:15]

Nit: I'm not digging this method. I would prefer if it always created a
statement element.

[Lasse Reichstein Nielsen, 2012/03/13 12:09:38]

The problem is that we want to create a StatementElement for a statement as soon
as we know that it's a potential break target.
For a labeled statement, that happens when we encounter the LabelledStatement.
If the statement happens to be a switch or loop, we want to reuse that
StatementElement when we get to the switch/loop.

So this function checks if a StatementElement is already attached to the Node,
if so, returns that, and if not, creates one.
Suggestions for another approach?

+    Map<String, LabelElement> continueLabels = <LabelElement>{};
+    Link<SwitchCase> cases = node.cases.nodes;
+    while (!cases.isEmpty()) {
+      SwitchCase switchCase = cases.head;
+      if (switchCase.label !== null) {
+        Identifier labelIdentifier = switchCase.label;
+        String labelName = labelIdentifier.source.slowToString();
+
+        LabelElement existingElement = continueLabels[labelName];
+        if (existingElement !== null) {
+          // It's an error if the same label occurs twice in the same switch.
+          warning(labelIdentifier, MessageKind.DUPLICATE_LABEL, [labelName]);
+          error(existingElement.label, MessageKind.EXISTING_LABEL, [labelName]);
+        } else {
+          // It's only a warning if it shadows another label.
+          existingElement = statementScope.lookupLabel(labelName);
+          if (existingElement !== null) {
+            warning(labelIdentifier, MessageKind.DUPLICATE_LABEL, [labelName]);
+            warning(existingElement.label,
+                    MessageKind.EXISTING_LABEL, [labelName]);
+          }
+        }
+
+        StatementElement statementElement =
+            new StatementElement(switchCase,
+                                 statementScope.nestingLevel,
+                                 enclosingElement);
+        mapping[switchCase] = statementElement;
+
+        LabelElement label =
+            new LabelElement(labelIdentifier, labelName,

[ahe, 2012/03/13 10:17:15]

I don't understand the mental model for this. The label is enclosing the
statement, yet the statementElement doesn't see the label element in its chain
of enclosing elements.

[Lasse Reichstein Nielsen, 2012/03/13 12:09:38]

Are you referring to enclosingElement? I have assumed that this refers to the
function, not to syntax inside the function.

+                             statementElement, enclosingElement);
+        mapping[labelIdentifier] = label;
+        continueLabels[labelName] = label;
+      }
+      cases = cases.tail;
+      if (switchCase.defaultKeyword !== null && !cases.isEmpty()) {
+        error(switchCase, MessageKind.INVALID_CASE_DEFAULT);
+      }
+    }
+    statementScope.enterSwitch(breakElement, continueLabels);
     node.cases.accept(this);
-    statementScope.exitLoop();
+    statementScope.exitSwitch();
+
+    // Clean-up unused labels
+    continueLabels.forEach((String key, LabelElement label) {

[ahe, 2012/03/13 10:17:15]

Wouldn't it better to set a bit on the label?

[Lasse Reichstein Nielsen, 2012/03/13 12:09:38]

The bit is set. This is pure cleanup. We know that we won't be needing these
labels and statement elements after this, since they are unreferenced. 
We could just let it all stay, and check flags later, but that's a waste of
memory.

+      StatementElement statementElement = label.target;
+      SwitchCase switchCase = statementElement.statement;
+      if (!label.isContinueTarget) {
+        mapping.remove(switchCase);
+        mapping.remove(label.label);
+      }
+    });
   }
 
   visitSwitchCase(SwitchCase node) {
-    // TODO(ahe): What about the label?
-    node.expression.accept(this);
-    visitIn(node.statements, new BlockScope(context));
-  }
-
-  visitDefaultCase(DefaultCase node) {
-    // TODO(ahe): What about the label?
+    // The label was handled in [visitSwitchStatement(SwitchStatement)].
+    node.expressions.accept(this);
     visitIn(node.statements, new BlockScope(context));
   }