Create specialized versions of getInterceptor.

Create specialized versions of getInterceptor.
Committed: https://code.google.com/p/dart/source/detail?r=15398

[ahe, 2012-11-27 14:29:45]

LGTM

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/js_backend/backend.dart (right):

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:684: final
Map<String, Collection<ClassElement>> specializedGetInterceptors;
What is the key?

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:753: return
<ClassElement>[jsStringClass, jsArrayClass, jsIntClass,
I suggest you create this list in initializeInterceptorElements.  Like what I
did in CL 11421060.  This way you increase the chance that this list is in sync
with what is constructed in initializeInterceptorElements.

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:806:
specializedGetInterceptors.putIfAbsent(name, () {
I don't understand why you do this.  Why shouldn't you unconditionally store the
complete list?

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

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1688: void
emitTypeCheck(ClassElement cls, CodeBuffer buffer) {
This is not really emitting a type check, which would be what you emit in
checked mode for:

int x = foo(); // Here is a type check that foo returns an int.

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1691: String
then = 'return ${namer.isolateAccess(cls)}.prototype;';
Weird variable name.  How about interceptorObject?

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1693:
buffer.add("if (typeof receiver == 'boolean') $then");
This pattern really hurts my eyes :-)

How about:

buffer.add("if (typeof receiver == 'boolean') return ");
buffer.add(namer.isolateAccess(cls));
buffer.add('.prototype');

Notice that the last two lines can be shared at the end of this method.  So you
don't need the "then" variable.

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1714: void
emitGetInterceptors(CodeBuffer buffer) {
How about "emitGetInterceptorMethods"?

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/lib/interceptors.dart (right):

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/interceptors.dart:26: Function
getInterceptor;
I suggest using this pattern instead:

getInterceptor(object) {
  // This is a magic method: the compiler does ...
}

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/ssa/codegen.dart (right):

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/ssa/codegen.dart:1783: DartType type =
types[node].computeType(compiler);
Argh!!! :-)

[ngeoffray, 2012-11-27 15:50:54]

Thanks Peter.

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/js_backend/backend.dart (right):

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:684: final
Map<String, Collection<ClassElement>> specializedGetInterceptors;
On 2012/11/27 14:29:45, ahe wrote:
> What is the key?

It's in the comment above: the name of the specialized method.

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:753: return
<ClassElement>[jsStringClass, jsArrayClass, jsIntClass,
On 2012/11/27 14:29:45, ahe wrote:
> I suggest you create this list in initializeInterceptorElements.  Like what I
> did in CL 11421060.  This way you increase the chance that this list is in
sync
> with what is constructed in initializeInterceptorElements.

Done.

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:806:
specializedGetInterceptors.putIfAbsent(name, () {
On 2012/11/27 14:29:45, ahe wrote:
> I don't understand why you do this.  Why shouldn't you unconditionally store
the
> complete list?

Because I did not want to call getListOfInterceptedClasses multiple times. As
discussed, I added a TODO to implement the set of intercepted classes with a
bitset.

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

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1688: void
emitTypeCheck(ClassElement cls, CodeBuffer buffer) {
On 2012/11/27 14:29:45, ahe wrote:
> This is not really emitting a type check, which would be what you emit in
> checked mode for:
> 
> int x = foo(); // Here is a type check that foo returns an int.

I renamed the method emitInterceptorCheck.

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1691: String
then = 'return ${namer.isolateAccess(cls)}.prototype;';
On 2012/11/27 14:29:45, ahe wrote:
> Weird variable name.  How about interceptorObject?

Done.

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1693:
buffer.add("if (typeof receiver == 'boolean') $then");
On 2012/11/27 14:29:45, ahe wrote:
> This pattern really hurts my eyes :-)
> 
> How about:
> 
> buffer.add("if (typeof receiver == 'boolean') return ");
> buffer.add(namer.isolateAccess(cls));
> buffer.add('.prototype');
> 
> Notice that the last two lines can be shared at the end of this method.  So
you
> don't need the "then" variable.

Done.

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1714: void
emitGetInterceptors(CodeBuffer buffer) {
On 2012/11/27 14:29:45, ahe wrote:
> How about "emitGetInterceptorMethods"?

Done.

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/lib/interceptors.dart (right):

https://codereview.chromium.org/11413184/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/interceptors.dart:26: Function
getInterceptor;
On 2012/11/27 14:29:45, ahe wrote:
> I suggest using this pattern instead:
> 
> getInterceptor(object) {
>   // This is a magic method: the compiler does ...
> }

Done.