Change switch to give errors when cases don't follow the newest syntax.

lib/compiler/implementation/ssa/builder.dart

 // Copyright (c) 2012, 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.
 
 class Interceptors {
   Compiler compiler;
   Interceptors(Compiler this.compiler);
 
   SourceString mapOperatorToMethodName(Operator op) {
     String name = op.source.stringValue;
@@ skipping 3120 matching lines @@
     }
     startBlock.setBlockFlow(
         new HLabeledBlockInformation.implicit(
             new HSubGraphBlockInformation(new SubGraph(startBlock, lastBlock)),
             elements[node]),
         joinBlock);
     jumpHandler.close();
   }
 
   bool tryBuildConstantSwitch(SwitchStatement node) {
+    // TODO(lrn): Move the constant resolution to the resolver, so
+    // we can report an error before reaching the backend.
     Map<CaseMatch, Constant> constants = new Map<CaseMatch, Constant>();
-    // First check whether all case expressions are compile-time constants.
+    // First check whether all case expressions are compile-time constants,
+    // and all have the same type that doesn't override operator==.
+    Constant firstConstant = null;
+    bool failure = false;
     for (SwitchCase switchCase in node.cases) {
       for (Node labelOrCase in switchCase.labelsAndCases) {
         if (labelOrCase is CaseMatch) {
           CaseMatch match = labelOrCase;
           Constant constant =
             compiler.constantHandler.tryCompileNodeWithDefinitions(
                 match.expression, elements);
-          if (constant === null) return false;
+          if (constant === null) {
+            compiler.reportWarning(match.expression,
+                MessageKind.NOT_A_COMPILE_TIME_CONSTANT.error());
+            failure = true;
+            continue;
+          }
+          if (firstConstant == null) {
+            firstConstant = constant;
+            if (nonPrimitiveTypeOverridesEquals(constant)) {
+              compiler.reportWarning(match.expression,
+                  MessageKind.SWITCH_CASE_VALUE_OVERRIDES_EQUALS.error());
+              failure = true;
+            }
+          } else {
+            if (!constant.isSameType(firstConstant)) {
+              compiler.reportWarning(match.expression,
+                  MessageKind.SWITCH_CASE_TYPES_NOT_EQUAL.error());
+              failure = true;
+            }
+          }
           constants[labelOrCase] = constant;
         } else {
-          // We don't handle labels yet.
-          return false;
+          compiler.reportWarning(node, "Unsupported: Labels on cases");
+          failure = true;
         }
       }
     }
+    if (failure) {
+      return false;
+    }
+
     // TODO(ngeoffray): Handle switch-instruction in bailout code.
     work.allowSpeculativeOptimization = false;
     // Then build a switch structure.
     HBasicBlock expressionStart = openNewBlock();
     visit(node.expression);
     HInstruction expression = pop();
     if (node.cases.isEmpty()) {
       return true;
     }
     HBasicBlock expressionEnd = current;
@@ skipping 91 matching lines @@
                                     statements,
                                     hasDefault,
                                     jumpHandler.target,
                                     jumpHandler.labels()),
         joinBlock);
 
     jumpHandler.close();
     return true;
   }
 
+  bool nonPrimitiveTypeOverridesEquals(Constant constant) {
+    // If constant is primitive, just return false. We know
+    // about the equals methods of num/String classes.
+    // [Map] and [List] does not override equals.

[floitsch, 2012/08/30 14:08:18]

do not

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

Done.

+    if (!constant.isConstructedObject()) return false;
+
+    ConstructedConstant constructedConstant = constant;
+    Type type = constructedConstant.type;
+    assert(type !== null);
+    Element element = type.element;

[ngeoffray, 2012/08/30 07:29:13]

The code below probably deserves to be in a helper method like in the Elements
class.

[Lasse Reichstein Nielsen, 2012/08/30 10:54:25]

Any wish for the abstraction level?
isOverridingEquals?
lookupEqualsImplementation?
lookupOperatorImplementation?

[ngeoffray, 2012/08/30 11:07:13]

I like the three of them :) The first two are easy to implement by calling the
third.

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

I went with
 typeOverridesObjectEquals
and
 lookupOperator

+    // If the type is not a class, we'll just assume it overrides
+    // operator==. Typedefs do, since [Function] does.
+    if (!element.isClass()) return true;
+    ClassElement classElement = element;
+    SourceString dartMethodName = Elements.constructOperatorName(
+        const SourceString('operator'),
+        const SourceString('=='));
+    Element operatorEq = classElement.lookupMember(dartMethodName);
+    if (operatorEq == null) return false;
+    // If the operator== declaration is in Object, it's not overridden.
+    return (operatorEq.getEnclosingClass() != compiler.objectClass);
+  }
+
 
   // Recursively build an if/else structure to match the cases.
   void buildSwitchCases(Link<Node> cases, HInstruction expression,
                         [int encounteredCaseTypes = 0]) {
     final int NO_TYPE = 0;
     final int INT_TYPE = 1;
     final int STRING_TYPE = 2;
     final int CONFLICT_TYPE = 3;
     int combine(int type1, int type2) => type1 | type2;
 
@@ skipping 40 matching lines @@
         Element equalsHelper = interceptors.getEqualsInterceptor();
         HInstruction target = new HStatic(equalsHelper);
         add(target);
         CaseMatch match = remainingCases.head;
         // TODO(lrn): Move the constant resolution to the resolver, so
         // we can report an error before reaching the backend.
         Constant constant =
             compiler.constantHandler.tryCompileNodeWithDefinitions(
                 match.expression, elements);
         if (constant !== null) {
-          if (constant.isInt()) {
-            // Report the first mixed-string/int type error only.
-            if (encounteredCaseTypes == STRING_TYPE) {
-              compiler.reportWarning(
-                  match, MessageKind.INVALID_CASE_EXPRESSION_TYPE);
-            }
-            encounteredCaseTypes = combine(encounteredCaseTypes, INT_TYPE);
-          } else if (constant.isString()) {
-            if (encounteredCaseTypes == INT_TYPE) {
-              compiler.reportWarning(
-                  match, MessageKind.INVALID_CASE_EXPRESSION_TYPE);
-            }
-            encounteredCaseTypes = combine(encounteredCaseTypes, STRING_TYPE);
-          } else {
-            compiler.reportWarning(match,
-                                   MessageKind.INVALID_CASE_EXPRESSION);
-            encounteredCaseTypes = CONFLICT_TYPE;
-          }
           stack.add(graph.addConstant(constant));
         } else {
-          // TODO(lrn): Remove this else branch, and make the constant
-          // evaluation mandatory when we are ready to break existing code using
-          // non constant-int-or-string expressions.
-          compiler.reportWarning(match,
-              'case expressions not compile-time constant int or string.');
           visit(match.expression);
-          encounteredCaseTypes = CONFLICT_TYPE;
         }
         push(new HEquals(target, pop(), expression));
       }
 
       // If this is the last expression, just return it.
       Link<Node> tail = skipLabels(remainingCases.tail);
       if (tail.isEmpty()) {
         left();
         return;
       }
@@ skipping 573 matching lines @@
           new HSubGraphBlockInformation(elseBranch.graph));
 
     HBasicBlock conditionStartBlock = conditionBranch.block;
     conditionStartBlock.setBlockFlow(info, joinBlock);
     SubGraph conditionGraph = conditionBranch.graph;
     HIf branch = conditionGraph.end.last;
     assert(branch is HIf);
     branch.blockInformation = conditionStartBlock.blockFlow;
   }
 }