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

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=

Committed: https://code.google.com/p/dart/source/detail?r=8332

[Lasse Reichstein Nielsen, 2012-06-06 12:11:40]

LvGTM.

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
File lib/compiler/implementation/lib/js_helper.dart (right):

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
lib/compiler/implementation/lib/js_helper.dart:379: String S(value) {
Or use a German scharf-S to represent the "stringString" name :P

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:1820: JSPrecedence rightPrecedence
= (node.inputs[1] is HStringConcat)
JSPrecedence -> int.
The JSPrecedence class is just a static class container for the int-constants
(i.e., it's just a namespace).

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:1833:
buffer.add(compiler.namer.isolateAccess(convertToString));
Maybe we should have a function that does all this, so we can just write:
  addHelper(const SourceString("S"));

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/nodes.dart (right):

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/nodes.dart:2215: toString() => "string concat";
Should this class be allowed to have more than two inputs? Nothing seems to
prevent it, the optimize code handles it, but codegen doesn't.

If you don't expect to allow 3+ inputs later, consider adding left/right getters
to extract the inputs. Or just extend HInvokeBinary, if possible. Otherwise make
a comment about it.

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/optimize.dart (right):

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/optimize.dart:605: for (int i = 0; i <
node.inputs.length; i++) {
This seems to assume more than two inputs. Intentional?

[floitsch, 2012-06-06 12:13:34]

LGTM.
please adapt the tracer for the new HNode.
Add tests for the fixed bugs.

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:3293: HInstruction prefix = null;
Maybe rename to 'result' and remove result-method?

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:3334: HInstruction result(Node
node) {
If you keep the result-method, remove the argument?

[kasperl, 2012-06-06 13:31:03]

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/builder.dart:3293: HInstruction prefix = null;
On 2012/06/06 12:13:34, floitsch wrote:
> Maybe rename to 'result' and remove result-method?

Done.

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/codegen.dart (right):

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/codegen.dart:1820: JSPrecedence rightPrecedence
= (node.inputs[1] is HStringConcat)
On 2012/06/06 12:11:40, Lasse Reichstein Nielsen wrote:
> JSPrecedence -> int.
> The JSPrecedence class is just a static class container for the int-constants
> (i.e., it's just a namespace).

Done.

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
File lib/compiler/implementation/ssa/nodes.dart (right):

https://chromiumcodereview.appspot.com/10544026/diff/1/lib/compiler/implement...
lib/compiler/implementation/ssa/nodes.dart:2215: toString() => "string concat";
On 2012/06/06 12:11:40, Lasse Reichstein Nielsen wrote:
> Should this class be allowed to have more than two inputs? Nothing seems to
> prevent it, the optimize code handles it, but codegen doesn't.
> 
> If you don't expect to allow 3+ inputs later, consider adding left/right
getters
> to extract the inputs. Or just extend HInvokeBinary, if possible. Otherwise
make
> a comment about it.
> 

Changed the constructor to take exactly two arguments (left and right).

[sra1, 2012-06-06 18:07:06]

https://chromiumcodereview.appspot.com/10544026/diff/7/lib/compiler/implement...
File lib/compiler/implementation/lib/js_helper.dart (right):

https://chromiumcodereview.appspot.com/10544026/diff/7/lib/compiler/implement...
lib/compiler/implementation/lib/js_helper.dart:413: String name =
getTypeNameOf(object);
I think you should use getTypeNameOf only if the object.constructor.name is not
a string.

Regular Dart objects will have a constructor name - the one you arranged for.

The principal purpose of getTypeNameOf is to extract a tag from the object
somehow for native dispatch.
We make it lie for browser compat reasons.
It was never intended to work for general objects, and making it do so will
complicate the native dispatch.
For example, we rename 'Window' to 'DOMWindow', but you may also have another
pure Dart class called Window, and that would get renamed by mistake.

Perhaps we should rename getTypeNameOf to getDispatchTagForNativeClass.