Request for reviews (L): 6939207: refactor constant pool index processing

[Christian Thalinger]

On Sat, 2010-05-01 at 23:28 -0700, John Rose wrote:
> On Apr 28, 2010, at 2:23 AM, Christian Thalinger wrote:
> 
> > On Fri, 2010-04-23 at 15:52 -0700, John Rose wrote:
> > > On Apr 23, 2010, at 7:06 AM, Christian Thalinger wrote:
> > > 
> > > > I have to say I was a little confused about the get_index1,
> > > > get_index2, ... naming as it looks like you were running out of
> > > > method
> > > > names and just append 1, 2, ... ;-)
> > > 
> > > You are right, it could look that way at first.  Can you suggest
> > > some
> > > comments that would short-circuit that impression quickly?
> > 
> > Well, you could rename it to get_index_u{1,2,4} which would
> > correspond
> > to the other Bytes::get* functions.
> 
> 
> Thanks, I like this idea.  I changed it to be this way.
> 
> 
> One wrinkle: get_offset2 changes to get_offset_s2 (since it's signed).
> I think it is good to advertise the signed-ness of the accessors.

I agree.

> It's a pre-existing hack.  I added comments.  I thought about
> splitting up the comparisons, but didn't want to risk a bug.
> 
> 
>     if (! _s_old->is_wide()) {
>       // We could use get_index_u1 and get_constant_u1, but it's simpler to grab both bytes at once:
>       if (Bytes::get_Java_u2(_s_old->bcp() + 1) != Bytes::get_Java_u2(_s_new->bcp() + 1))
>         return false;
>     } else {
>       // We could use get_index_u2 and get_constant_u2, but it's simpler to grab all four bytes at once:
>       if (Bytes::get_Java_u4(_s_old->bcp() + 1) != Bytes::get_Java_u4(_s_new->bcp() + 1))
>         return false;
>     }

The comments are good.

-- Christian