sort classes in dependency order, or load lazily if needed, fixes #78

sort classes in dependency order, or load lazily if needed, fixes #78

this also starts using compiled code in dart_core.js (just copy+paste for Object)

visitClassDeclaration was getting rather long, so it's now refactored to make the flow more clear.

R=sigmund@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/97750a8a32b720f3857f473334535e0d9c56acd4

[Jennifer Messerly, 2015-03-18 22:50:55]

Patchset #1 (id:1) has been deleted

[Jennifer Messerly, 2015-03-18 22:51:41]

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

[Siggi Cherem (dart-lang), 2015-03-18 23:54:56]

this is really nice! LGTM

I think we can keep this technique long term too, especially if we can pick a
canonical order for other cycles outside the SDK. In particular, many cycles are
between the entry library of a package and some of it's subpieces (e.g.
path.dart), where we can clearly indicate to users which library they are
supposed to import.

https://codereview.chromium.org/1016003003/diff/20001/lib/src/codegen/js_code...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1016003003/diff/20001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:424: if (library.isInSdk &&
!currentLibrary.isInSdk) return false;
maybe move the second condition to be an assertion above? (since you never call
this for sdk libs)

https://codereview.chromium.org/1016003003/diff/20001/test/codegen/expect/dar...
File test/codegen/expect/dart/core.js (right):

https://codereview.chromium.org/1016003003/diff/20001/test/codegen/expect/dar...
test/codegen/expect/dart/core.js:1429: return class Runes extends
collection.IterableBase$(int) {
mmm... overall I find the generic-class pattern slightly easier to read than
this one, I think that might be because of the 'return' here.

Could we do the same pattern for this? Or move the return to a separate line?

get Runes() {
  class Runes ... {}
  return Runes();
}

[Jennifer Messerly, 2015-03-19 16:03:29]

Thanks!

https://codereview.chromium.org/1016003003/diff/20001/lib/src/codegen/js_code...
File lib/src/codegen/js_codegen.dart (right):

https://codereview.chromium.org/1016003003/diff/20001/lib/src/codegen/js_code...
lib/src/codegen/js_codegen.dart:424: if (library.isInSdk &&
!currentLibrary.isInSdk) return false;
On 2015/03/18 23:54:55, Siggi Cherem (dart-lang) wrote:
> maybe move the second condition to be an assertion above? (since you never
call
> this for sdk libs)

I think it actually can be called via recursive path, e.g.

// assume currentLibrary is sunflower
sunflower.dart:
    // call _islibraryCycle(dom);
    import 'dom.dart';

dom.dart:
    // implicit import 'dart:core';
    // call _islibraryCycle(dart.core);

In asking if sunflower.dart and dom.dart are part of a cycle, it could hit core
libs. Added a comment.

Note also, there is a fall through case in too _classMightNotBeLoaded: if we
don't know about the order of the dart:* lib in question, we go through and
check _inLibraryCycle. That makes it future proof, so when we add something like
maybe "dart:js" that is not cyclic with "dart:core", we'll understand that
without needing to tweak the code (because _inLibraryCycle works for SDK libs).
I tweaked the code to try and make it easier to follow

https://codereview.chromium.org/1016003003/diff/20001/test/codegen/expect/dar...
File test/codegen/expect/dart/core.js (right):

https://codereview.chromium.org/1016003003/diff/20001/test/codegen/expect/dar...
test/codegen/expect/dart/core.js:1429: return class Runes extends
collection.IterableBase$(int) {
On 2015/03/18 23:54:55, Siggi Cherem (dart-lang) wrote:
> mmm... overall I find the generic-class pattern slightly easier to read than
> this one, I think that might be because of the 'return' here.
> 
> Could we do the same pattern for this? Or move the return to a separate line?
> 
> get Runes() {
>   class Runes ... {}
>   return Runes();
> }
> 

Sure thing. It's actually easier to emit that way too :)

[Jennifer Messerly, 2015-03-19 17:03:27]

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