Extend compiler API to support multiple outputs.

Extend compiler API to support multiple outputs.

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

[ngeoffray, 2013-01-10 12:15:21]

Nice, LGTM but I would wait for Johnni's comments too.

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/compiler.dart (right):

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/compiler.dart:101: }
Use the ternary operator?

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/compiler.dart:1089: /// A sink
that drains into /dev/null.
Really?

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/compiler.dart:1102: static
outputProvider(String name, String extension) {
Use the typedef for the return type?

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart
(right):

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:2170:
..close();
Would that look nicer if it was just below the '.' of line 2168?

[Johnni Winther, 2013-01-10 12:25:25]

lgtm

[kasperl, 2013-01-18 08:30:09]

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/compiler.dart (right):

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/compiler.dart:98: String outputType = 'js';
I'd probably use a ?: to initialize outputType.

[ahe, 2013-01-25 15:50:08]

Thank you, Johnni, Nicolas, and Kasper.

I have addressed your comments, and uploaded a new patch set. However, I will
also have to rebase this on Johnni's changes to source_file_provider.dart.

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/compiler.dart (right):

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/compiler.dart:98: String outputType = 'js';
On 2013/01/18 08:30:09, kasperl wrote:
> I'd probably use a ?: to initialize outputType.

I prefer to not use the ?: because it doesn't fit one line, and because I think
it is more clear that 'js' is the default.

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/compiler.dart:101: }
On 2013/01/10 12:15:21, ngeoffray wrote:
> Use the ternary operator?

See above.

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/compiler.dart:1089: /// A sink
that drains into /dev/null.
On 2013/01/10 12:15:21, ngeoffray wrote:
> Really?

For all intents and purposes. If you can observe the difference, you're doing
something wrong ;-)

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/compiler.dart:1102: static
outputProvider(String name, String extension) {
On 2013/01/10 12:15:21, ngeoffray wrote:
> Use the typedef for the return type?

Done.

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart
(right):

https://codereview.chromium.org/11828043/diff/1006/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:2170:
..close();
On 2013/01/10 12:15:21, ngeoffray wrote:
> Would that look nicer if it was just below the '.' of line 2168?

It might look nicer, but it I think it would be confusing. One might think that
we called "add" and "close" on "compiler", not the result of
"compiler.outputProvider('', 'js.map')"

[ahe, 2013-01-28 15:29:48]

PTAL

Patch #4 contains a fix to restore counting of output characters and updates to
various subclasses of the apiimpl compiler. These are minor changes, but not
something I'm comfortable submitting without my reviewers taking another look.

Cheers,
Peter

[Johnni Winther, 2013-01-28 18:48:31]

Still LGTM