Remove holders for runtime type information.

Remove holders for runtime type information.

Use the constructors of interceptors for native classes and mark classes that are used as type arguments only as needed.

To output the correct class name for native types, encode the type name in the class description.

R=sra@google.com

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

[ngeoffray, 2013-04-29 14:14:22]

DBC

https://codereview.chromium.org/14018036/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/14018036/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:2563:
rti.allArguments.forEach((ClassElement c) {
Please add a comment on why you're iterating over this collection.

[sra1, 2013-04-29 19:04:12]

lgtm

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

https://codereview.chromium.org/14018036/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:610: *
descriptor.
Commeny should say what name 'name' is.
Comment should say it is optional and when it is used.

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

https://codereview.chromium.org/14018036/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/namer.dart:645: String
getNativeName(ClassElement cls) {
What is 'native' about this - we tend to use 'native' for something related to
native (DOM) classes.

getRuntimeTypeNameForPrimitiveInterceptor ?

https://codereview.chromium.org/14018036/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/namer.dart:659: }
If you intend to return null please be explicit, otherwise it looks like an
oversight.

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

https://codereview.chromium.org/14018036/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/runtime_types.dart:181:
TypeChecks computeRequiredChecks() {
There is no return - should this be 'void'?

[karlklose, 2013-04-30 09:29:40]

Thanks for the review, Nicolas and Stephen.

https://codereview.chromium.org/14018036/diff/1/sdk/lib/_internal/compiler/im...
File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/14018036/diff/1/sdk/lib/_internal/compiler/im...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:2563:
rti.allArguments.forEach((ClassElement c) {
On 2013/04/29 14:14:22, ngeoffray wrote:
> Please add a comment on why you're iterating over this collection.

Done.

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

https://codereview.chromium.org/14018036/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:610: *
descriptor.
On 2013/04/29 19:04:12, sra1 wrote:
> Commeny should say what name 'name' is.
> Comment should say it is optional and when it is used.

Done.

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

https://codereview.chromium.org/14018036/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/namer.dart:645: String
getNativeName(ClassElement cls) {
Changed to getPrimitiveInterceptorRuntimeName.

I meant JS native meaning it is not a normal representation of a Dart class in
JS (and it has to be intercepted). 
On 2013/04/29 19:04:12, sra1 wrote:
> What is 'native' about this - we tend to use 'native' for something related to
> native (DOM) classes.
> 
> getRuntimeTypeNameForPrimitiveInterceptor ?

https://codereview.chromium.org/14018036/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/namer.dart:659: }
On 2013/04/29 19:04:12, sra1 wrote:
> If you intend to return null please be explicit, otherwise it looks like an
> oversight.

Done.

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

https://codereview.chromium.org/14018036/diff/5001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/runtime_types.dart:181:
TypeChecks computeRequiredChecks() {
Yes, changed to void.

[karlklose, 2013-04-30 10:22:42]

Committed patchset #4 manually as r22172 (presubmit successful).

[ahe, 2013-07-19 16:30:25]

https://codereview.chromium.org/14018036/diff/18001/sdk/lib/_internal/compile...
File sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart (right):

https://codereview.chromium.org/14018036/diff/18001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1470:
getterCode += backend.fieldHasInterceptedGetter(member) ? 2 : 0;
Read my comment on line 2602 first.

Consider CssStyleDeclaration.cssText:

CssStyleDeclaration is not instantiated, so we have not recorded that cssText
should be intercepted.

So this is equivalent to:

getterCode += 0;

https://codereview.chromium.org/14018036/diff/18001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1471:
getterCode += backend.isInterceptorClass(classElement) ? 0 : 1;
For CssStyleDeclaration.cssText, classElement is an interceptor class, so we add
0.

getterCode remains 0.

https://codereview.chromium.org/14018036/diff/18001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1475:
assert(getterCode != 0);
This assertion fails.

https://codereview.chromium.org/14018036/diff/18001/sdk/lib/_internal/compile...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:2602:
neededClasses.add(c);
I'm not sure how this is supposed to work?

These classes might have been tree-shaken away and the rest of the compiler has
not analyzed them.  For example,  consider this class:

@DomName('CSSStyleDeclaration')
class CssStyleDeclaration extends Interceptor native
"CSSStyleDeclaration,MSStyleCSSProperties,CSS2Properties" {
  factory CssStyleDeclaration() => new CssStyleDeclaration.css('');

  factory CssStyleDeclaration.css(String css) {
    final style = new Element.tag('div').style;
    style.cssText = css;
    return style;
  }


  @DomName('CSSStyleDeclaration.cssText')
  @DocsEditable()
  String cssText;

...
}

This class is not necessarily instantiated, but instantiated classes might still
use it in a type annotation.

But if it is not instantiated, we have not recorded that the element
corresponding to cssText is intercepted.  However, this is a native class, so
scroll up to line 1475, and see my comments.