change to source-order crawling of directives instead of alphabetical

change to source-order crawling of directives instead of alphabetical

R=sigmund@google.com

Committed: https://github.com/dart-lang/initialize/commit/ebcbc606e9504574ea368b44427009ebb25e27c5

[jakemac, 2015-03-17 17:24:48]

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

[jakemac, 2015-03-17 17:24:49]

Fixes https://github.com/dart-lang/polymer-dart/issues/33

[Siggi Cherem (dart-lang), 2015-03-17 19:50:29]

https://codereview.chromium.org/1018643002/diff/60001/lib/src/mirror_loader.dart
File lib/src/mirror_loader.dart (right):

https://codereview.chromium.org/1018643002/diff/60001/lib/src/mirror_loader.d...
lib/src/mirror_loader.dart:133: return 0;
this might be problematic depending on the size of the list. It might be safer
to iterate the list twice, once focusing on classes, the second on methods

https://codereview.chromium.org/1018643002/diff/60001/lib/transformer.dart
File lib/transformer.dart (right):

https://codereview.chromium.org/1018643002/diff/60001/lib/transformer.dart#ne...
lib/transformer.dart:357: ..sort((a, b) => a.uriOffset.compareTo(b.uriOffset));
I wonder if analyzer by default returns them in order (now that we don't compare
by name below)

https://codereview.chromium.org/1018643002/diff/60001/lib/transformer.dart#ne...
lib/transformer.dart:364: ..sort((a, b) => a.uriOffset.compareTo(b.uriOffset));
is the uriOffset here the uri where it is imported from? Or a property of the
unit itself? (in which case they are not really comparable here)

[jakemac, 2015-03-17 21:54:04]

https://codereview.chromium.org/1018643002/diff/60001/lib/src/mirror_loader.dart
File lib/src/mirror_loader.dart (right):

https://codereview.chromium.org/1018643002/diff/60001/lib/src/mirror_loader.d...
lib/src/mirror_loader.dart:133: return 0;
On 2015/03/17 19:50:29, Siggi Cherem (dart-lang) wrote:
> this might be problematic depending on the size of the list. It might be safer
> to iterate the list twice, once focusing on classes, the second on methods

This now filters out all declarations that don't have any annotations, which
speeds things up a lot.

https://codereview.chromium.org/1018643002/diff/60001/lib/transformer.dart
File lib/transformer.dart (right):

https://codereview.chromium.org/1018643002/diff/60001/lib/transformer.dart#ne...
lib/transformer.dart:357: ..sort((a, b) => a.uriOffset.compareTo(b.uriOffset));
On 2015/03/17 19:50:29, Siggi Cherem (dart-lang) wrote:
> I wonder if analyzer by default returns them in order (now that we don't
compare
> by name below)

I would rather not rely on it

https://codereview.chromium.org/1018643002/diff/60001/lib/transformer.dart#ne...
lib/transformer.dart:364: ..sort((a, b) => a.uriOffset.compareTo(b.uriOffset));
On 2015/03/17 19:50:29, Siggi Cherem (dart-lang) wrote:
> is the uriOffset here the uri where it is imported from? Or a property of the
> unit itself? (in which case they are not really comparable here)

This now does alphabetical order, to be consistent with the mirror mode.

[Siggi Cherem (dart-lang), 2015-03-17 22:17:18]

lgtm

https://codereview.chromium.org/1018643002/diff/80001/lib/transformer.dart
File lib/transformer.dart (right):

https://codereview.chromium.org/1018643002/diff/80001/lib/transformer.dart#ne...
lib/transformer.dart:393: // The first item is the source library, remove it for
now.
actually, you can use library.definingCompilationUnit to avoid relying on that
internal detail of analyzer.

So this would look like the code you had in the mirror implementation:

var definingUnit = library.definingCompilationUnit;
var orderedUnits = new List.from(library.units)
  ..sort((a, b) {
     if (a == definingUnit) return 1;
     if (b == definingUnit) return -1;
     return compare...;
  });

[jakemac, 2015-03-17 22:50:48]

https://codereview.chromium.org/1018643002/diff/80001/lib/transformer.dart
File lib/transformer.dart (right):

https://codereview.chromium.org/1018643002/diff/80001/lib/transformer.dart#ne...
lib/transformer.dart:393: // The first item is the source library, remove it for
now.
On 2015/03/17 22:17:18, Siggi Cherem (dart-lang) wrote:
> actually, you can use library.definingCompilationUnit to avoid relying on that
> internal detail of analyzer.
> 
> So this would look like the code you had in the mirror implementation:
> 
> var definingUnit = library.definingCompilationUnit;
> var orderedUnits = new List.from(library.units)
>   ..sort((a, b) {
>      if (a == definingUnit) return 1;
>      if (b == definingUnit) return -1;
>      return compare...;
>   });

Done.

[jakemac, 2015-03-17 22:51:44]

Committed patchset #6 (id:100001) manually as
ebcbc606e9504574ea368b44427009ebb25e27c5 (presubmit successful).