use js_ast instead of strings to generate JS

use js_ast instead of strings to generate JS
fixes https://github.com/dart-lang/dev_compiler/issues/61

R=sigmund@google.com, vsm@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/9d2f597d369791df86ed6fc9402464065456f8c4

[Jennifer Messerly, 2015-02-25 00:35:01]

Patchset #1 (id:1) has been deleted

[Jennifer Messerly, 2015-02-25 00:47:18]

Patchset #1 (id:20001) has been deleted

[Jennifer Messerly, 2015-02-25 15:59:40]

Patchset #1 (id:40001) has been deleted

[Jennifer Messerly, 2015-02-25 17:04:14]

Patchset #1 (id:60001) has been deleted

[Jennifer Messerly, 2015-02-25 18:06:20]

Patchset #1 (id:80001) has been deleted

[Jennifer Messerly, 2015-02-25 18:06:26]

Patchset #1 (id:100001) has been deleted

[Jennifer Messerly, 2015-02-25 18:06:33]

Patchset #1 (id:120001) has been deleted

[Jennifer Messerly, 2015-02-25 18:07:15]

Patchset #1 (id:140001) has been deleted

[Jennifer Messerly, 2015-02-25 18:08:33]

jmesserly@google.com changed reviewers:
+ sigmund@google.com

[vsm, 2015-02-25 18:26:33]

vsm@google.com changed reviewers:
+ vsm@google.com

[vsm, 2015-02-25 18:26:34]

nice!

https://codereview.chromium.org/949383003/diff/160001/test/codegen/expect/cor...
File test/codegen/expect/core/core.js (right):

https://codereview.chromium.org/949383003/diff/160001/test/codegen/expect/cor...
test/codegen/expect/core/core.js:18: class _ListConstructorSentinel extends
dynamic {
Extends dynamic?

https://codereview.chromium.org/949383003/diff/160001/test/codegen/expect/htm...
File test/codegen/expect/html_input_c/html_input_c.js (right):

https://codereview.chromium.org/949383003/diff/160001/test/codegen/expect/htm...
test/codegen/expect/html_input_c/html_input_c.js:8: set y() {}
Lost the param to the setter.

[Jennifer Messerly, 2015-02-25 18:33:54]

https://codereview.chromium.org/949383003/diff/160001/test/codegen/expect/cor...
File test/codegen/expect/core/core.js (right):

https://codereview.chromium.org/949383003/diff/160001/test/codegen/expect/cor...
test/codegen/expect/core/core.js:18: class _ListConstructorSentinel extends
dynamic {
On 2015/02/25 18:26:33, vsm wrote:
> Extends dynamic?

I dug into this a bit before sending out the CL ... what's going on is, JSInt
was an unresolved type. We used to emit the raw source name for "extends", but
now type printing is more consistent, and we use the resolved type name, which
is dynamic.

https://codereview.chromium.org/949383003/diff/160001/test/codegen/expect/htm...
File test/codegen/expect/html_input_c/html_input_c.js (right):

https://codereview.chromium.org/949383003/diff/160001/test/codegen/expect/htm...
test/codegen/expect/html_input_c/html_input_c.js:8: set y() {}
On 2015/02/25 18:26:33, vsm wrote:
> Lost the param to the setter.

yeah ... it's kind of on purpose, because this is our dummy setter. It's our way
of indicating that this particular lazy property is writable. That said, if we
can enforce read-only for static fields at compile time, we could just always
have it be writable on the JS side.

The corresponding Dart code for this is:

    int y = 3 + w;

[Jennifer Messerly, 2015-02-25 18:36:20]

here's some context for my previous 2 comments

https://codereview.chromium.org/949383003/diff/160001/lib/src/codegen/js_code...
File lib/src/codegen/js_codegen.dart (left):

https://codereview.chromium.org/949383003/diff/160001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:187: void visitTypeName(TypeName node) {
types used to be handled in two ways, here and _writeTypeName. Now they both
work the same way, and use the resolved type always.

https://codereview.chromium.org/949383003/diff/160001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:1010: if (!node.isFinal) out.write('set $name(x)
{},\n');
this is the dummy setter code. I agree we should find a better way to handle
this long term :)

[vsm, 2015-02-25 18:46:51]

lgtm

[Siggi Cherem (dart-lang), 2015-02-25 18:56:02]

lgtm

This looks super super nice!

https://codereview.chromium.org/949383003/diff/160001/lib/src/codegen/js_code...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/949383003/diff/160001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:400: return js.statement('this.#(#);', [name,
node.argumentList.accept(this)]);
would it work if you did '#.call(this, #)'?

I'm assuming they are equivalent, but the old way read more like a redirecting
constructor call. either way it's fine by me though

https://codereview.chromium.org/949383003/diff/160001/test/codegen/expect/cor...
File test/codegen/expect/core/core.js (left):

https://codereview.chromium.org/949383003/diff/160001/test/codegen/expect/cor...
test/codegen/expect/core/core.js:28: 
I think it's better with the separation between classes and some empty lines
here and there.

I added https://github.com/dart-lang/dev_compiler/issues/66 to track of changes
we'd like in js_ast to make the output a bit better.

[Jennifer Messerly, 2015-02-25 19:07:51]

Thank you!

https://codereview.chromium.org/949383003/diff/160001/lib/src/codegen/js_code...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/949383003/diff/160001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:400: return js.statement('this.#(#);', [name,
node.argumentList.accept(this)]);
On 2015/02/25 18:56:02, Siggi Cherem (dart-lang) wrote:
> would it work if you did '#.call(this, #)'?
> 
> I'm assuming they are equivalent, but the old way read more like a redirecting
> constructor call. either way it's fine by me though

ah, good catch! this was actually a bug fix, I should have called it out ...
what happened is
https://github.com/dart-lang/dev_compiler/commit/601940ad199c77df5af4f26ac087...
changed the call signature to be this.ConstructorName(args) ... but the code
here wasn't updated. I'll add a test to constructors.dart if we don't have one
yet.

https://codereview.chromium.org/949383003/diff/160001/test/codegen/expect/cor...
File test/codegen/expect/core/core.js (left):

https://codereview.chromium.org/949383003/diff/160001/test/codegen/expect/cor...
test/codegen/expect/core/core.js:28: 
On 2015/02/25 18:56:02, Siggi Cherem (dart-lang) wrote:
> I think it's better with the separation between classes and some empty lines
> here and there.
> 
> I added https://github.com/dart-lang/dev_compiler/issues/66 to track of
changes
> we'd like in js_ast to make the output a bit better.

awesome, thank you!

https://codereview.chromium.org/949383003/diff/160001/test/codegen/expect/cor...
File test/codegen/expect/core/core.js (right):

https://codereview.chromium.org/949383003/diff/160001/test/codegen/expect/cor...
test/codegen/expect/core/core.js:18: class _ListConstructorSentinel extends
dynamic {
On 2015/02/25 18:33:53, John Messerly wrote:
> On 2015/02/25 18:26:33, vsm wrote:
> > Extends dynamic?
> 
> I dug into this a bit before sending out the CL ... what's going on is, JSInt
> was an unresolved type. We used to emit the raw source name for "extends", but
> now type printing is more consistent, and we use the resolved type name, which
> is dynamic.

fyi, filed https://github.com/dart-lang/dev_compiler/issues/67

[Jennifer Messerly, 2015-02-25 19:14:23]

Committed patchset #2 (id:180001) manually as
9d2f597d369791df86ed6fc9402464065456f8c4 (presubmit successful).