Thank you, Mandy and Daniel.
As a point of interest, automated testing uncovered a couple things I
had to change before pushing:
http://hg.openjdk.java.net/jdk9/dev/hotspot/rev/accf1676e416
* Updated the CHECK_OFFSET line, so fastdebug builds
http://hg.openjdk.java.net/jdk9/dev/jdk/rev/ddc8f2ae290b
* In the test case, only check for a classloader named 'app' if we are
using the system classloader. This is usually the case, but not when
running regtests using the 'make' target (URLClassLoader).
Thanks,
-Brent
On 12/10/2016 06:51 AM, Daniel Fuchs wrote:
Hi Brent,
This looks really good now!
best regards,
-- daniel
On 10/12/16 01:16, Brent Christian wrote:
On 12/07/2016 04:05 PM, Mandy Chung wrote:
I suggest to add two utility methods rather than the has method:
boolean dropClassLoaderName()
boolean dropModuleVersion()
Done.
430 if (m != null && m.isNamed() &&
431 (isHashedInJavaBase(m) ||
!m.getDescriptor().version().isPresent())) {
432 bits |= JDK_NON_UPGRADEABLE_MODULE;
433 }
I think this should simply be:
if (isHashedInJavaBase(m)) {..}
Done.
Can you retain the javadoc of toLoaderModuleClassName, revised if
appropriate, in the computeFormat method?
Updated.
line 322-325: what about:
The toString method may return two different values on two
StackTraceElement instances that are equal for example when
one created via the constructor and one obtained from Throwable
or StackFrame where an implementation may choose to omit some
element in the returned string.
That sounds good.
Is @apiNote in equals necessary? Maybe the one added in toString is
adequate?
I'm fine without it - removed.
I also fixed test code for a case which only works with the images build.
Updated webrev:
http://cr.openjdk.java.net/~bchristi/8169389/webrev.04/
-Brent