[12] RFR (M) JDK-8205611: Improve the wording of LinkageErrors to include module and class loader information

[Lindenmaier, Goetz]

Hi Lois, 

Looks good, thanks for all the changes!

Best regards,
  Goetz.

> -----Original Message-----
> From: Lois Foltan <lois.foltan at oracle.com>
> Sent: Thursday, July 12, 2018 9:10 PM
> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; hotspot-dev
> developers <hotspot-dev at openjdk.java.net>
> Subject: Re: [12] RFR (M) JDK-8205611: Improve the wording of LinkageErrors
> to include module and class loader information
> 
> On 7/12/2018 11:03 AM, Lindenmaier, Goetz wrote:
> 
> > Hi Lois,
> >
> > I had a look at your change. It's a much better format bearing the
> > same information. Thus looks good to me.
> >
> > I would appreciate if you change printing the messages in linkResolver.cpp
> and
> > klassVtable.cpp to use stringStream, similar to linkResolver.cpp:1020
> > and other places (I should have changed that in first place when
> > I extended these messages before).
> >
> > systemDictionary.cpp
> >    I think it's convention to have a space after the comma:
> >    existing_klass->class_in_module_of_loader(false,true));
> >   This is in several places.
> 
> Thanks Goetz!  All you comments have been addressed.  See new webrev at
> 
> http://cr.openjdk.java.net/~lfoltan/bug_jdk8205611.1/webrev/
> 
> Lois
> >
> > Best regards,
> >    Goetz.
> >
> >
> >
> >> -----Original Message-----
> >> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On
> >> Behalf Of Lois Foltan
> >> Sent: Mittwoch, 27. Juni 2018 15:20
> >> To: hotspot-dev developers <hotspot-dev at openjdk.java.net>
> >> Subject: [12] RFR (M) JDK-8205611: Improve the wording of LinkageErrors
> to
> >> include module and class loader information
> >>
> >> Please review this change to migrate existing loader constraint
> >> LinkageErrors to the new error message format proposal.  The actual
> >> wording of the loader constraint messages has not changed.  Module and
> >> class loader information have been moved into the error message's
> REASON
> >> section.  This change also removes the method
> >> java_lang_ClassLoader::describe_external() in favor of
> >> Klass::class_in_module_of_loader().
> >>
> >> open webrev at
> >> http://cr.openjdk.java.net/~lfoltan/bug_jdk8205611/webrev/
> >> bug link https://bugs.openjdk.java.net/browse/JDK-8205611
> >>
> >> JDK-8166633 outlines a new proposal where error messages follow a
> format
> >> of ERRROR: PROBLEM (REASON) where the PROBLEM is aggressively
> simple
> >> (and definitely avoids arbitrary-length loader names) so the REASON
> >> bears all the cost of explaining the PROBLEM with more specifics.  See
> >> the proposal in more detail at
> >> https://bugs.openjdk.java.net/browse/JDK-8166633. The new utility
> method
> >> Klass::class_in_module_of_loader() implements the proposed format.
> >>
> >> Some example text:
> >>
> >> (JDK 11) 'DuplicateLE_Test_Loader_IF' @<id> (instance of
> >> PreemptingClassLoader, child of 'app'
> >> jdk.internal.loader.ClassLoaders$AppClassLoader) attempted duplicate
> >> interface definition for test.J.
> >> to
> >> (JDK 12) loader 'DuplicateLE_Test_Loader_IF' @6eeee674 attempted
> >> duplicate interface definition for test.J. (test.J is in unnamed module
> >> of loader 'DuplicateLE_Test_Loader_IF' @6eeee674, parent loader 'app')
> >>
> >>
> >> (JDK 11) loader constraint violation: loader PreemptingClassLoader @<id>
> >> (instance of PreemptingClassLoader, child of 'app'
> >> jdk.internal.loader.ClassLoaders$AppClassLoader) wants to load class
> >> test.D_ambgs. A different class with the same name was previously
> loaded
> >> by 'app' (instance of jdk.internal.loader.ClassLoaders$AppClassLoader).
> >> to
> >> (JDK 12) loader constraint violation: loader PreemptingClassLoader
> >> @5bc79a1c wants to load class test.D_ambgs. A different class with the
> >> same name was previously loaded by 'app'. (test.D_ambgs is in unnamed
> >> module of loader 'app')
> >>
> >>
> >> (JDK 11) loader constraint violation for class test.Task: when selecting
> >> overriding method test.Task.m()Ltest/Foo; the class loader
> >> PreemptingClassLoader @<id> (instance of PreemptingClassLoader, child
> of
> >> 'app' jdk.internal.loader.ClassLoaders$AppClassLoader) of the selected
> >> method's type test.Task, and the class loader 'app' (instance of
> >> jdk.internal.loader.ClassLoaders$AppClassLoader) for its super type
> >> test.J have different Class objects for the type test.Foo used in the
> >> signature
> >> to
> >> (JDK 12) loader constraint violation for class test.Task: when selecting
> >> overriding method test.Task.m()Ltest/Foo; the class loader
> >> PreemptingClassLoader @7884e077 of the selected method's type
> test.Task,
> >> and the class loader 'app' for its super type test.J have different
> >> Class objects for the type test.Foo used in the signature (test.Task is
> >> in unnamed module of loader PreemptingClassLoader @7884e077, parent
> >> loader 'app'; test.J is in unnamed module of loader 'app')
> >>
> >>
> >> Testing: hs-tier(1-3), jdk-tier(1-3) complete
> >>                  hs-tier(4,5) in progress
> >>                  JCK vm, lang in progress
> >>
> >> Thanks,
> >> Lois