Steps towards making dart2js JS AST templates an indepentent library.

Steps towards making dart2js JS AST templates an indepentent library.

- Removed unused type Blob
- Remove use of CodeBuffer, Compiler, SourceInformation
- Moved existing code into library js_ast
- Added new library js that re-exports js_ast and adds dart2js specific code.

The next step is to move the code into its own package under pkg.

BUG=
R=floitsch@google.com

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

[sra1, 2015-02-10 05:38:58]

sra@google.com changed reviewers:
+ sigurdm@google.com

[sra1, 2015-02-10 05:38:58]

This is a work in progress.
I will fix the new TODOs before committing.

[sra1, 2015-02-10 05:39:12]

sra@google.com changed reviewers:
+ floitsch@google.com

[floitsch, 2015-02-10 14:43:52]

DBC.

https://codereview.chromium.org/915533002/diff/1/pkg/compiler/lib/src/js/prin...
File pkg/compiler/lib/src/js/printer.dart (right):

https://codereview.chromium.org/915533002/diff/1/pkg/compiler/lib/src/js/prin...
pkg/compiler/lib/src/js/printer.dart:19: * The context contains
nit: we tend to use "///" now (and this file seems to use it already).

[sra1, 2015-02-11 00:13:07]

https://chromiumcodereview.appspot.com/915533002/diff/1/pkg/compiler/lib/src/...
File pkg/compiler/lib/src/js/printer.dart (right):

https://chromiumcodereview.appspot.com/915533002/diff/1/pkg/compiler/lib/src/...
pkg/compiler/lib/src/js/printer.dart:19: * The context contains
On 2015/02/10 14:43:52, floitsch wrote:
> nit: we tend to use "///" now (and this file seems to use it already).

Done. I prefer /**/ for longer comments, but /// looks OK to me for up to 2-3
lines.

[sra1, 2015-02-11 02:29:55]

PTAL.

I'd like to land this tomorrow if possible so I can work on getting the code to
build and snapshot from pkg.

I'd like to explore a more generic way to have the Await and DartYield nodes and
the AsyncModifier after the move to pkg.

[floitsch, 2015-02-11 16:01:13]

LGTM with one concern.

https://chromiumcodereview.appspot.com/915533002/diff/40001/pkg/compiler/lib/...
File pkg/compiler/lib/src/js/js.dart (right):

https://chromiumcodereview.appspot.com/915533002/diff/40001/pkg/compiler/lib/...
pkg/compiler/lib/src/js/js.dart:33: class Dart2JSJavaScriptPrintingContext
extends JavaScriptPrintingContext {
I'm not sure I like this.

The JavaScriptPrintingContext has a buffer field that has a StringBuffer. That
one seems to be unused now.

I wouldn't feel comfortable that I caught all overloads where I have to replace
"buffer" with "outBuffer". That is, if we add an "emitAll" to the
JavaScriptPrintingContext that does buffer.addAll(strings) we would silently
break this class.

I would rather make JavaScriptPrintingContext an interface and remove the
"buffer" from it.

https://chromiumcodereview.appspot.com/915533002/diff/40001/pkg/compiler/lib/...
File pkg/compiler/lib/src/js/printer.dart (right):

https://chromiumcodereview.appspot.com/915533002/diff/40001/pkg/compiler/lib/...
pkg/compiler/lib/src/js/printer.dart:12: JavaScriptPrintingOptions(
New line before.

https://chromiumcodereview.appspot.com/915533002/diff/40001/pkg/compiler/lib/...
pkg/compiler/lib/src/js/printer.dart:35: void emit(String string) {
New line before.

https://chromiumcodereview.appspot.com/915533002/diff/40001/pkg/compiler/lib/...
pkg/compiler/lib/src/js/printer.dart:38: String getText() => buffer.toString();
New line before.

https://chromiumcodereview.appspot.com/915533002/diff/40001/pkg/compiler/lib/...
pkg/compiler/lib/src/js/printer.dart:132: // TODO(sra): This formatting choice
via JavaScriptPrintingOptions.
I don't understand this TODO.

[sra1, 2015-02-11 18:20:04]

New patchsets have been uploaded after l-g-t-m from floitsch@google.com

[sra1, 2015-02-11 18:20:19]

https://chromiumcodereview.appspot.com/915533002/diff/40001/pkg/compiler/lib/...
File pkg/compiler/lib/src/js/js.dart (right):

https://chromiumcodereview.appspot.com/915533002/diff/40001/pkg/compiler/lib/...
pkg/compiler/lib/src/js/js.dart:33: class Dart2JSJavaScriptPrintingContext
extends JavaScriptPrintingContext {
On 2015/02/11 16:01:13, floitsch wrote:
> I'm not sure I like this.
> 
> The JavaScriptPrintingContext has a buffer field that has a StringBuffer. That
> one seems to be unused now.
> 
> I wouldn't feel comfortable that I caught all overloads where I have to
replace
> "buffer" with "outBuffer". That is, if we add an "emitAll" to the
> JavaScriptPrintingContext that does buffer.addAll(strings) we would silently
> break this class.
> 
> I would rather make JavaScriptPrintingContext an interface and remove the
> "buffer" from it.

I agree.

JavaScriptPrintingContext *is* the interface you desire.

SimpleJavaScriptPrintingContext, not used here, is the class with a buffer. It
is used from a test and presumably a new client of the library.

Since all dummy methods are overridden, I have changed 'extends' to
'implements'.

https://chromiumcodereview.appspot.com/915533002/diff/40001/pkg/compiler/lib/...
File pkg/compiler/lib/src/js/printer.dart (right):

https://chromiumcodereview.appspot.com/915533002/diff/40001/pkg/compiler/lib/...
pkg/compiler/lib/src/js/printer.dart:12: JavaScriptPrintingOptions(
On 2015/02/11 16:01:13, floitsch wrote:
> New line before.

Done.

https://chromiumcodereview.appspot.com/915533002/diff/40001/pkg/compiler/lib/...
pkg/compiler/lib/src/js/printer.dart:35: void emit(String string) {
On 2015/02/11 16:01:13, floitsch wrote:
> New line before.

Done.

https://chromiumcodereview.appspot.com/915533002/diff/40001/pkg/compiler/lib/...
pkg/compiler/lib/src/js/printer.dart:38: String getText() => buffer.toString();
On 2015/02/11 16:01:13, floitsch wrote:
> New line before.

Done.

https://chromiumcodereview.appspot.com/915533002/diff/40001/pkg/compiler/lib/...
pkg/compiler/lib/src/js/printer.dart:132: // TODO(sra): This formatting choice
via JavaScriptPrintingOptions.
On 2015/02/11 16:01:13, floitsch wrote:
> I don't understand this TODO.

I fixed the TODO issue.  Thanks for catching the remaining comment.

[sra1, 2015-02-11 18:25:04]

Committed patchset #5 (id:80001) manually as 43695 (presubmit successful).