Make string juxtaposition combine properly with string interpolations.

frog/leg/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 1387 matching lines @@
     visit(node.receiver);
     HNot not = new HNot(popBoolified());
     push(not);
   }
 
   void visitUnary(Send node, Operator op) {
     assert(node.argumentsNode is Prefix);
     visit(node.receiver);
     assert(op.token.kind !== PLUS_TOKEN);
     HInstruction operand = pop();
+
     HInstruction target =
         new HStatic(interceptors.getPrefixOperatorInterceptor(op));
     add(target);
     HInvokeUnary result;
     switch (op.source.stringValue) {
       case "-": result = new HNegate(target, operand); break;
       case "~": result = new HBitNot(target, operand); break;
       default: unreachable();
     }
     // See if we can constant-fold right away. This avoids rewrites later on.
@@ skipping 655 matching lines @@
   }
 
   void visitLiteralBool(LiteralBool node) {
     stack.add(graph.addConstantBool(node.value));
   }
 
   void visitLiteralString(LiteralString node) {
     stack.add(graph.addConstantString(node.dartString));
   }
 
-  void visitLiteralStringJuxtaposition(LiteralStringJuxtaposition node) {
-    visitLiteralString(node);
+  void visitStringJuxtaposition(StringJuxtaposition node) {
+    if (!node.isInterpolation) {
+      // This is a simple string with no interpolations.
+      stack.add(graph.addConstantString(node.dartString));
+      return;
+    }
+    int offset = node.getBeginToken().charOffset;
+    StringBuilderVisitor stringBuilder =
+        new StringBuilderVisitor(this, offset);
+    stringBuilder.visit(node);
+    stack.add(stringBuilder.result());
   }
 
   void visitLiteralNull(LiteralNull node) {
     stack.add(graph.addConstantNull());
   }
 
   visitNodeList(NodeList node) {
     for (Link<Node> link = node.nodes; !link.isEmpty(); link = link.tail) {
       if (isAborted()) {
         compiler.reportWarning(link.head, 'dead code');
@@ skipping 102 matching lines @@
     open(joinBlock);
 
     localsHandler.mergeWith(thenLocals, joinBlock);
     HPhi phi = new HPhi.manyInputs(null, [thenInstruction, elseInstruction]);
     joinBlock.addPhi(phi);
     stack.add(phi);
   }
 
   visitStringInterpolation(StringInterpolation node) {
     int offset = node.getBeginToken().charOffset;
-    Operator op = new Operator(new StringToken(PLUS_INFO, "+", offset));
-    HInstruction target = new HStatic(interceptors.getOperatorInterceptor(op));
-    add(target);
-    visit(node.string);
-    // Handle the parts here, to avoid recreating [target].
-    for (StringInterpolationPart part in node.parts) {
-      HInstruction prefix = pop();
-      visit(part.expression);
-      push(new HAdd(target, prefix, pop()));
-      prefix = pop();
-      visit(part.string);
-      push(new HAdd(target, prefix, pop()));
-    }
+    StringBuilderVisitor stringBuilder =
+        new StringBuilderVisitor(this, offset);
+    stringBuilder.visit(node);
+    stack.add(stringBuilder.result());
   }
 
   visitStringInterpolationPart(StringInterpolationPart node) {
     // The parts are iterated in visitStringInterpolation.
     unreachable();
   }
 
   visitEmptyStatement(EmptyStatement node) {
     // Do nothing, empty statement.
   }
@@ skipping 471 matching lines @@
     buildBody() {
       // TODO(lrn): Make sure to take continue into account.
       visit(body);
       if (isAborted()) {
         compiler.reportWarning(body, "aborting loop body");
       }
     }
     handleIf(buildBody, null);
   }
 }
+
+/**
+ * Visitor that handles generation of string literals (LiteralString,
+ * StringInterpolation), and otherwise delegates to the given visitor for
+ * non-literal subexpressions.
+ * TODO(lrn): Consider whether to handle compile time constant int/boolean
+ * expressions as well.
+ */
+class StringBuilderVisitor extends AbstractVisitor {
+  final SsaBuilder builder;
+  // Offset used for the synthetic operator token used by concat.

[ahe, 2012/03/19 15:53:06]

Documentation comment.

[Lasse Reichstein Nielsen, 2012/03/20 09:34:33]

Done.

+  // Can probably be removed when we stop using String.operator+.
+  final int offset;
+  // Used to collect concatenated string literals into a single literal
+  // instead of introducing unnecessary concatenations.

[ahe, 2012/03/19 15:53:06]

Ditto.

[Lasse Reichstein Nielsen, 2012/03/20 09:34:33]

Done.

+  DartString accumulator = const LiteralDartString("");
+  // The string value generated so far (not including that which is still

[ahe, 2012/03/19 15:53:06]

Ditto.

[Lasse Reichstein Nielsen, 2012/03/20 09:34:33]

Done.

+  // in [accumulator]).
+  HInstruction prefix = null;
+
+  StringBuilderVisitor(this.builder, this.offset);
+
+  void visit(Node node) {
+    node.accept(this);
+  }
+
+  void visitNode() {
+    unreachable();

[ahe, 2012/03/19 15:53:06]

There is no node argument to visitNode, and there is no location given if the
compiler trips over this.

How about:

visitNode(Node node) {
  compiler.internalError('unexpected node', node: node); 
}

[Lasse Reichstein Nielsen, 2012/03/20 09:34:33]

Good catch.
It'll most likely never get hit (requires a statement as subexpression of a
StringInterpolation), but better be correct.

+  }
+
+  void visitExpression(Node node) {
+    flushAccumulator();
+    node.accept(builder);
+    prefix = concat(prefix, builder.pop());
+  }
+
+  void visitStringInterpolation(StringInterpolation node) {
+    node.visitChildren(this);
+  }
+
+  void visitStringInterpolationPart(StringInterpolationPart node) {
+    visit(node.expression);
+    visit(node.string);
+  }
+
+  void visitLiteralString(LiteralString node) {
+    accumulator = new DartString.concat(accumulator, node.dartString);
+  }
+
+  void visitStringJuxtaposition(StringJuxtaposition node) {
+    node.visitChildren(this);
+  }
+
+  void visitNodeList(NodeList node) {
+     node.visitChildren(this);
+  }
+
+  /**
+   * Combine the strings in [accumulator] into the prefix instruction.
+   * After this, the [accumulator] is empty and [prefix] is non-null.
+   */
+  void flushAccumulator() {
+    if (accumulator.isEmpty()) {
+      if (prefix === null) {
+        prefix = builder.graph.addConstantString(accumulator);
+      }
+      return;
+    }
+    HInstruction string = builder.graph.addConstantString(accumulator);
+    accumulator = new DartString.empty();
+    if (prefix !== null) {
+      prefix = concat(prefix, string);
+    } else {
+      prefix = string;
+    }
+  }
+
+  HInstruction concat(HInstruction left, HInstruction right) {
+    Operator op = new Operator(new StringToken(PLUS_INFO, "+", offset));
+    HStatic target =
+        new HStatic(builder.interceptors.getOperatorInterceptor(op));
+    builder.add(target);
+    HInstruction concat = new HAdd(target, left, right);
+    builder.add(concat);
+    return concat;
+  }
+
+  HInstruction result() {
+    flushAccumulator();
+    return prefix;
+  }
+}