Cleaned up runtime macros a bit.

Cleaned up runtime macros a bit.

The bulk of this CL is purely mechanical: Make the CONVERT_FOO macros more
uniform by always using an index instead of an object. Apart from this, it
includes a few minor changes like using CONVERT_SMI_ARG_CHECKED a bit more or
introducing a new macro for PropertyDetails. Nothing spectacular, just something
sitting on my disk for quite some time now...

Committed: https://code.google.com/p/v8/source/detail?r=10772

[Jakob Kummerow, 2012-02-20 17:31:47]

LGTM with comments. 

Happy to take another look if you decide to make substantial  changes.

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

http://codereview.chromium.org/9395075/diff/1/src/runtime.cc#newcode72
src/runtime.cc:72: #define CONVERT_CHECKED(Type, name, index)                   
       \
Now that there's so little difference between CONVERT_CHECKED and
CONVERT_ARG_CHECKED (and you wouldn't guess *what* the difference is from the
macros' names!), have you thought about (1) merging or (2) renaming them?

(1) would effectively mean to handlify most of this file.

A suggestion for (2) would be:
CONVERT_CHECKED -> CONVERT_ARG_CHECKED
CONVERT_ARG_CHECKED -> CONVERT_ARG_HANDLE_CHECKED

http://codereview.chromium.org/9395075/diff/1/src/runtime.cc#newcode130
src/runtime.cc:130: ASSERT(args[index]->IsSmi());                               
      \
RUNTIME_ASSERT here, too?

http://codereview.chromium.org/9395075/diff/1/src/runtime.cc#newcode2061
src/runtime.cc:2061: return fun;
Is this change intentional?

[Sven Panne, 2012-02-21 07:21:01]

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

http://codereview.chromium.org/9395075/diff/1/src/runtime.cc#newcode72
src/runtime.cc:72: #define CONVERT_CHECKED(Type, name, index)                   
       \
On 2012/02/20 17:31:47, Jakob wrote:
> Now that there's so little difference between CONVERT_CHECKED and
> CONVERT_ARG_CHECKED (and you wouldn't guess *what* the difference is from the
> macros' names!), have you thought about (1) merging or (2) renaming them?
> 
> (1) would effectively mean to handlify most of this file.
> 
> A suggestion for (2) would be:
> CONVERT_CHECKED -> CONVERT_ARG_CHECKED
> CONVERT_ARG_CHECKED -> CONVERT_ARG_HANDLE_CHECKED

I am not 100% sure if we can really handlify everything here (probably yes, but
we need e.g. Object::IsObject, too, which shouldn't be too hard to implement
;-), so I left both macros. Merging them can be done in a separate CL, so I'll
follow your renaming proposal for now.

http://codereview.chromium.org/9395075/diff/1/src/runtime.cc#newcode130
src/runtime.cc:130: ASSERT(args[index]->IsSmi());                               
      \
On 2012/02/20 17:31:47, Jakob wrote:
> RUNTIME_ASSERT here, too?

This is what initially thought, too, but the functions using it have a return
type of ObjectPair, so it won't work. ObjectPair itself looks quite scary, and
I'm sure there's a nicer way without it, but I wasn't brave enough to touch that
voodoo magic...

http://codereview.chromium.org/9395075/diff/1/src/runtime.cc#newcode2061
src/runtime.cc:2061: return fun;
On 2012/02/20 17:31:47, Jakob wrote:
> Is this change intentional?

Yep, it is just our usual ultra-confusing way of signaling "OK". Thinking about
it, I'll replace it with undefined to make it a bit clearer.

[Sven Panne, 2012-02-21 07:32:22]

FYI: While I renamed the 2 macros, I renamed 2 more for more consistency...