Request for reviews (L): 6939203: JSR 292 needs method handle constants

[Tom Rodriguez]

On Jun 4, 2010, at 6:11 PM, John Rose wrote:

> On Jun 3, 2010, at 11:57 AM, Tom Rodriguez wrote:
> 
>> Can you finish out the SA support or at least file a bug for it to be done?  The obvious list is printing support in ConstantPool.java and HTMLGenerator.java plus the writing support in ClassWriter.java.  All these paths will throw exceptions if they see unexpected constant pool types.
> 
> Yes, I'll do this over the weekend, plus add the missing CI support.
> 
>> classFileParser.cpp:
>> 
>> why are these lines in the code?
>> 
>> +    //cp->method_handle_at_put(index, patch());
>> 
>> +      //cp->method_type_at_put(index, patch());
>> 
>> +      //cp->unresolved_method_type_at_put(index, name());
> 
> They match the NYI bits the follow.  Or, I could just delete that code and feed all the unimplemented cases through the default.  Would that be better?

I think leave it completely unimplemented until it's fully implemented.

> 
>> Aren't the new constant pool tags restricted to a minimum class file version?  I don't see a check for that.
> 
> Last year I was permissive about classfile version for practical reasons which no longer exist.  I'll add the version checks, and also to the verifier logic.

Ok.

> 
> constantPoolOop.cpp:
>> 
>> I don't think JVM_CONSTANT_UnresolvedClassInError should be handled in resolve_constant_at_impl.  I thought any attempt to access an entry with that tag should rethrow the same exception kind that caused the original failure?
> 
> Under that case, the call to klass_at_impl will just re-throw the same exception, which is the correct meaning of a resolve.  Elsewhere, this is how it is handled, since constantTag.is_unresolved_klass reports true in such cases, causing any re-resolution attempts to throw the exception.

Right.  I was having trouble figuring out where the rethrow logic is and that makes sense.

tom

> 
>> Otherwise this seems ok.
> 
> Thanks for taking a good look at it.  I'll send out another update when I finish the pending changes.
> 
> -- John
> 
>> tom
>> 
>> On Jun 2, 2010, at 1:29 PM, John Rose wrote:
>> 
>>> On Apr 22, 2010, at 12:22 AM, Christian Thalinger wrote:
>>> 
>>>> On Thu, 2010-04-22 at 00:08 -0700, John Rose wrote:
>>>>> Here's the final webrev (if you agree that I fixed the issues you raised):
>>>>> http://cr.openjdk.java.net/~jrose/6939203/hs-webrev.01
>>>> 
>>>> Yes, that looks good.  -- Christian
>>> 
>>> The bug tail from the cpindex changes is tamed, so now it's time to push the "ldc MH" functionality.
>>> 
>>> The round of bug fixes on cpindex led me to look more closely at BytecodeTracer and MethodComparator.  This led to some more changes to make to the ldc MH code.
>>> 
>>> Here they are, for re-review:
>>> http://cr.openjdk.java.net/~jrose/6939203/hs-webrev.03
>>> 
>>> Here are the diffs relative to the last reviewed change set (02):
>>> http://cr.openjdk.java.net/~jrose/6939203/hs-webrev.03/diff-02-to-03.patch
>>> 
>>> Summary:
>>> - invoke, field ref, and load constant bytecode accessors are regularized; constructors are all the same signature.
>>> - CP cache indexes are more clearly distinguished from plain pool indexes.
>>> - BytecodeTracer and MethodComparator now work properly on fast_aldc instructions.
>>> - BytecodeTracer prints object constants a little more usefully.
>>> 
>>> Thanks in advance.
>>> 
>>> -- John
>> 
>