review request (XL): 6839872: remove implementation inheritance from JSR 292 APIs
Rémi Forax
forax at univ-mlv.fr
Thu Mar 3 02:44:09 PST 2011
Hi John & Christian,
I've noticed several gotchas.
in CallSite:
- makesite you create a InvokeDynamicBootstrapError but forget to
throw it.
- TRANSITIONAL_BEFORE_PFD is not final
- maybeReBox can be split in two methods to avoid to double-check if a
value is an instanceof Integer.
in MethodHandle:
1 typo, line 451: by by
Also I'm not a big fan of using an import static for something else
than cos() and sin().
It takes me times to find getNameString().
in MethodHandleStatics:
You should split getNameString in 3 overloaded methods:
getNameString(MethodHandle, MethodType)
getNameString(MethodHandle)
getNameString(MethodHandle, MethodHandle)
The last twos should delegate to getNameString(MethodHandle, MethodType)
Rémi
On 02/27/2011 11:18 AM, John Rose wrote:
> http://cr.openjdk.java.net/~jrose/6839872/jdk-webrev.00/
> <http://cr.openjdk.java.net/%7Ejrose/6839872/jdk-webrev.00/>
>
> Summary: move everything into a single package; remove all
> multi-package machinery
>
> This is a complex change which consolidates the code into a single
> package, to prepare for a clean rename from java.dyn to java.lang.invoke.
>
> This also fixes some defects in the API which arise from dependencies
> between multiple packages. That was the original motivation of
> bug 6839872.
>
> For ease of review, this change may be reviewed in seven parts:
> - meth-impl-6839872.1-rename.patch moves files between packages
> - meth-impl-6839872.2-super.patch merges sun.dyn superclasses into
> java.dyn subclasses
> - meth-impl-6839872.3-moves.patch moves some method code around in
> java.dyn classes
> - meth-impl-6839872.4-vconv.patch addresses a dependency from
> ValueConversions
> - meth-impl-6839872.5-mtform.patch removes a cross-package wormhole
> from MethodType & MethodTypeForm
> - meth-impl-6839872.6-access.patch removes cross-package access checking
> - meth-impl-6839872.7-misc.patch is a few more miscellaneous changes
>
> These individual patches may be found in the mlvm repository:
> http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/tip/
>
> -rw-r--r-- 36661 meth-impl-6839872.1-rename.patch
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/ae2641d94773/meth-impl-6839872.1-rename.patch>
> file
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/ae2641d94773/meth-impl-6839872.1-rename.patch> |
> revisions
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/log/ae2641d94773/meth-impl-6839872.1-rename.patch> |
> annotate
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/annotate/ae2641d94773/meth-impl-6839872.1-rename.patch>
>
> -rw-r--r-- 24772 meth-impl-6839872.2-super.patch
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/ae2641d94773/meth-impl-6839872.2-super.patch>
> file
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/ae2641d94773/meth-impl-6839872.2-super.patch> |
> revisions
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/log/ae2641d94773/meth-impl-6839872.2-super.patch> |
> annotate
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/annotate/ae2641d94773/meth-impl-6839872.2-super.patch>
>
> -rw-r--r-- 45216 meth-impl-6839872.3-moves.patch
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/ae2641d94773/meth-impl-6839872.3-moves.patch>
> file
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/ae2641d94773/meth-impl-6839872.3-moves.patch> |
> revisions
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/log/ae2641d94773/meth-impl-6839872.3-moves.patch> |
> annotate
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/annotate/ae2641d94773/meth-impl-6839872.3-moves.patch>
>
> -rw-r--r-- 6895 meth-impl-6839872.4-vconv.patch
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/ae2641d94773/meth-impl-6839872.4-vconv.patch>
> file
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/ae2641d94773/meth-impl-6839872.4-vconv.patch> |
> revisions
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/log/ae2641d94773/meth-impl-6839872.4-vconv.patch> |
> annotate
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/annotate/ae2641d94773/meth-impl-6839872.4-vconv.patch>
>
> -rw-r--r-- 23819 meth-impl-6839872.5-mtform.patch
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/ae2641d94773/meth-impl-6839872.5-mtform.patch>
> file
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/ae2641d94773/meth-impl-6839872.5-mtform.patch> |
> revisions
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/log/ae2641d94773/meth-impl-6839872.5-mtform.patch> |
> annotate
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/annotate/ae2641d94773/meth-impl-6839872.5-mtform.patch>
>
> -rw-r--r-- 89039 meth-impl-6839872.6-access.patch
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/ae2641d94773/meth-impl-6839872.6-access.patch>
> file
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/ae2641d94773/meth-impl-6839872.6-access.patch> |
> revisions
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/log/ae2641d94773/meth-impl-6839872.6-access.patch> |
> annotate
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/annotate/ae2641d94773/meth-impl-6839872.6-access.patch>
>
> -rw-r--r-- 19404 meth-impl-6839872.7-misc.patch
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/ae2641d94773/meth-impl-6839872.7-misc.patch>
> file
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/ae2641d94773/meth-impl-6839872.7-misc.patch> |
> revisions
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/log/ae2641d94773/meth-impl-6839872.7-misc.patch> |
> annotate
> <http://hg.openjdk.java.net/mlvm/mlvm/jdk/annotate/ae2641d94773/meth-impl-6839872.7-misc.patch>
>
>
>
>
>
>
> ____________________________________________________________
>
> http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/tip/meth-impl-6839872.1-rename.patch
>
> rev 3506 : factored part of meth-impl-6839872.patch
>
> Move almost all files from sun.dyn.* to java.dyn.
>
> Exceptions:
> - WrapperInstance needs to be a public interface, but is not part of
> API, so must be in sun.dyn.
> - sun.dyn.Access is going away in a subsequent part of this change set
> - package-info.java stays
>
> Other changes:
> - change occurrences of Access.TOKEN to IMPL_TOKEN
> - added java.dyn.MethodHandleStatics to hold IMPL_TOKEN, temporarily
> - asked NetBeans to fix imports in all files
> - remove MemberName-based test
> - remove MethodTypeForm (*)
>
> The removal of MethodTypeForm is a breaking change.
> Apart from this change, the software continues to build correctly
> after this patch.
>
> ____________________________________________________________
>
> http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/tip/meth-impl-6839872.2-super.patch
>
> rev 3507 : factored part of meth-impl-6839872.patch
>
> - After move from sun.dyn.* to java.dyn, merge sun.dyn superclasses
> into java.dyn subclasses.
> - Move fields and constructor for MethodHandleImpl into MethodHandle.
> Keep factory methods.
> - Rename MethodTypeImpl to MethodTypeForm, thereby moving fields and
> constructor.
>
> After this change, this JDK software builds and passes basic unit
> tests with the corresponding JVM.
>
> ____________________________________________________________
>
> http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/tip/meth-impl-6839872.3-moves.patch
>
> rev 3508 : factored part of meth-impl-6839872.patch
>
> Move some misplaced methods and constants between classes.
>
> Also:
> - Move some functions verbatim into MethodHandleStatics.
> - Move raiseException and checkSpreadArgument into
> MethodHandleNatives, for the JVM.
> - Move all of CallSiteImpl into CallSite.
> - Move some exception creation functions from MemberName into
> MethodHandleStatics, with light refactoring.
> - Change newNoAccessException to a virtual function
> MemberName.makeAccessException.
> - Remove an extra copy of IMPL_LOOKUP; import it regularly from
> MethodHandles.Lookup.
> - Remove an inconvenient dependency from MethodHandles.<clinit> to
> IMPL_LOOKUP.
>
> After this change, this JDK software builds and passes basic unit
> tests with the corresponding JVM.
>
> ____________________________________________________________
>
> http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/tip/meth-impl-6839872.4-vconv.patch
>
> rev 3509 : factored part of meth-impl-6839872.patch
>
> Deal with an external dependency from sun.dyn.util.ValueConversions to
> privileged MH access.
> Remove "raw retype" capabilities from ValueConversions, moving them
> into the trusted package.
>
> After this change, this JDK software builds and passes basic unit
> tests with the corresponding JVM.
>
> ____________________________________________________________
>
> http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/tip/meth-impl-6839872.5-mtform.patch
>
> rev 3510 : Remove MethodTypeFriend, a "wormhole" for privileged
> communication between a MethodType and its Form.
>
> Use trusted package-private methods on MethodType for such
> communication instead.
>
> Also, put the internal Invokers struct from a virtual method on
> MethodType.
>
> After this change, this JDK software builds and passes basic unit
> tests with the corresponding JVM.
>
> ____________________________________________________________
>
> http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/tip/meth-impl-6839872.6-access.patch
>
> rev 3511 : factored part of meth-impl-6839872.patch
>
> - Remove the Access class and all uses.
> - Remove most public access modifiers on non-public class members.
>
> This is a large volume of simple change.
>
> All privileged methods become non-public, and lose their first "Access
> token" arguments.
>
> After this change, this JDK software builds and passes basic unit
> tests with the corresponding JVM.
>
> ____________________________________________________________
>
> http://hg.openjdk.java.net/mlvm/mlvm/jdk/file/tip/meth-impl-6839872.7-misc.patch
>
> rev 3512 : Miscellaneous changes.
>
> After this change, this JDK software builds and passes basic unit
> tests with the corresponding JVM.
>
>
> _______________________________________________
> mlvm-dev mailing list
> mlvm-dev at openjdk.java.net
> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/mlvm-dev/attachments/20110303/8dc6becd/attachment-0001.html
More information about the mlvm-dev
mailing list