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

[Lois Foltan]

Thanks Goetz for the review!
Lois

On 7/13/2018 1:43 AM, Lindenmaier, Goetz wrote:
> 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