Hi Michael,
On 09/25/2015 03:37 PM, Michael Haupt wrote:
Hi Peter,
thanks for this changeset. Note I'm not a Reviewer (with a capital R); please
read this review in lower-case. ;-)
One question about MethodType: would you mind doing something about the naming
of the newly introduced fromDescriptor() method? Its name does not suggest in
any way that it chooses the class loader differently. The name is subtly
different from that of fromDescriptorString(), and the signature is identical -
it's probably really easy to confuse the two when working at the core libs
level. Unfortunately, the only proposal for a name I can make,
fromDescriptorStringBootCL(), is clunky. Maybe that's acceptable for a
low-level method only visible at package level.
Well, the correct self-describing name would then be
fromDescriptorStringNullIsBootstrapCL()...
Perhaps a note in the javadoc that this is the preferred method for
internal use would be sufficient? We can't make a reference from public
method to the package-private one in javadoc though.
Another option would be to create new method as public API and
@Deprecate the old one. There is a convention that null means bootstrap
loader in other public APIs as well (for example: Class.forName(String
name, boolean initialize, ClassLoader loader); ) although passing null
from user code is rarely needed if at all. This is usually just for
internal use.
Regards, Peter
Am 25.09.2015 um 08:46 schrieb Peter Levart <peter.lev...@gmail.com>:
I have run the tests in java.lang.invoke and only have a problem with 1 test
which seems to be related to the test or jtreg itself and happens also without
my patch applied:
#Test Results (version 2)
#Tue Sep 22 09:48:38 CEST 2015
...
#section:script_messages
----------messages:(0/0)----------
test result: Error. Parse Exception: `@library' must appear before first `@run'
Yes. The test is also marked as ignored until another issue is fixed, so that
can be ignored.
Other than the above remark/suggestion, this looks fine. I'll defer to an
upper-case Reviewer, though.
Best,
Michael
_______________________________________________
mlvm-dev mailing list
mlvm-dev@openjdk.java.net
http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
