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

[coleen.phillimore at oracle.com]

This change looks really good!  The new error format in linkResolver and 
klassVtable is nice.
Thank you all for the work to agree on a format!
Coleen


On 7/12/18 3:10 PM, Lois Foltan wrote:
> 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
>