Use interceptors to handle runtime types.

Use interceptors to handle runtime types.

[Johnni Winther, 2013-09-06 06:08:28]

lgtm

[ngeoffray, 2013-09-06 06:54:06]

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart
(right):

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:3163: //
and potentially initialize the dispatch object during finishClasses.
Please add a comment on what this is for, and what does builtin$cls means.

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:3169: //
this test (so I'm keeping the redundant "object" test).
Ditto for having "func"

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:3172: '
&& receiver.constructor === Object'
Add a comment on why this constructor check?

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/lib...
File dart/sdk/lib/_internal/lib/js_types.dart (right):

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/lib...
dart/sdk/lib/_internal/lib/js_types.dart:10: * Interceptor for JavaScript
constructor functions that create Dart
What is a JavaScript constructor function? Something that we generate? IS that
*the* constructor dart2js creates for each class?

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/lib...
dart/sdk/lib/_internal/lib/js_types.dart:29: * Interceptor for JavaScript
objects that represent function types.
Where do these JavaScript objects come from?

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/lib...
dart/sdk/lib/_internal/lib/js_types.dart:31: // TODO(ahe): Shouldn't use
interceptors for this, instead create a constant.
What's "this"? How can you make it a constant?

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/lib...
dart/sdk/lib/_internal/lib/js_types.dart:35: bool get hasReturnType =>
JS('bool', '"ret" in #', this);
Maybe add a helper method hasPropertyIn(String prop, object) => JS(bool, "# in
#", prop, object).

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/lib...
dart/sdk/lib/_internal/lib/js_types.dart:38: bool get isVoid => JS('bool',
'!!#.void', this);
Will that work on the Android browser?

[ahe, 2013-09-06 15:11:53]

Nicolas has some concerns about using interceptors for this, and we'll try out
some different options once I have refactored runtime type handling.

The class JsFunctionType is only using interceptors temporarily. I'm using this
as an aid in turning the object literals we generate in to regular Dart objects
(possibly constants).

Of greater concern is JsRuntimeType. Nicolas has a great idea for not using
interceptors for that. Since JsRuntimeType is an interceptor for the JavaScript
constructor functions (created in defineClass) that create Dart objects, they
are objects we have built and that we can monkey patch as we please. So we can
consider JsRuntimeType to be a mixin whose properties we monkey patch into all
the functions we create in defineClass. This way, these JavaScript function
objects will behave like Dart objects without using interceptors.

We don't know yet which is most efficient: interceptors or the mixin approach.
So for now I'm going with interceptors. This is, in my opinion, the fastest path
to raising our abstraction level and increase sharing between runtime type
information and mirrors. It is possible that I'm wrong, but currently, my
overall understanding of runtime types is limited, and mirrors are becoming
unwieldy. This CL is the first time where I feel I that mirrors have helped to
simplify our runtime implementation, and Karl already noticed that this should
help us print better diagnostics in failed function type assertions.

[ahe, 2013-09-09 11:26:29]

PTAL

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/com...
File dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart
(right):

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:3163: //
and potentially initialize the dispatch object during finishClasses.
On 2013/09/06 06:54:06, ngeoffray wrote:
> Please add a comment on what this is for, and what does builtin$cls means.

Done.

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:3169: //
this test (so I'm keeping the redundant "object" test).
On 2013/09/06 06:54:06, ngeoffray wrote:
> Ditto for having "func"

Done.

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/com...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:3172: '
&& receiver.constructor === Object'
On 2013/09/06 06:54:06, ngeoffray wrote:
> Add a comment on why this constructor check?

Done.

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/lib...
File dart/sdk/lib/_internal/lib/js_types.dart (right):

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/lib...
dart/sdk/lib/_internal/lib/js_types.dart:10: * Interceptor for JavaScript
constructor functions that create Dart
On 2013/09/06 06:54:06, ngeoffray wrote:
> What is a JavaScript constructor function? Something that we generate? IS that
> *the* constructor dart2js creates for each class?

Done.

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/lib...
dart/sdk/lib/_internal/lib/js_types.dart:29: * Interceptor for JavaScript
objects that represent function types.
On 2013/09/06 06:54:06, ngeoffray wrote:
> Where do these JavaScript objects come from?

Done.

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/lib...
dart/sdk/lib/_internal/lib/js_types.dart:31: // TODO(ahe): Shouldn't use
interceptors for this, instead create a constant.
On 2013/09/06 06:54:06, ngeoffray wrote:
> What's "this"? How can you make it a constant?

Done.

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/lib...
dart/sdk/lib/_internal/lib/js_types.dart:35: bool get hasReturnType =>
JS('bool', '"ret" in #', this);
On 2013/09/06 06:54:06, ngeoffray wrote:
> Maybe add a helper method hasPropertyIn(String prop, object) => JS(bool, "# in
> #", prop, object).

I hope to be able to clean this up in a more fundamental way. Additional
abstractions would feel like a step in the wrong direction to me.

https://codereview.chromium.org/23996002/diff/5001/dart/sdk/lib/_internal/lib...
dart/sdk/lib/_internal/lib/js_types.dart:38: bool get isVoid => JS('bool',
'!!#.void', this);
On 2013/09/06 06:54:06, ngeoffray wrote:
> Will that work on the Android browser?

I think it will now.

[ngeoffray, 2013-09-10 10:42:26]

LGTM

https://codereview.chromium.org/23996002/diff/12001/dart/sdk/lib/_internal/co...
File dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart
(right):

https://codereview.chromium.org/23996002/diff/12001/dart/sdk/lib/_internal/co...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:3172: //
[TypeRepresentationGenerator.visitFunctionType] and always have the
missing something after 'and'.