Refactor runtime into libraries, better type reps

Most of this CL just re-organizes the runtime code into multiple sub-libraries.  js_utils and loader remain as top level symbols (the former so that it has no dependencies, the latter since it defines the module system that the rest use).

The dart runtime files are roughly split around functional boundaries.  Comments at the top of each file describe the function of the library.  I'm not sure that these are 100% right yet, but they're closer.  We may want to iterate on the organization.

The main dart runtime file just re-exports various symbols from the sub-libraries.  It has hard dependencies on the other runtime files, some of which have soft (lazy) dependencies on SDK files.  The generated code now depends on dart and dartx as a imported libraries, rather than as top level symbols.

This CL also includes some new code to make type representations a bit better.  This is mostly in the new types.js file, which makes all of the non-instance types (dynamic, bottom, void, function types) subtypes of a common representation type.  Among other things, they now have the correct runtime type (core.Type).  Some of the type code has been rationalized around this a bit, but there is more left to do here.  The setSignature code in classes.js now also places the correct runtime type on nominal (instance) types.  There are new tests in the runtime_tests.js file to test for this.

The static object method code that implements the Object methods has not yet been updated to dispatch on type representations.  I will do that in a separate CL.


BUG=
R=jmesserly@google.com, vsm@google.com

Committed: https://github.com/dart-lang/dev_compiler/commit/5ac465cdaa1eb9508abf8fae24fa66a22bd2932b

[Leaf, 2015-06-12 00:31:42]

leafp@google.com changed reviewers:
+ jmesserly@google.com, vsm@google.com

[Leaf, 2015-06-12 00:33:11]

Hi Guys - Sorry for the massive re-org CL.  The CL description gives a bit of a
map as to what's going on, let me know if you want a walk through.  Take a look,
see what you think about the proposed organization.

[vsm, 2015-06-12 15:50:33]

lgtm with some comments

A couple general high-level comments / questions:
- "loader" and "js_util" are maybe too generic for top-level symbols - seems
like a high chance of conflict.  Maybe "dart_library" or "dart_loader", etc? 
E.g.,

dart_library.load(...)

would read reasonably well.

- The JS files should probably have a different naming conventions.  E.g.,
"dartx/classes", to make it clear they are different.

