Split loader from the rest of the compiler. This adds several abstractions to

Split loader from the rest of the compiler. This adds several abstractions to
make the decoupling easier. Some of the new abstractions should be revisited,
but it's a step forward :).

R=het@google.com

Committed: https://github.com/dart-lang/sdk/commit/bada3cac9de026784015074b44ddf1def8a79670

[Siggi Cherem (dart-lang), 2016-03-21 19:22:58]

Patchset #1 (id:1) has been deleted

[Siggi Cherem (dart-lang), 2016-03-21 19:25:48]

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

[Siggi Cherem (dart-lang), 2016-03-21 19:25:49]

https://codereview.chromium.org/1819053002/diff/20001/pkg/compiler/lib/src/ap...
File pkg/compiler/lib/src/apiimpl.dart (left):

https://codereview.chromium.org/1819053002/diff/20001/pkg/compiler/lib/src/ap...
pkg/compiler/lib/src/apiimpl.dart:398: diagnoseCrashInUserCode('Uncaught
exception in input provider', ex, s);
diagnosCrashInUserCode was always empty and unused, it seemed to me that we can
debug crashes like these directly without having to add the extra callback.

[Harry Terkelsen, 2016-03-22 21:07:57]

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

[Harry Terkelsen, 2016-03-22 21:07:58]

https://codereview.chromium.org/1819053002/diff/20001/pkg/compiler/lib/src/ap...
File pkg/compiler/lib/src/apiimpl.dart (right):

https://codereview.chromium.org/1819053002/diff/20001/pkg/compiler/lib/src/ap...
pkg/compiler/lib/src/apiimpl.dart:29: import 'environment.dart';
Did you forget to upload environment.dart?

[Siggi Cherem (dart-lang), 2016-03-22 21:10:41]

https://codereview.chromium.org/1819053002/diff/20001/pkg/compiler/lib/src/ap...
File pkg/compiler/lib/src/apiimpl.dart (right):

https://codereview.chromium.org/1819053002/diff/20001/pkg/compiler/lib/src/ap...
pkg/compiler/lib/src/apiimpl.dart:29: import 'environment.dart';
On 2016/03/22 21:07:58, Harry Terkelsen wrote:
> Did you forget to upload environment.dart?

Indeed I did. Just uploaded it.

[Harry Terkelsen, 2016-03-22 21:49:28]

lgtm

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/co...
File pkg/compiler/lib/src/compiler.dart (right):

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/co...
pkg/compiler/lib/src/compiler.dart:291: fromEnvironment(String name) =>
environment.valueOf(name);
add @deprecated

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/co...
pkg/compiler/lib/src/compiler.dart:402: this, enableConditionalDirectives:
options.enableConditionalDirectives),
long line

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/co...
pkg/compiler/lib/src/compiler.dart:413: this, enableConditionalDirectives:
options.enableConditionalDirectives),
ditto

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/co...
pkg/compiler/lib/src/compiler.dart:415: this, enableConditionalDirectives:
options.enableConditionalDirectives),
ditto

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/li...
File pkg/compiler/lib/src/library_loader.dart (right):

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/li...
pkg/compiler/lib/src/library_loader.dart:273: /// Translates interal uris (like
dart:core) to a disk location.
s/interal/internal/

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/li...
pkg/compiler/lib/src/library_loader.dart:276: /// Loads the contents of a a
script file (a .dart file). Used when loading
s/a a/a/

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/li...
pkg/compiler/lib/src/library_loader.dart:1448: /// API used by the library
loader to translate internal SDK uri's into file
Be consistent with "uri's" or "URIs"

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/li...
pkg/compiler/lib/src/library_loader.dart:1505: /// API used byt he library
loader to sychronously scan a library or compilation
byt he
sychronously

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/li...
pkg/compiler/lib/src/library_loader.dart:1516: abstract class
LibraryLoaderListener {
Maybe the loader can just publish a stream of libraries that it loads?

[Siggi Cherem (dart-lang), 2016-03-23 22:51:05]

thanks

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/co...
File pkg/compiler/lib/src/compiler.dart (right):

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/co...
pkg/compiler/lib/src/compiler.dart:291: fromEnvironment(String name) =>
environment.valueOf(name);
On 2016/03/22 21:49:28, Harry Terkelsen wrote:
> add @deprecated

Done.

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/co...
pkg/compiler/lib/src/compiler.dart:413: this, enableConditionalDirectives:
options.enableConditionalDirectives),
On 2016/03/22 21:49:28, Harry Terkelsen wrote:
> ditto

Done.

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/co...
pkg/compiler/lib/src/compiler.dart:415: this, enableConditionalDirectives:
options.enableConditionalDirectives),
On 2016/03/22 21:49:27, Harry Terkelsen wrote:
> ditto

Done.

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/li...
File pkg/compiler/lib/src/library_loader.dart (right):

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/li...
pkg/compiler/lib/src/library_loader.dart:273: /// Translates interal uris (like
dart:core) to a disk location.
On 2016/03/22 21:49:28, Harry Terkelsen wrote:
> s/interal/internal/

Done.

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/li...
pkg/compiler/lib/src/library_loader.dart:276: /// Loads the contents of a a
script file (a .dart file). Used when loading
On 2016/03/22 21:49:28, Harry Terkelsen wrote:
> s/a a/a/

Done.

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/li...
pkg/compiler/lib/src/library_loader.dart:1448: /// API used by the library
loader to translate internal SDK uri's into file
On 2016/03/22 21:49:28, Harry Terkelsen wrote:
> Be consistent with "uri's" or "URIs"

Done.

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/li...
pkg/compiler/lib/src/library_loader.dart:1505: /// API used byt he library
loader to sychronously scan a library or compilation
On 2016/03/22 21:49:28, Harry Terkelsen wrote:
> byt he
> sychronously

Done.

https://codereview.chromium.org/1819053002/diff/40001/pkg/compiler/lib/src/li...
pkg/compiler/lib/src/library_loader.dart:1516: abstract class
LibraryLoaderListener {
On 2016/03/22 21:49:28, Harry Terkelsen wrote:
> Maybe the loader can just publish a stream of libraries that it loads?

Might be worth looking into a similar idea in the future. Johnni pointed out
that a stream might not be enough because we don't know when we are done that
easily (some callbacks enqueue more libraries to load). Maybe a FutureGroup is
more appropriate?

I want to see though if we can move away from the callbacks entirely, so there
would be no listener and just a full list of loaded libaries. Maybe a list of
libraries + extra info needed to figure out what else to load (e.g. if there is
something specific to the backend)

[Siggi Cherem (dart-lang), 2016-03-23 23:02:38]

Description was changed from

==========
Split loader from the rest of the compiler. This adds several abstractions to
make the decoupling easier. Some of the new abstractions should be revisited,
but it's a step forward :).
==========

to

==========
Split loader from the rest of the compiler. This adds several abstractions to
make the decoupling easier. Some of the new abstractions should be revisited,
but it's a step forward :).

R=het@google.com

Committed:
https://github.com/dart-lang/sdk/commit/bada3cac9de026784015074b44ddf1def8a79670
==========

[Siggi Cherem (dart-lang), 2016-03-23 23:02:40]

Committed patchset #2 (id:40001) manually as
bada3cac9de026784015074b44ddf1def8a79670 (presubmit successful).