dart2js: put all type-test related properties on the prototype and not on the constructor.

dart2js: put all type-test related properties on the prototype and not on the constructor.

R=karlklose@google.com

Committed: https://code.google.com/p/dart/source/detail?r=42392
Reverted: https://code.google.com/p/dart/source/detail?r=42408

Committed: https://code.google.com/p/dart/source/detail?r=42435

[floitsch, 2014-12-13 20:48:55]

floitsch@google.com changed reviewers:
+ karlklose@google.com

[floitsch, 2014-12-15 10:33:13]

Minor improvements.

[karlklose, 2014-12-15 13:12:22]

LGTM with a few comments.

I hope that some test(s) will start to pass once you fixed the copy/paste bug in
js_rti.

https://codereview.chromium.org/802653005/diff/20001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/old_emitter/type_test_emitter.dart (right):

https://codereview.chromium.org/802653005/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/type_test_emitter.dart:78:
generateIsTestsOn(classElement, generateIsTest,
We have three names for this operation:
 - generateIsTest
 - generateTypeCheck
 - emitTypeCheck

Perhaps you could normalize the names to one (not necessarily in this CL).

https://codereview.chromium.org/802653005/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/type_test_emitter.dart:146: // Emit
code for is-checks on generic arguments. For example `x is T`;
I don't think the comment describes what is generated here. This loop will
generate the is- and as-properties for `cls` that have been precomputed in the
class RuntimeTypeInformation.

https://codereview.chromium.org/802653005/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/js_lib/js_rti.dart (right):

https://codereview.chromium.org/802653005/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/js_lib/js_rti.dart:401: } else if (hasField(t,
'${JS_FUNCTION_TYPE_TAG()}') ||
One of them should be 'hasField(o, ...'.

https://codereview.chromium.org/802653005/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/js_lib/js_rti.dart:445: * look for closure properties
on the object). if [t] is not a function type
'closure properties' -> 'function type tag'?

[floitsch, 2014-12-15 16:04:35]

No new tests pass.

PTAL.
I cleaned up the code.

https://codereview.chromium.org/802653005/diff/20001/pkg/compiler/lib/src/js_...
File pkg/compiler/lib/src/js_emitter/old_emitter/type_test_emitter.dart (right):

https://codereview.chromium.org/802653005/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/type_test_emitter.dart:78:
generateIsTestsOn(classElement, generateIsTest,
On 2014/12/15 13:12:21, karlklose wrote:
> We have three names for this operation:
>  - generateIsTest
>  - generateTypeCheck
>  - emitTypeCheck
> 
> Perhaps you could normalize the names to one (not necessarily in this CL).

Should get better with further cleanups.

https://codereview.chromium.org/802653005/diff/20001/pkg/compiler/lib/src/js_...
pkg/compiler/lib/src/js_emitter/old_emitter/type_test_emitter.dart:146: // Emit
code for is-checks on generic arguments. For example `x is T`;
On 2014/12/15 13:12:21, karlklose wrote:
> I don't think the comment describes what is generated here. This loop will
> generate the is- and as-properties for `cls` that have been precomputed in the
> class RuntimeTypeInformation.

Moved it up next to the checkedClasses, and changed comment.

https://codereview.chromium.org/802653005/diff/20001/sdk/lib/_internal/compil...
File sdk/lib/_internal/compiler/js_lib/js_rti.dart (right):

https://codereview.chromium.org/802653005/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/js_lib/js_rti.dart:401: } else if (hasField(t,
'${JS_FUNCTION_TYPE_TAG()}') ||
On 2014/12/15 13:12:21, karlklose wrote:
> One of them should be 'hasField(o, ...'.

argh.
In fact o can never have a function-type-tag.
Cleaned up the code.

https://codereview.chromium.org/802653005/diff/20001/sdk/lib/_internal/compil...
sdk/lib/_internal/compiler/js_lib/js_rti.dart:445: * look for closure properties
on the object). if [t] is not a function type
On 2014/12/15 13:12:21, karlklose wrote:
> 'closure properties' -> 'function type tag'?

Changed.
Not true anymore.

[karlklose, 2014-12-16 13:25:59]

SLGTM.

[floitsch, 2014-12-16 13:49:00]

Committed patchset #4 (id:60001) manually as 42392 (presubmit successful).

[floitsch, 2014-12-16 20:30:22]

I had to revert.

PTAL. Patchset 5 vs 6 shows the new changes.

[floitsch, 2014-12-16 20:50:57]

Actually there are still some tests failing.
I will send out another request for review once I have fixed them.

[floitsch, 2014-12-16 21:16:06]

Fixed the tests.
One was assuming that "Object" kept its name in minified code. The other was
resurfacing the bug where we emit references to non-existing classes
(http://dartbug.com/21896)

PTAL.

[karlklose, 2014-12-17 14:46:56]

Still LGTM.

[floitsch, 2014-12-17 15:05:24]

Committed patchset #7 (id:120001) manually as 42435 (presubmit successful).