- The CL does a nice job spelling out dependences, but the lazy imports still
seem leave things a little brittle.  Longer term, I think an eager imported
"precore" that defined the types the new internal libs rely on might make sense.

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/classes.js
File lib/runtime/classes.js (right):

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/classes.js#...
lib/runtime/classes.js:13: loader.library('dart/classes', null, /* Imports */[
Perhaps rename 'dart/classes', etc., to 'dart/_classes', etc. to make it clear
these are internal libs.

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/classes.js#...
lib/runtime/classes.js:131: throwRuntimeError('type arguments should not be
null: ' + typeConstructor);
nit: length

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/classes.js#...
lib/runtime/classes.js:359: // cost is we need to call defineExtensionMEmbers
any time a subclass overrides
nit: line len && fix cap on MEmbers

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/classes.js#...
lib/runtime/classes.js:392: // setRuntimeType / field scheme for structural
types (e.g., functions
I think setRuntimeType is gone.  Can you either remove or amend this comment?

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/dart_runtim...
File lib/runtime/dart_runtime.js (right):

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:99: exports.typeName = _export(types.typeToString);
I think I renamed typeToString to  _typeName.  Could just make it typeName in
the first place.

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/js_utils.js
File lib/runtime/js_utils.js (right):

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/js_utils.js...
lib/runtime/js_utils.js:65: if (f === null) throwRuntimeError('circular
initialization for field ' + name);
length

[Jennifer Messerly, 2015-06-12 17:32:25]

a few comments in addition to Vijay's

BTW, the overall organization here looks great, the divisions totally make
sense!

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/classes.js
File lib/runtime/classes.js (right):

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/classes.js#...
lib/runtime/classes.js:33: const throwRuntimeError = js_utils.throwRuntimeError;
do we really need this function?
if so, maybe just `throwError` ...

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/classes.js#...
lib/runtime/classes.js:37: let _mixins = Symbol('mixins');
s/let/const?

(JS `let` : Dart `var` :: JS `const` : Dart `final`)

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/classes.js#...
lib/runtime/classes.js:113: if (length < 1) throwRuntimeError('must have at
least one generic type argument');
long line

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/classes.js#...
lib/runtime/classes.js:131: throwRuntimeError('type arguments should not be
null: ' + typeConstructor);
here too

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/classes.js#...
lib/runtime/classes.js:359: // cost is we need to call defineExtensionMEmbers
any time a subclass overrides
long line, also looks like I typo'd defineExtensionMEmbers at some point :)

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/dartx.js
File lib/runtime/dartx.js (right):

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/dartx.js#ne...
lib/runtime/dartx.js:8: loader.library('dart/dartx', null, /* Imports */[
not sure this is worth its own file.

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/js_utils.js
File lib/runtime/js_utils.js (right):

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/js_utils.js...
lib/runtime/js_utils.js:14: js_utils.defineProperty = defineProperty;
hmm, not sure this is worth it. We can get all of these from Object already
right? Why move them to js_util?

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/js_utils.js...
lib/runtime/js_utils.js:65: if (f === null) throwRuntimeError('circular
initialization for field ' + name);
long line

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/rtti.js
File lib/runtime/rtti.js (right):

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/rtti.js#new...
lib/runtime/rtti.js:20: /// Tag a closure with a type, using one of three forms:
fyi .. very minor ... I've been trying to use /** */ style doc comments in JS
code. I don't think JS tools understand /// like DartDoc does

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/types.js
File lib/runtime/types.js (right):

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/types.js#ne...
lib/runtime/types.js:26: class Dynamic extends TypeRep {
awesome!

[Leaf, 2015-06-12 19:52:14]

Ok, I changed a few things based on comments.  

dartx is a field on dart, imported via a let statement at the start of each
file.  

Naming convention is (e.g.) dart_runtime/_classes, changed filenames to (e.g.)
_classes.js to match.  

The two remaining top level names are dart_library and dart_utils.  We could
fold both of those into (e.g.) dart_runtime if desired.

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/classes.js
File lib/runtime/classes.js (right):

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/classes.js#...
lib/runtime/classes.js:13: loader.library('dart/classes', null, /* Imports */[
On 2015/06/12 15:50:32, vsm wrote:
> Perhaps rename 'dart/classes', etc., to 'dart/_classes', etc. to make it clear
> these are internal libs.

Done.

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/classes.js#...
lib/runtime/classes.js:131: throwRuntimeError('type arguments should not be
null: ' + typeConstructor);
On 2015/06/12 15:50:32, vsm wrote:
> nit: length

Done.

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/classes.js#...
lib/runtime/classes.js:359: // cost is we need to call defineExtensionMEmbers
any time a subclass overrides
On 2015/06/12 15:50:32, vsm wrote:
> nit: line len && fix cap on MEmbers

Done.

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/classes.js#...
lib/runtime/classes.js:392: // setRuntimeType / field scheme for structural
types (e.g., functions
On 2015/06/12 15:50:32, vsm wrote:
> I think setRuntimeType is gone.  Can you either remove or amend this comment?

Done.

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/dart_runtim...
File lib/runtime/dart_runtime.js (right):

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/dart_runtim...
lib/runtime/dart_runtime.js:99: exports.typeName = _export(types.typeToString);
On 2015/06/12 15:50:32, vsm wrote:
> I think I renamed typeToString to  _typeName.  Could just make it typeName in
> the first place.  

I think I re-renamed it as part of a merge.  I'm happy to make it all typeName
though.

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/js_utils.js
File lib/runtime/js_utils.js (right):

https://codereview.chromium.org/1182653002/diff/40001/lib/runtime/js_utils.js...
lib/runtime/js_utils.js:65: if (f === null) throwRuntimeError('circular
initialization for field ' + name);
On 2015/06/12 15:50:32, vsm wrote:
> length

Done.

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/classes.js
File lib/runtime/classes.js (right):

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/classes.js#...
lib/runtime/classes.js:33: const throwRuntimeError = js_utils.throwRuntimeError;
On 2015/06/12 17:32:25, John Messerly wrote:
> do we really need this function?
> if so, maybe just `throwError` ...

I wanted us to have a specific canonical way to throw errors that are problems
with the runtime/compiler (not problems with the user code). I'll doc this and
rename.

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/classes.js#...
lib/runtime/classes.js:37: let _mixins = Symbol('mixins');
On 2015/06/12 17:32:25, John Messerly wrote:
> s/let/const?
> 
> (JS `let` : Dart `var` :: JS `const` : Dart `final`)

Done.

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/classes.js#...
lib/runtime/classes.js:113: if (length < 1) throwRuntimeError('must have at
least one generic type argument');
On 2015/06/12 17:32:25, John Messerly wrote:
> long line

Done.

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/classes.js#...
lib/runtime/classes.js:131: throwRuntimeError('type arguments should not be
null: ' + typeConstructor);
On 2015/06/12 17:32:25, John Messerly wrote:
> here too

Done.

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/classes.js#...
lib/runtime/classes.js:359: // cost is we need to call defineExtensionMEmbers
any time a subclass overrides
On 2015/06/12 17:32:25, John Messerly wrote:
> long line, also looks like I typo'd defineExtensionMEmbers at some point :)

Done.

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/dartx.js
File lib/runtime/dartx.js (right):

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/dartx.js#ne...
lib/runtime/dartx.js:8: loader.library('dart/dartx', null, /* Imports */[
On 2015/06/12 17:32:25, John Messerly wrote:
> not sure this is worth its own file.

I moved this to be a property on dart (dart.dartx).  WDYT?

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/js_utils.js
File lib/runtime/js_utils.js (right):

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/js_utils.js...
lib/runtime/js_utils.js:14: js_utils.defineProperty = defineProperty;
On 2015/06/12 17:32:25, John Messerly wrote:
> hmm, not sure this is worth it. We can get all of these from Object already
> right? Why move them to js_util?

I was having a hard time remembering which of the defineXXX, getXXX, hasXXX came
from Object and which we defined on our own, so I found it easier to just export
them all from one place to reduce the load on my sorry noggin.  If you'd prefer
to import them directly though, I'll deal.

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/js_utils.js...
lib/runtime/js_utils.js:65: if (f === null) throwRuntimeError('circular
initialization for field ' + name);
On 2015/06/12 17:32:25, John Messerly wrote:
> long line

Done.

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/rtti.js
File lib/runtime/rtti.js (right):

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/rtti.js#new...
lib/runtime/rtti.js:20: /// Tag a closure with a type, using one of three forms:
On 2015/06/12 17:32:25, John Messerly wrote:
> fyi .. very minor ... I've been trying to use /** */ style doc comments in JS
> code. I don't think JS tools understand /// like DartDoc does

Done.

[Jennifer Messerly, 2015-06-12 20:12:49]

lgtm

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/js_utils.js
File lib/runtime/js_utils.js (right):

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/js_utils.js...
lib/runtime/js_utils.js:5: /* This library defines a set of general javascript
utilities for us
btw, I just noticed this library could be more tightly scoped. Most of this
stuff is JS reflection, so maybe `js_reflect` ? ...  There's a bit about assert
and throwing errors that doesn't quite fit, but that could stay here with a TODO
to pull out later if need

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/js_utils.js...
lib/runtime/js_utils.js:14: js_utils.defineProperty = defineProperty;
On 2015/06/12 19:52:14, Leaf wrote:
> On 2015/06/12 17:32:25, John Messerly wrote:
> > hmm, not sure this is worth it. We can get all of these from Object already
> > right? Why move them to js_util?
> 
> I was having a hard time remembering which of the defineXXX, getXXX, hasXXX
came
> from Object and which we defined on our own, so I found it easier to just
export
> them all from one place to reduce the load on my sorry noggin.  If you'd
prefer
> to import them directly though, I'll deal.

yeah I don't feel super strongly. Seems like it's fairly self documenting if the
top of the file says where it's imported from? These are pretty standard JS APIs
(they are on Object after all :) ) and documented on MDN:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...
... we could also leave them qualified every time (e.g.
Object.getOwnPropertyNames ... ) if that helped?

Maybe we should have a different naming convention for our helpers, though... I
was sort of trying to make them feel like natural extensions to the built in JS
mirror operations.

At some point we'll move off all of these, though, since Reflect is better ...
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...

Also, what the rule is for adding things here, vs using JS APIs directly?

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/js_utils.js...
lib/runtime/js_utils.js:26: /* This error indicates a bug in the runtime or the
compiler.
/** ?

[Leaf, 2015-06-12 20:34:25]

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/js_utils.js
File lib/runtime/js_utils.js (right):

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/js_utils.js...
lib/runtime/js_utils.js:14: js_utils.defineProperty = defineProperty;
On 2015/06/12 20:12:49, John Messerly wrote:
> On 2015/06/12 19:52:14, Leaf wrote:
> > On 2015/06/12 17:32:25, John Messerly wrote:
> > > hmm, not sure this is worth it. We can get all of these from Object
already
> > > right? Why move them to js_util?
> > 
> > I was having a hard time remembering which of the defineXXX, getXXX, hasXXX
> came
> > from Object and which we defined on our own, so I found it easier to just
> export
> > them all from one place to reduce the load on my sorry noggin.  If you'd
> prefer
> > to import them directly though, I'll deal.
> 
> yeah I don't feel super strongly. Seems like it's fairly self documenting if
the
> top of the file says where it's imported from? These are pretty standard JS
APIs
> (they are on Object after all :) ) and documented on MDN:
>
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...
> ... we could also leave them qualified every time (e.g.
> Object.getOwnPropertyNames ... ) if that helped?
> 
> Maybe we should have a different naming convention for our helpers, though...
I
> was sort of trying to make them feel like natural extensions to the built in
JS
> mirror operations.
> 
> At some point we'll move off all of these, though, since Reflect is better ...
>
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Obje...
> 
> Also, what the rule is for adding things here, vs using JS APIs directly? 

My problem was in remembering where to import them from - once they were
imported into a file I was fine.  :)  I'll just suck it up and learn though -
not like we'll be changing this that often.  Fixed.

https://codereview.chromium.org/1182653002/diff/30057/lib/runtime/js_utils.js...
lib/runtime/js_utils.js:26: /* This error indicates a bug in the runtime or the
compiler.
On 2015/06/12 20:12:49, John Messerly wrote:
> /** ?

Already fixed in last update.

[Leaf, 2015-06-12 20:34:45]

Committed patchset #7 (id:110001) manually as
5ac465cdaa1eb9508abf8fae24fa66a22bd2932b (presubmit successful).