Simplify generated code for string interpolation by delaying the constant folding.

Try to fix http://dartbug.com/3389 and http://dartbug.com/3240 by
using getTypeNameOf to improve browser compatibility.

R=lrn@google.com,floitsch@google.com
BUG=
TEST=

Review URL: https://chromiumcodereview.appspot.com//10544026

git-svn-id: https://dart.googlecode.com/svn/branches/bleeding_edge/dart@8332 260f80e4-7a28-3924-810f-c04153c831b5

lib/compiler/implementation/lib/js_helper.dart

@@ -376,17 +376,12 @@ checkGrowable(list, reason) {
   }
 }
 
-String stringToString(value) {
+String S(value) {
   var res = value.toString();
   if (res is !String) throw new IllegalArgumentException(value);
   return res;
 }
 
-String stringConcat(String receiver, String other) {
-  return JS('String', @'# + #', receiver, other);
-}
-
-
 class ListIterator<T> implements Iterator<T> {
   int i;
   List<T> list;
@@ -415,13 +410,10 @@ class Primitives {
   static final int DOLLAR_CHAR_VALUE = 36;
 
   static String objectToString(Object object) {
-    String name = JS('String', @'#.constructor.name', object);
-    if (name === null) {
-      name = JS('String', @'#.match(/^\s*function\s*\$?(\S*)\s*\(/)[1]',
-                JS('String', @'#.constructor.toString()', object));
-    } else {
-      if (name.charCodeAt(0) === DOLLAR_CHAR_VALUE) name = name.substring(1);
-    }
+    String name = getTypeNameOf(object);
+    // TODO(kasperl): If the namer gave us a fresh global name, we may
+    // want to remove the numeric suffix that makes it unique too.
+    if (name.charCodeAt(0) === DOLLAR_CHAR_VALUE) name = name.substring(1);
     return "Instance of '$name'";
   }
 

lib/compiler/implementation/ssa/builder.dart

@@ -2562,11 +2562,9 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {
       stack.add(graph.addConstantString(node.dartString, node));
       return;
     }
-    int offset = node.getBeginToken().charOffset;
-    StringBuilderVisitor stringBuilder =
-        new StringBuilderVisitor(this, offset);
+    StringBuilderVisitor stringBuilder = new StringBuilderVisitor(this);
     stringBuilder.visit(node);
-    stack.add(stringBuilder.result(node));
+    stack.add(stringBuilder.result);
   }
 
   void visitLiteralNull(LiteralNull node) {
@@ -2716,11 +2714,9 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {
   }
 
   visitStringInterpolation(StringInterpolation node) {
-    int offset = node.getBeginToken().charOffset;
-    StringBuilderVisitor stringBuilder =
-        new StringBuilderVisitor(this, offset);
+    StringBuilderVisitor stringBuilder = new StringBuilderVisitor(this);
     stringBuilder.visit(node);
-    stack.add(stringBuilder.result(node));
+    stack.add(stringBuilder.result);
   }
 
   visitStringInterpolationPart(StringInterpolationPart node) {
@@ -3395,33 +3391,13 @@ class SsaBuilder extends ResolvedVisitor implements Visitor {
  */
 class StringBuilderVisitor extends AbstractVisitor {
   final SsaBuilder builder;
-  final Element stringConcat;
-  final Element stringToString;
 
   /**
-   * Offset used for the synthetic operator token used by concat.
-   * Can probably be removed when we stop using String.operator+.
+   * The string value generated so far.
    */
-  final int offset;
+  HInstruction result = null;
 
-  /**
-   * Used to collect concatenated string literals into a single literal
-   * instead of introducing unnecessary concatenations.
-   */
-  DartString literalAccumulator = const LiteralDartString("");
-
-  /**
-   * The string value generated so far (not including that which is still
-   * in [literalAccumulator]).
-   */
-  HInstruction prefix = null;
-
-  StringBuilderVisitor(builder, this.offset)
-    : this.builder = builder,
-      stringConcat = builder.compiler.findHelper(
-          const SourceString("stringConcat")),
-      stringToString = builder.compiler.findHelper(
-          const SourceString("stringToString"));
+  StringBuilderVisitor(this.builder);
 
   void visit(Node node) {
     node.accept(this);
@@ -3432,27 +3408,9 @@ class StringBuilderVisitor extends AbstractVisitor {
   }
 
   void visitExpression(Node node) {
-    flushLiterals(node);
     node.accept(builder);
-    HInstruction asString = buildToString(node, builder.pop());
-    prefix = buildConcat(prefix, asString);
-  }
-
-  void visitLiteralNull(LiteralNull node) {
-    addLiteral(const LiteralDartString("null"));
-  }
-
-  void visitLiteralInt(LiteralInt node) {
-    addLiteral(new DartString.literal(node.value.toString()));
-  }
-
-  void visitLiteralDouble(LiteralDouble node) {
-    addLiteral(new DartString.literal(node.value.toString()));
-  }
-
-  void visitLiteralBool(LiteralBool node) {
-    addLiteral(node.value ? const LiteralDartString("true")
-                          : const LiteralDartString("false"));
+    HInstruction expression = builder.pop();
+    result = (result === null) ? expression : concat(result, expression);
   }
 
   void visitStringInterpolation(StringInterpolation node) {
@@ -3464,10 +3422,6 @@ class StringBuilderVisitor extends AbstractVisitor {
     visit(node.string);
   }
 
-  void visitLiteralString(LiteralString node) {
-    addLiteral(node.dartString);
-  }
-
   void visitStringJuxtaposition(StringJuxtaposition node) {
     node.visitChildren(this);
   }
@@ -3476,52 +3430,9 @@ class StringBuilderVisitor extends AbstractVisitor {
      node.visitChildren(this);
   }
 
-  /**
-   * Add another literal string to the literalAccumulator.
-   */
-  void addLiteral(DartString dartString) {
-    literalAccumulator = new DartString.concat(literalAccumulator, dartString);
-  }
-
-  /**
-   * Combine the strings in [literalAccumulator] into the prefix instruction.
-   * After this, the [literalAccumulator] is empty and [prefix] is non-null.
-   */
-  void flushLiterals(Node node) {
-    if (literalAccumulator.isEmpty()) {
-      if (prefix === null) {
-        prefix = builder.graph.addConstantString(literalAccumulator, node);
-      }
-      return;
-    }
-    HInstruction string =
-        builder.graph.addConstantString(literalAccumulator, node);
-    literalAccumulator = new DartString.empty();
-    if (prefix !== null) {
-      prefix = buildConcat(prefix, string);
-    } else {
-      prefix = string;
-    }
-  }
-
-  HInstruction buildConcat(HInstruction left, HInstruction right) {
-    HStatic target = new HStatic(stringConcat);
-    builder.add(target);
-    builder.push(new HAdd(target, left, right));
-    return builder.pop();
-  }
-
-  HInstruction buildToString(Node node, HInstruction input) {
-    HStatic target = new HStatic(stringToString);
-    builder.add(target);
-    builder.push(new HInvokeStatic(Selector.INVOCATION_1,
-                                   <HInstruction>[target, input],
-                                   HType.STRING));
-    return builder.pop();
-  }
-
-  HInstruction result(Node node) {
-    flushLiterals(node);
-    return prefix;
+  HInstruction concat(HInstruction left, HInstruction right) {
+    HInstruction instruction = new HStringConcat(left, right);
+    builder.add(instruction);
+    return instruction;
   }
 }

lib/compiler/implementation/ssa/codegen.dart

@@ -1856,6 +1856,50 @@ class SsaCodeGenerator implements HVisitor, HBlockInformationVisitor {
     endExpression(JSPrecedence.ASSIGNMENT_PRECEDENCE);
   }
 
+  void visitStringConcat(HStringConcat node) {
+    if (isEmptyString(node.left)) {
+      beginExpression(JSPrecedence.CALL_PRECEDENCE);
+      useStringified(node.right, JSPrecedence.EXPRESSION_PRECEDENCE);
+      endExpression(JSPrecedence.CALL_PRECEDENCE);
+   } else if (isEmptyString(node.right)) {
+      beginExpression(JSPrecedence.CALL_PRECEDENCE);
+      useStringified(node.left, JSPrecedence.EXPRESSION_PRECEDENCE);
+      endExpression(JSPrecedence.CALL_PRECEDENCE);
+    } else {
+      JSBinaryOperatorPrecedence operatorPrecedences = JSPrecedence.binary['+'];
+      beginExpression(operatorPrecedences.precedence);
+      useStringified(node.left, operatorPrecedences.left);
+      buffer.add(' + ');
+      // If the right hand side is a string concatenation itself it is
+      // safe to make it left associative.
+      int rightPrecedence = (node.right is HStringConcat)
+          ? JSPrecedence.ADDITIVE_PRECEDENCE
+          : operatorPrecedences.right;
+      useStringified(node.right, rightPrecedence);
+      endExpression(operatorPrecedences.precedence);
+    }
+  }
+
+  bool isEmptyString(HInstruction node) {
+    if (!node.isConstantString()) return false;
+    HConstant constant = node;
+    StringConstant string = constant.constant;
+    return string.value.length == 0;
+  }
+
+  void useStringified(HInstruction node, int precedence) {
+    if (node.isString()) {
+      use(node, precedence);
+    } else {
+      Element convertToString = compiler.findHelper(const SourceString("S"));
+      world.registerStaticUse(convertToString);
+      buffer.add(compiler.namer.isolateAccess(convertToString));
+      buffer.add('(');
+      use(node, JSPrecedence.EXPRESSION_PRECEDENCE);
+      buffer.add(')');
+    }
+  }
+
   void visitLiteralList(HLiteralList node) {
     generateArrayLiteral(node);
   }

lib/compiler/implementation/ssa/nodes.dart

@@ -51,6 +51,7 @@ interface HVisitor<R> {
   R visitShiftRight(HShiftRight node);
   R visitStatic(HStatic node);
   R visitStaticStore(HStaticStore node);
+  R visitStringConcat(HStringConcat node);
   R visitSubtract(HSubtract node);
   R visitSwitch(HSwitch node);
   R visitThis(HThis node);
@@ -304,6 +305,7 @@ class HBaseVisitor extends HGraphVisitor implements HVisitor {
   visitSwitch(HSwitch node) => visitControlFlow(node);
   visitStatic(HStatic node) => visitInstruction(node);
   visitStaticStore(HStaticStore node) => visitInstruction(node);
+  visitStringConcat(HStringConcat node) => visitInstruction(node);
   visitThis(HThis node) => visitParameterValue(node);
   visitThrow(HThrow node) => visitControlFlow(node);
   visitTry(HTry node) => visitControlFlow(node);
@@ -2232,6 +2234,18 @@ class HTypeConversion extends HCheck {
   bool hasSideEffects() => checked;
 }
 
+class HStringConcat extends HInstruction {
+  HStringConcat(HInstruction left, HInstrunction right)
+      : super(<HInstruction>[left, right]);
+  HType get guaranteedType() => HType.STRING;
+
+  HInstruction get left() => inputs[0];
+  HInstruction get right() => inputs[1];
+
+  accept(HVisitor visitor) => visitor.visitStringConcat(this);
+  toString() => "string concat";
+}
+
 /** Non-block-based (aka. traditional) loop information. */
 class HLoopInformation {
   final HBasicBlock header;

lib/compiler/implementation/ssa/optimize.dart

@@ -590,6 +590,19 @@ class SsaConstantFolder extends HBaseVisitor implements OptimizationPhase {
     if (field === null) return node;
     return new HFieldSet(field, node.inputs[0], node.inputs[1]);
   }
+
+  HInstruction visitStringConcat(HStringConcat node) {
+    DartString folded = const LiteralDartString("");
+    for (int i = 0; i < node.inputs.length; i++) {
+      HInstruction part = node.inputs[i];
+      if (!part.isConstant()) return node;
+      HConstant constant = part;
+      if (!constant.constant.isPrimitive()) return node;
+      PrimitiveConstant primitive = constant.constant;
+      folded = new DartString.concat(folded, primitive.toDartString());
+    }
+    return graph.addConstantString(folded, node);
+  }
 }
 
 class SsaCheckInserter extends HBaseVisitor implements OptimizationPhase {

lib/compiler/implementation/ssa/tracer.dart

@@ -381,6 +381,12 @@ class HInstructionStringifier implements HVisitor<String> {
     return "Static $lhs = ${temporaryId(node.inputs[0])}";
   }
 
+  String visitStringConcat(HStringConcat node) {
+    var leftId = temporaryId(node.left);
+    var rightId = temporaryId(node.right);
+    return "StringConcat: $leftId + $rightId";
+  }
+
   String visitSubtract(HSubtract node) => visitInvokeStatic(node);
 
   String visitSwitch(HSwitch node) {