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@openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
>
_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
