Check compiler exitCode.

Check compiler exitCode.

R=ahe@google.com

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

[ahe, 2013-11-21 12:44:52]

Nice! I'd like to talk face to face about the big refactoring of dart2js.dart.

https://codereview.chromium.org/80793002/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/dart2js.dart (right):

https://codereview.chromium.org/80793002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/dart2js.dart:48: class _Dart2Js {
If you think it makes sense to create a class here, we can discuss that, but I'm
not sure I see why it is necessary. If convince me that it is, I think it should
be done as a separate CL.

https://codereview.chromium.org/80793002/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/dart2js.dart:616: if (exception is
CompilerCrashedException) {
This looks like a break down of the API to me.

CompilerCrashedException now belongs in ../compiler.dart, but I don't think the
concept CompilerCrashedException belongs there.

The idea is that this library also serves as a demonstration of how to use the
compiler API (but without importing implementation details).

This is also why I'm concerned about importing source_file.dart and
source_file_provider.dart: these are also becoming compiler implementation
details, and we have no separation between implementation details and compiler
API.

[Johnni Winther, 2013-11-25 09:02:00]

PTAL

[ahe, 2013-11-26 11:18:24]

LGTM!

https://codereview.chromium.org/80793002/diff/60001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/80793002/diff/60001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/compiler.dart:714: void
pleaseReportCrash(String message) {
I think you can eliminate this argument as well (otherwise it will be
duplicated).

https://codereview.chromium.org/80793002/diff/60001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/dart2js.dart (right):

https://codereview.chromium.org/80793002/diff/60001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/dart2js.dart:7: import 'dart:async';
use show?

https://codereview.chromium.org/80793002/diff/60001/tests/compiler/dart2js/ex...
File tests/compiler/dart2js/exit_code_test.dart (right):

https://codereview.chromium.org/80793002/diff/60001/tests/compiler/dart2js/ex...
tests/compiler/dart2js/exit_code_test.dart:113: 
Extra line.

https://codereview.chromium.org/80793002/diff/60001/tests/compiler/dart2js/ex...
tests/compiler/dart2js/exit_code_test.dart:118: void onTest(String testMarker,
String testType) {
Add lines around function.

https://codereview.chromium.org/80793002/diff/60001/tests/compiler/dart2js/ex...
tests/compiler/dart2js/exit_code_test.dart:223: }));
Can we add a then() section here that checks that a counter of all tests has the
expected value?

[Johnni Winther, 2013-11-27 07:58:03]

https://codereview.chromium.org/80793002/diff/60001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/80793002/diff/60001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/compiler.dart:714: void
pleaseReportCrash(String message) {
On 2013/11/26 11:18:25, ahe wrote:
> I think you can eliminate this argument as well (otherwise it will be
> duplicated).

Done.

https://codereview.chromium.org/80793002/diff/60001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/dart2js.dart (right):

https://codereview.chromium.org/80793002/diff/60001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/dart2js.dart:7: import 'dart:async';
On 2013/11/26 11:18:25, ahe wrote:
> use show?

Done.

https://codereview.chromium.org/80793002/diff/60001/tests/compiler/dart2js/ex...
File tests/compiler/dart2js/exit_code_test.dart (right):

https://codereview.chromium.org/80793002/diff/60001/tests/compiler/dart2js/ex...
tests/compiler/dart2js/exit_code_test.dart:113: 
On 2013/11/26 11:18:25, ahe wrote:
> Extra line.

Done.

https://codereview.chromium.org/80793002/diff/60001/tests/compiler/dart2js/ex...
tests/compiler/dart2js/exit_code_test.dart:118: void onTest(String testMarker,
String testType) {
On 2013/11/26 11:18:25, ahe wrote:
> Add lines around function.

Done.

https://codereview.chromium.org/80793002/diff/60001/tests/compiler/dart2js/ex...
tests/compiler/dart2js/exit_code_test.dart:223: }));
On 2013/11/26 11:18:25, ahe wrote:
> Can we add a then() section here that checks that a counter of all tests has
the
> expected value?

Done.

[Johnni Winther, 2013-11-27 08:19:15]

Committed patchset #3 manually as r30696 (presubmit successful).