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

[Tom Rodriguez]

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.

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());

Aren't the new constant pool tags restricted to a minimum class file version?  I don't see a check for that.

verifier.cpp:

This has a check for EnableMethodHandles but the parser does not.  I should be consistent one way or another.  A check at load time would obviate later checks I think.

bytecodes.hpp:

This comment:

 // Abstraction for invoke_{virtual, static, interface, special}

should move along with Bytecode_invoke and something appropriate should be placed above Bytecode_member_ref.

Any reason not to delete adjusted_invoke_code completely?

Instead of having to ifdef uses of CPCACHE_INDEX_TAG why not simply define it as 0 in non-debug builds?  DEBUG_ONLY in expressions is fairly awful.

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?

Otherwise this seems ok.

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