Add JSNull, JSBool and JSFunction, and move toString into the new interceptor scheme.

Add JSNull, JSBool and JSFunction, and move toString into the new interceptor scheme.
Committed: https://code.google.com/p/dart/source/detail?r=15142

[sra1, 2012-11-19 18:18:24]

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

https://codereview.chromium.org/11348133/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:957: if
(message.contains('JSNull') ||
Will this work with --minify?

[ngeoffray, 2012-11-20 08:45:45]

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

https://codereview.chromium.org/11348133/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:957: if
(message.contains('JSNull') ||
On 2012/11/19 18:18:24, sra1 wrote:
> Will this work with --minify?

Good catch Stephen! I wouldn't worry too much about it though. This code is only
there to know whether we should throw a NullPointerException or
NoSuchMethodError. Since Lasse is removing NullPointerException real soon now,
this code will just be removed.

[Johnni Winther, 2012-11-20 09:26:20]

lgtm

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

http://codereview.chromium.org/11348133/diff/4001/sdk/lib/_internal/compiler/...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1234: :
'this.$extraArg$joinedArgs';
Why the 'this.' prefix?

http://codereview.chromium.org/11348133/diff/4001/sdk/lib/_internal/compiler/...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1235:
boundClosureBuffer.add(" return this.self[this.target]($callArgs);");
If extraArgWithoutComma is '' then callArgs becomes 'this.'. Is that
intentional?

[ngeoffray, 2012-11-20 10:30:50]

Thanks Johnni

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

http://codereview.chromium.org/11348133/diff/4001/sdk/lib/_internal/compiler/...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1234: :
'this.$extraArg$joinedArgs';
On 2012/11/20 09:26:20, Johnni Winther wrote:
> Why the 'this.' prefix? 

Because you're using a field of the bound closure, which is the actual receiver
of the method. For example:

var c = a.toString;
c();

The call to 'c' does not pass 'a', so when creating the bound closure for
'a.toString', you assign 'a' to a field of the bound closure. Then, when you
call the toString method you basically do:
this.self['toString'](this.receiver);

http://codereview.chromium.org/11348133/diff/4001/sdk/lib/_internal/compiler/...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1235:
boundClosureBuffer.add(" return this.self[this.target]($callArgs);");
On 2012/11/20 09:26:20, Johnni Winther wrote:
> If extraArgWithoutComma is '' then callArgs becomes 'this.'. Is that
> intentional?

If extraArgWithoutComma is '', then extrArg is ''. Which makes callArlgs be
joinedArgs and not contain extraArgWithoutComma.

[karlklose, 2012-11-20 10:30:59]

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

https://codereview.chromium.org/11348133/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1235:
boundClosureBuffer.add(" return this.self[this.target]($callArgs);");
On 2012/11/20 09:26:20, Johnni Winther wrote:
> If extraArgWithoutComma is '' then callArgs becomes 'this.'. Is that
> intentional?

I don't think this can happen, because extraArgWithoutComma.isEmpty ->
extraArg.isEmpty by ll. 1189-1197 and that implies callArs == joimedArgs bu l.
1231.

However, this code is really a bit complex. Can you perhaps calculate two flags
in around l. 1190 and use them down here?

[ngeoffray, 2012-11-20 10:36:21]

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

https://codereview.chromium.org/11348133/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1235:
boundClosureBuffer.add(" return this.self[this.target]($callArgs);");
On 2012/11/20 10:31:00, karlklose wrote:
> On 2012/11/20 09:26:20, Johnni Winther wrote:
> > If extraArgWithoutComma is '' then callArgs becomes 'this.'. Is that
> > intentional?
> 
> I don't think this can happen, because extraArgWithoutComma.isEmpty ->
> extraArg.isEmpty by ll. 1189-1197 and that implies callArs == joimedArgs bu l.
> 1231.
> 
> However, this code is really a bit complex. Can you perhaps calculate two
flags
> in around l. 1190 and use them down here? 

I added one flag: hasExtraArgument. What other flag did upi have in mind?

[karlklose, 2012-11-20 14:06:23]

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

https://codereview.chromium.org/11348133/diff/4001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:1235:
boundClosureBuffer.add(" return this.self[this.target]($callArgs);");
I thought about isInterceptor and hasArguments, but you could as well test for
joinedArguments.isEmpty.

[karlklose, 2012-11-20 14:09:12]

LGTM with one question:

https://codereview.chromium.org/11348133/diff/5004/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/lib/js_helper.dart (right):

https://codereview.chromium.org/11348133/diff/5004/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:947: if
(message.contains('JSNull') ||
Where does this come from?

[ngeoffray, 2012-11-20 14:15:17]

https://codereview.chromium.org/11348133/diff/5004/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/lib/js_helper.dart (right):

https://codereview.chromium.org/11348133/diff/5004/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/lib/js_helper.dart:947: if
(message.contains('JSNull') ||
On 2012/11/20 14:09:12, karlklose wrote:
> Where does this come from?

Take the call foo.forEach(...). Since forEach is intercepted, we get an
interceptor object:
getInterceptor(foo).forEach(foo).

Now if 'foo' is null, we get a JSNull instance and call forEach on it. Since
JSNull does not have forEach, we get a message saying that "JSNull does not have
proprerty forEach' at runtime.