RFR(L): 8008688: Make MethodHandleInfo public

[Vladimir Ivanov]

John,

I have some minor suggestions about code style and code readability.
Otherwise, the change looks good!

src/share/classes/java/lang/invoke/MemberName.java:
      public MemberName(Method m, boolean wantSpecial) {
...
          MethodHandleNatives.init(this, m);
+        if (clazz == null) {  // MHN.init failed
+            if (m.getDeclaringClass() == MethodHandle.class &&
+                isMethodHandleInvokeName(m.getName())) {

Please, add a comment with a short description why a custom init 
procedure for MH.invoke* cases is needed.


+    /** Create a name for a signature-polymorphic invoker. */
+    static MemberName makeMethodHandleInvoke(String name, MethodType 
type) {
+        return makeMethodHandleInvoke(name, type, MH_INVOKE_MODS | 
SYNTHETIC);

Please, add a comment why SYNTHETIC flag is necessary.


src/share/classes/java/lang/invoke/MethodHandleInfo.java:
src/share/classes/java/lang/invoke/MethodHandles.java:

+import java.security.*;

This import isn't used.


src/share/classes/java/lang/invoke/MethodHandles.java:

+        public MethodHandleInfo revealDirect(MethodHandle target) {
...
+            byte refKind = member.getReferenceKind();
...
+            // Check SM permissions and member access before cracking.
+            try {
+                //@@checkSecurityManager(defc, member, lookupClass());
+                checkSecurityManager(defc, member);
+                checkAccess(member.getReferenceKind(), defc, member);
+            } catch (IllegalAccessException ex) {
+                throw new IllegalArgumentException(ex);
+            }

You prepare a separate refKind for MethodHandleInfo, but perform 
security checks against original member.getReferenceKind(). Is it 
intentional?


src/share/classes/java/lang/invoke/InfoFromMemberName.java:

81     public <T extends Member> T reflectAs(Class<T> expected, Lookup 
lookup) {
82         if (member.isMethodHandleInvoke() && !member.isVarargs()) {
83             // this member is an instance of a signature-polymorphic 
method, which cannot be reflected
84             throw new IllegalArgumentException("cannot reflect 
signature polymorphic method");

Please, add a comment why (!member.isVarargs()) check is necessary.


src/share/classes/java/lang/invoke/InfoFromMemberName.java:

  127     private void checkAccess(Member mem, Lookup lookup) throws 
IllegalAccessException {
  128         byte refKind = (byte) getReferenceKind();
  129         if (mem instanceof Method) {
  130             boolean wantSpecial = (refKind == REF_invokeSpecial);
  131             lookup.checkAccess(refKind, getDeclaringClass(), new 
MemberName((Method) mem, wantSpecial));
  132         } else if (mem instanceof Constructor) {
  133             lookup.checkAccess(refKind, getDeclaringClass(), new 
MemberName((Constructor) mem));
  134         } else if (mem instanceof Field) {
  135             boolean isSetter = (refKind == REF_putField || refKind 
== REF_putStatic);
  136             lookup.checkAccess(refKind, getDeclaringClass(), new 
MemberName((Field) mem, isSetter));
  137         }
  138     }

Can you introduce a factory method to convert a Member instance into 
MemberName and call lookup.checkAccess(refKind, getDeclaringClass(), 
covertToMemberName(mem)) instead? It'll considerably simplify the code 
and make the intention clearer.

Best regards,
Vladimir Ivanov

On 6/27/13 10:00 AM, John Rose wrote:
> This change implements the MethodHandleInfo API for cracking a direct method handle back into its symbolic reference components.  A DMH is any CONSTANT_MethodHandle or any result of a Lookup.find* or Lookup.unreflect* API call.
>
> http://cr.openjdk.java.net/~jrose/8008688/webrev.04
>
> Problem:
>
> JDK 8 (esp. Project Lambda) needs a stable API for "cracking" CONSTANT_MethodHandle constants that are involved with lambda capture sites (which are implemented with invokedynamic).
>
> Solution:
>
> Create, specify, implement, and test such an API.  Run the API design past the 292 EG, the Project Lambda folks, and the Oracle internal review council (CCC).
>
> Testing:
>
> Regular JSR 292 regression tests, plus a new jtreg test with positive and 3 kinds of negative tests, in hundreds of combinations.  (See below.)
>
> — John
>
> P.S. Output from RevealDirectTest.java.  (It runs with and without a security manager.)
>
> @Test testSimple executed 107 positive tests in 446 ms
> @Test testPublicLookup/1 executed 56 positive tests in 99 ms
> @Test testPublicLookup/2 executed 80 positive tests in 551 ms
> @Test testPublicLookup/3 executed 56 positive tests in 47 ms
> @Test testPublicLookupNegative/1 executed 23/0/0 negative tests in 2 ms
> @Test testPublicLookupNegative/2 executed 0/27/0 negative tests in 4 ms
> @Test testPublicLookupNegative/3 executed 0/0/27 negative tests in 10 ms
> @Test testJavaLangClass executed 60 positive tests in 67 ms
> @Test testCallerSensitive executed 30 positive tests in 425 ms
> @Test testCallerSensitiveNegative executed 12/0/0 negative tests in 1 ms
> @Test testMethodHandleNatives executed 4 positive tests in 5 ms
> @Test testMethodHandleInvokes/1 executed 640 positive tests in 828 ms
> @Test testMethodHandleInvokes/2 executed 640 positive tests in 177 ms
>
> _______________________________________________
> mlvm-dev mailing list
> mlvm-dev at openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>