Re: [RFA] _Jv_InterpFrame "this" pointer

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Kyle" == Kyle Galloway <kgallowa@redhat.com> writes:

Kyle> This patch add a method to _Jv_InterpFrame to get the "this" pointer
Kyle> for an interpreted method.  When the method is run, the first argument
Kyle> is copied into a new field in _Jv_InterpFrame.  I have added a method
Kyle> get_this_ptr that hecks if the method is static (and returns null if
Kyle> it is) and returns the "this" pointer for this frame.

Just a couple notes on this.



Kyle> Index: libjava/include/java-interp.h
Kyle> ===================================================================
Kyle> --- libjava/include/java-interp.h	(revision 121649)
Kyle> +++ libjava/include/java-interp.h	(working copy)
Kyle> @@ -372,6 +372,9 @@
Kyle>    _Jv_word *locals;
Kyle>    char *locals_type;
 
Kyle> +  // Object pointer for this frame ("this")
Kyle> +  _Jv_word obj_ptr;

This should just be 'jobject'.  We know 'this' will always be an
object.

Kyle>    _Jv_InterpFrame (void *meth, java::lang::Thread *thr, jclass proxyCls = NULL)
Kyle>    : _Jv_Frame (reinterpret_cast<_Jv_MethodBase *> (meth), thr,
Kyle>  	             frame_interpreter)

I think we should initialize obj_ptr to NULL in the constructor.
I like to avoid uninitialized values at the source when possible.

Kyle> +    if ((method->accflags & java::lang::reflect::Modifier::STATIC) == 0)
Kyle> +      obj = iframe->obj_ptr.o;
Kyle> +    else
Kyle> +      obj = NULL;

...then this can always return obj_ptr.

Kyle> +  // Get the object pointer for this method, if it is non-static,

The test doesn't actually test for non-static-ness.  I think it ought
to.

Kyle> +  if (meth->args_raw_size > 0)
Kyle> +    memcpy ((void *) &(frame_desc.obj_ptr), (void *) (&locals[0]), sizeof (_Jv_word));

This is pretty roundabout.  Once there's a check for non-static-ness
it can just read:

    frame_desc.obj_ptr = locals[0].o;

Tom