Throw an exception when wrong arguments are passed into SwapElements.

Throw an exception when wrong arguments are passed into SwapElements.

This runtime function should operate on elements and thus 'receiver'
must be JSObject and indices should be numbers.

TBR=lrm@chromium.org,ricow@chromium.org

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

[antonm, 2010-05-04 17:24:25]

Lasse, Rico, may you have a look?

I am going to commit to see that it solves a problem (it gives a notable speed
up on my box.)

I think that's semantically it's fine, but would appreciate your thoughts.

[Rico, 2010-05-06 08:47:44]

LGTM (Sorry for late reply)

[Lasse Reichstein, 2010-05-06 09:03:47]

LGTM too.

http://codereview.chromium.org/1960001/diff/1/2
File src/runtime.cc (right):

http://codereview.chromium.org/1960001/diff/1/2#newcode7781
src/runtime.cc:7781: Handle<Object> key1 = args.at<Object>(1);
Consider using CONVERT_ARG_CHECKED for the remaining two arguments too. It's
(slightly) more efficient than creating a new handle, since it creates a handle
that is backed by the argument on the stack instead of a HandleScope entry.

[antonm, 2010-05-06 16:40:15]

Rico, Lasse,

thanks a lot for review.

http://codereview.chromium.org/1960001/diff/1/2
File src/runtime.cc (right):

http://codereview.chromium.org/1960001/diff/1/2#newcode7781
src/runtime.cc:7781: Handle<Object> key1 = args.at<Object>(1);
On 2010/05/06 09:03:47, Lasse Reichstein wrote:
> Consider using CONVERT_ARG_CHECKED for the remaining two arguments too. It's
> (slightly) more efficient than creating a new handle, since it creates a
handle
> that is backed by the argument on the stack instead of a HandleScope entry.

Lasse, any chances you meant some other macro:

   65 #define CONVERT_ARG_CHECKED(Type, name, index)                       \    
                                                                                
                                                                                
           
   66   RUNTIME_ASSERT(args[index]->Is##Type());                           \
   67   Handle<Type> name = args.at<Type>(index);

?

[Lasse Reichstein, 2010-05-07 07:02:03]

http://codereview.chromium.org/1960001/diff/1/2
File src/runtime.cc (right):

http://codereview.chromium.org/1960001/diff/1/2#newcode7781
src/runtime.cc:7781: Handle<Object> key1 = args.at<Object>(1);
No, that's the one. See the definition of Arguments::at in arguments.h. It
returns a Handle with a location_ that points directly to the argument on the
stack, instead of allocating a new handle location from the HandleScope.

[antonm, 2010-05-07 10:06:00]

http://codereview.chromium.org/1960001/diff/1/2
File src/runtime.cc (right):

http://codereview.chromium.org/1960001/diff/1/2#newcode7781
src/runtime.cc:7781: Handle<Object> key1 = args.at<Object>(1);
On 2010/05/07 07:02:03, Lasse Reichstein wrote:
> No, that's the one. See the definition of Arguments::at in arguments.h. It
> returns a Handle with a location_ that points directly to the argument on the
> stack, instead of allocating a new handle location from the HandleScope.

Sorry, I use Handle<Object> key1 = arg.at<Object>(1) which looks exactly like
the thing the macro would do, so I cannot spot the difference.

[Lasse Reichstein, 2010-05-07 11:44:01]

http://codereview.chromium.org/1960001/diff/1/2
File src/runtime.cc (right):

http://codereview.chromium.org/1960001/diff/1/2#newcode7781
src/runtime.cc:7781: Handle<Object> key1 = args.at<Object>(1);
Whoops, my bad. Didn't read your rhs' properly (too used to seeing args[1]).

You could still use the macro, but only for consistency.