Make sure that the name accessor on functions return the expected...

Make sure that the name accessor on functions return the expected
names.

- Set the correct name of library functions.
- Set the name of C++ callback functions.
- Clean up a couple of out-dated comments related to literal creation.


Committed: http://code.google.com/p/v8/source/detail?r=414

[Kasper Lund, 2008-10-03 05:55:18]

LGTM with a few comments:

http://codereview.chromium.org/6223/diff/1/10
File src/array.js (right):

http://codereview.chromium.org/6223/diff/1/10#newcode932
Line 932: InstallFunctions($Array.prototype, DONT_ENUM, new $Array(
I guess an array literal doesn't work here? You could also get rid of the 'new'
keyword...

http://codereview.chromium.org/6223/diff/1/7
File src/date-delay.js (right):

http://codereview.chromium.org/6223/diff/1/7#newcode645
Line 645: };
I would rather remove the semicolons than add new ones, but maybe we should do
that in a separate pass?

http://codereview.chromium.org/6223/diff/1/7#newcode965
Line 965: InstallFunctions($Date, DONT_ENUM, new $Array(
See other comment.

http://codereview.chromium.org/6223/diff/1/7#newcode976
Line 976: InstallFunctions($Date.prototype, DONT_ENUM, new $Array(
Already mentioned in other comment.

http://codereview.chromium.org/6223/diff/1/13
File src/math.js (right):

http://codereview.chromium.org/6223/diff/1/13#newcode143
Line 143: InstallFunctions($Math, DONT_ENUM, new $Array(
Yup. Again.

http://codereview.chromium.org/6223/diff/1/9
File src/messages.js (right):

http://codereview.chromium.org/6223/diff/1/9#newcode654
Line 654: DefineError('Error', function Error(msg) { });
Do you need to pass in the name here too? Maybe you could just get that from the
f function? I don't know. Does that (msg) part buy us anything? We replace the
code of the function anyway, right?

http://codereview.chromium.org/6223/diff/1/6
File src/regexp-delay.js (right):

http://codereview.chromium.org/6223/diff/1/6#newcode292
Line 292: InstallFunctions($RegExp.prototype, DONT_ENUM, new $Array(
Already commented on elsewhere.

http://codereview.chromium.org/6223/diff/1/6#newcode342
Line 342: function NoOpSetter(ignored) {};
No need for the ; - let's clean that up soon.

http://codereview.chromium.org/6223/diff/1/6#newcode363
Line 363: // A local scope to hide the loop index i.
Is this local scope necessary anymore? It would be great to get rid of it if
not.

http://codereview.chromium.org/6223/diff/1/12
File src/string.js (right):

http://codereview.chromium.org/6223/diff/1/12#newcode1
Line 1: // Copyright 2006-2008 the V8 project authors. All rights reserved.
This file could actually use a slight rewrite to make it use the
InstallFunctions helper. I'm sure that would generate less code too.

http://codereview.chromium.org/6223/diff/1/11
File src/uri.js (right):

http://codereview.chromium.org/6223/diff/1/11#newcode360
Line 360: InstallFunctions(global, DONT_ENUM, new $Array(
Yes, yes.

http://codereview.chromium.org/6223/diff/1/4
File src/v8natives.js (right):

http://codereview.chromium.org/6223/diff/1/4#newcode78
Line 78: return $floor(string);
Maybe add $floor to the requirements section like the other // const $xxx
comments?

[Mads Ager (chromium), 2008-10-03 07:13:42]

Thanks.  Committed.

http://codereview.chromium.org/6223/diff/1/10
File src/array.js (right):

http://codereview.chromium.org/6223/diff/1/10#newcode932
Line 932: InstallFunctions($Array.prototype, DONT_ENUM, new $Array(
Right.  An array literal will not work.

http://codereview.chromium.org/6223/diff/1/7
File src/date-delay.js (right):

http://codereview.chromium.org/6223/diff/1/7#newcode645
Line 645: };
No need for a separate round of reviews for that.  I have removed the semicolons
I could find.

http://codereview.chromium.org/6223/diff/1/9
File src/messages.js (right):

http://codereview.chromium.org/6223/diff/1/9#newcode654
Line 654: DefineError('Error', function Error(msg) { });
The (msg) part is gone.  It is replaced anyway.

I'm now getting the name from the function passed in.

http://codereview.chromium.org/6223/diff/1/6
File src/regexp-delay.js (right):

http://codereview.chromium.org/6223/diff/1/6#newcode342
Line 342: function NoOpSetter(ignored) {};
Done.

http://codereview.chromium.org/6223/diff/1/6#newcode363
Line 363: // A local scope to hide the loop index i.
True, none of the local scope are needed here since we are in a function scope
already.  Removed.

http://codereview.chromium.org/6223/diff/1/12
File src/string.js (right):

http://codereview.chromium.org/6223/diff/1/12#newcode1
Line 1: // Copyright 2006-2008 the V8 project authors. All rights reserved.
I agree.  I'll do that in a separate change list.

http://codereview.chromium.org/6223/diff/1/4
File src/v8natives.js (right):

http://codereview.chromium.org/6223/diff/1/4#newcode78
Line 78: return $floor(string);
Done.