Fasta targets and loaders.

Fasta targets and loaders.

R=johnniwinther@google.com

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

[ahe, 2017-01-13 12:55:48]

ahe@google.com changed reviewers:
+ johnniwinther@google.com, mravn@google.com

[ahe, 2017-01-13 12:55:48]

Mikkel: these names are particularly bad, ideas are welcome :-)

[Johnni Winther, 2017-01-19 09:21:34]

lgtm

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/dill/dill...
File pkg/fasta/lib/src/dill/dill_loader.dart (right):

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/dill/dill...
pkg/fasta/lib/src/dill/dill_loader.dart:55: input =
Uri.parse("org.dartlang:platform");
Add a comment about the semantics of this value.

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/kernel/ke...
File pkg/fasta/lib/src/kernel/kernel_target.dart (right):

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/kernel/ke...
pkg/fasta/lib/src/kernel/kernel_target.dart:281: Program link(List<Library>
libraries) {
Add dartdoc to this method.

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/kernel/ke...
pkg/fasta/lib/src/kernel/kernel_target.dart:521: for (Field field in
uninitializedFields) {
Add a comment to this and the next loop. The difference in effect is subtle.

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/kernel/ke...
pkg/fasta/lib/src/kernel/kernel_target.dart:546: void
transformMixinApplicationsX() {
Is this used?

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/kernel/ke...
pkg/fasta/lib/src/kernel/kernel_target.dart:590: Constructor
defaultSuperConstructor(Class cls) {
Add dartdoc that this return a constructor that can be called as a default
constructor (no name, no arguments) - not an implicit default constructor (which
take no arguments).

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/loader.dart
File pkg/fasta/lib/src/loader.dart (right):

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/loader.da...
pkg/fasta/lib/src/loader.dart:100: ticker.log((Duration elapsed, Duration
sinceStart) {
Share this code with [buildBodies].

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/source/so...
File pkg/fasta/lib/src/source/source_loader.dart (right):

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/source/so...
pkg/fasta/lib/src/source/source_loader.dart:246: Set<ClassBuilder>
allSupertypes(ClassBuilder cls) {
Add dartdoc (it is strict supertypes, right?)

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/source/so...
pkg/fasta/lib/src/source/source_loader.dart:256: Set<ClassBuilder>
directSupertypes(Iterable<ClassBuilder> classes) {
Add dartdoc

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/source/so...
pkg/fasta/lib/src/source/source_loader.dart:264: Iterable<ClassBuilder>
cyclicCandidates(Iterable<ClassBuilder> classes) {
Document how/why this works.

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/source/so...
pkg/fasta/lib/src/source/source_loader.dart:276: Iterable<ClassBuilder>
candidates = cyclicCandidates(allClasses);
Return early if [candidate] is empty?

[ahe, 2017-01-19 10:48:31]

Thank you, Johnni!

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/dill/dill...
File pkg/fasta/lib/src/dill/dill_loader.dart (right):

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/dill/dill...
pkg/fasta/lib/src/dill/dill_loader.dart:55: input =
Uri.parse("org.dartlang:platform");
On 2017/01/19 09:21:33, Johnni Winther wrote:
> Add a comment about the semantics of this value.

I've moved this code to pkg/fasta/lib/testing/suite.dart.

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/kernel/ke...
File pkg/fasta/lib/src/kernel/kernel_target.dart (right):

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/kernel/ke...
pkg/fasta/lib/src/kernel/kernel_target.dart:281: Program link(List<Library>
libraries) {
On 2017/01/19 09:21:33, Johnni Winther wrote:
> Add dartdoc to this method.

Done.

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/kernel/ke...
pkg/fasta/lib/src/kernel/kernel_target.dart:521: for (Field field in
uninitializedFields) {
On 2017/01/19 09:21:34, Johnni Winther wrote:
> Add a comment to this and the next loop. The difference in effect is subtle.

Done.

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/kernel/ke...
pkg/fasta/lib/src/kernel/kernel_target.dart:546: void
transformMixinApplicationsX() {
On 2017/01/19 09:21:34, Johnni Winther wrote:
> Is this used?

No. Removed.

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/kernel/ke...
pkg/fasta/lib/src/kernel/kernel_target.dart:590: Constructor
defaultSuperConstructor(Class cls) {
On 2017/01/19 09:21:34, Johnni Winther wrote:
> Add dartdoc that this return a constructor that can be called as a default
> constructor (no name, no arguments) - not an implicit default constructor
(which
> take no arguments).

Done.

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/loader.dart
File pkg/fasta/lib/src/loader.dart (right):

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/loader.da...
pkg/fasta/lib/src/loader.dart:100: ticker.log((Duration elapsed, Duration
sinceStart) {
On 2017/01/19 09:21:34, Johnni Winther wrote:
> Share this code with [buildBodies].

I'll add a TODO. These messages are confusing as we can't tell if the message is
from reading sources or from reading dill files.

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/source/so...
File pkg/fasta/lib/src/source/source_loader.dart (right):

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/source/so...
pkg/fasta/lib/src/source/source_loader.dart:246: Set<ClassBuilder>
allSupertypes(ClassBuilder cls) {
On 2017/01/19 09:21:34, Johnni Winther wrote:
> Add dartdoc (it is strict supertypes, right?)

Depends on what you mean by "strict". Since [cls] is added to the result, I
don't think this is "strict".

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/source/so...
pkg/fasta/lib/src/source/source_loader.dart:256: Set<ClassBuilder>
directSupertypes(Iterable<ClassBuilder> classes) {
On 2017/01/19 09:21:34, Johnni Winther wrote:
> Add dartdoc

Done.

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/source/so...
pkg/fasta/lib/src/source/source_loader.dart:264: Iterable<ClassBuilder>
cyclicCandidates(Iterable<ClassBuilder> classes) {
On 2017/01/19 09:21:34, Johnni Winther wrote:
> Document how/why this works.

Done.

https://codereview.chromium.org/2623033007/diff/1/pkg/fasta/lib/src/source/so...
pkg/fasta/lib/src/source/source_loader.dart:276: Iterable<ClassBuilder>
candidates = cyclicCandidates(allClasses);
On 2017/01/19 09:21:34, Johnni Winther wrote:
> Return early if [candidate] is empty?

Then it wouldn't print the log message.

[ahe, 2017-01-19 10:49:05]

Description was changed from

==========
Fasta targets and loaders.
==========

to

==========
Fasta targets and loaders.

R=johnniwinther@google.com

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

[ahe, 2017-01-19 10:49:06]

Committed patchset #2 (id:20001) manually as
19db708fed956adc28a696fedd9c75b2b47a69df (presubmit successful).