Refactoring of v8:Arguments

Refactoring of v8:Arguments similary we did with v8::AccessorInfo (http://codereview.chromium.org/242050). GC-controlled values moved to a separate array.

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

[SeRya, 2010-10-29 16:45:29]

It's preparation for direct calls.

[antonm, 2010-10-29 19:00:27]

LGTM.

My main complain is names.

http://codereview.chromium.org/4117010/diff/1/2
File include/v8.h (right):

http://codereview.chromium.org/4117010/diff/1/2#newcode1798
include/v8.h:1798: inline Arguments(internal::Object** extra,
any chances for better names?  for example args might be a better name for
values, but alas I don't have any good idea for extra.

http://codereview.chromium.org/4117010/diff/1/3
File src/apiutils.h (right):

http://codereview.chromium.org/4117010/diff/1/3#newcode48
src/apiutils.h:48: // a pointer to the last element of 3-ements array controlled
by GC.
nit: 3-e<le>ments

http://codereview.chromium.org/4117010/diff/1/3#newcode49
src/apiutils.h:49: static void PrepareArgumentsData(internal::Object** data,
maybe rename data to args or extra and call_data to data?

[SeRya, 2010-11-01 08:54:58]

http://codereview.chromium.org/4117010/diff/1/2
File include/v8.h (right):

http://codereview.chromium.org/4117010/diff/1/2#newcode1798
include/v8.h:1798: inline Arguments(internal::Object** extra,
On 2010/10/29 19:00:27, antonm wrote:
> any chances for better names?  for example args might be a better name for
> values, but alas I don't have any good idea for extra.

'args' would sould like tautology for me (the class itself is named Arguments).
'extra' I renamed to 'implicit_args'. Does it sound good for you?

http://codereview.chromium.org/4117010/diff/1/3
File src/apiutils.h (right):

http://codereview.chromium.org/4117010/diff/1/3#newcode48
src/apiutils.h:48: // a pointer to the last element of 3-ements array controlled
by GC.
On 2010/10/29 19:00:27, antonm wrote:
> nit: 3-e<le>ments

Done.

http://codereview.chromium.org/4117010/diff/1/3#newcode49
src/apiutils.h:49: static void PrepareArgumentsData(internal::Object** data,
On 2010/10/29 19:00:27, antonm wrote:
> maybe rename data to args or extra and call_data to data?

Done.

[antonm, 2010-11-01 09:32:34]

Still LGTM

http://codereview.chromium.org/4117010/diff/1/2
File include/v8.h (right):

http://codereview.chromium.org/4117010/diff/1/2#newcode1798
include/v8.h:1798: inline Arguments(internal::Object** extra,
On 2010/11/01 08:54:58, SeRya wrote:
> On 2010/10/29 19:00:27, antonm wrote:
> > any chances for better names?  for example args might be a better name for
> > values, but alas I don't have any good idea for extra.
> 
> 'args' would sould like tautology for me (the class itself is named
Arguments).
> 'extra' I renamed to 'implicit_args'. Does it sound good for you?

Yes, implicit_args sounds really nice, thank you.

http://codereview.chromium.org/4117010/diff/14001/15002
File src/apiutils.h (right):

http://codereview.chromium.org/4117010/diff/14001/15002#newcode47
src/apiutils.h:47: // Packs additional parameters for the NewArguments function.
'data' is
should 'data' here be changed to |implicit_args|?  Note, that names of
arguments/vars are usually placed into || in docs.

http://codereview.chromium.org/4117010/diff/14001/15003
File src/arguments.h (right):

http://codereview.chromium.org/4117010/diff/14001/15003#newcode88
src/arguments.h:88: inline CustomArguments() {
maybe kZapValue value_ in debug mode?

[SeRya, 2010-11-01 10:32:49]

http://codereview.chromium.org/4117010/diff/14001/15002
File src/apiutils.h (right):

http://codereview.chromium.org/4117010/diff/14001/15002#newcode47
src/apiutils.h:47: // Packs additional parameters for the NewArguments function.
'data' is
On 2010/11/01 09:32:34, antonm wrote:
> should 'data' here be changed to |implicit_args|?  Note, that names of
> arguments/vars are usually placed into || in docs.

Done.

http://codereview.chromium.org/4117010/diff/14001/15003
File src/arguments.h (right):

http://codereview.chromium.org/4117010/diff/14001/15003#newcode88
src/arguments.h:88: inline CustomArguments() {
On 2010/11/01 09:32:34, antonm wrote:
> maybe kZapValue value_ in debug mode?

Done.