RFR(M): 8199940: Print more information about class loaders in IllegalAccessErrors.

[Lois Foltan]

> Hi Mandy,
>
> I like this proposal a lot.
>   * it contains all necessary information
>   * I like that it also contains the @id
>   * I like that the major message is in a clear sentence at the
>     beginning, the further information in parentheses at the end.
>
> I don't care about single or double quotes, but in the messages
> I only saw double quotes so far. A lot of messages will have to
> be changed to make this consistent.
>
> About the implementation:
>
> Do I understand correctly that
> classLoaderData::loader_name() should be changed to return either of
>    com.acme.MySameClassLoader at 423aefd2
>    'app'
>    'Cool lib loader' @4567    (is the space before @ intended?)
> and Klass::class_loader_and_module_name(verbose) should return
>    test.IAE1_B in unnamed module of loader com.acme.MySameClassLoader at 423aefd2
>    test.IAE1_A in unnamed module of loader 'app'
>    p2.C2 in module m2x at 9.0 of loader 'app'
>    p1.C1 in module m1x at 2.0 of loader 'app'
>
> Also, there currently are
>    Klass::external_name()
>    Klass::class_loader_and_module_name()
>    classLoaderData::loader_name()
>    java_lang_ClassLoader::name()
>    java_lang_ClassLoader::describe_external()
>
> These function names are often misunderstood.
> It's unclear that classLoaderData::loader_name() and
> java_lang_ClassLoader::name() do different things,
> loader_name let's one assume it returns the name field
> of the ClassLoader class.
Hi Goetz,
Glad you like the proposal.  I am currently working on consolidating all 
the above methods you list into something more consistent for class 
loaders.  See https://bugs.openjdk.java.net/browse/JDK-8202605.

>
> Klass::class_loader_and_module_name() sounds as if
> only the name of the classloader and the name of
> the module are printed, not the name of the class.
>
> Maybe we should rename like this:
>    Klass::class_loader_and_module_name() --> full_class_description
>    classLoaderData::loader_name()        --> full_loader_description

I will also be working on implementing new utility methods probably 
within Klass to be used consistently by the various error messages. See 
the RFE, https://bugs.openjdk.java.net/browse/JDK-8169559.  I may decide 
to either remove Klass::class_loader_and_module_name() or rename it to 
something very specific to indicate that it is the format that is used 
for StackTraceElement toString() as documented at 
https://docs.oracle.com/javase/9/docs/api/java/lang/StackTraceElement.html#toString--. 
So there may be value in keeping it.  I will address this as well in 
JDK-8169559.

Thanks,
Lois

>
> Best regards,
>    Goetz.
>
>> -----Original Message-----
>> From: mandy chung [mailto:mandy.chung at oracle.com]
>> Sent: Donnerstag, 7. Juni 2018 19:49
>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>; Lois Foltan
>> <lois.foltan at oracle.com>
>> Cc: hotspot-runtime-dev at openjdk.java.net
>> Subject: Re: RFR(M): 8199940: Print more information about class loaders in
>> IllegalAccessErrors.
>>
>> Hi Goetz,
>>
>> We have discussed several options to include the loader info that
>> developers reading the exception message can easily tell the
>> problem and reason. We look at a few exception messages and
>> like the following most:
>>
>> class test.IAE1_B cannot access its superinterface test.IAE1_A
>> (test.IAE1_B in unnamed module of loader com.acme.MySameClassLoader
>> @423aefd2; test.IAE1_A in unnamed module of loader 'app')
>>
>> class p1.C1 cannot access private method p2.C2.method2()V
>> (p1.C1 in module m1x at 2.0 of loader 'app'; p2.C2 in module m2x at 9.0 of
>> loader 'app')
>>
>> class p.C cannot access class jdk.internal.misc.VM (p.C in unnamed
>> module of loader 'Cool lib loader' @4567; jdk.internal.misc.VM in module
>> java.base; java.base does not export jdk.internal.misc to the unnamed
>> module)
>>
>> loader constraint violation for type pkg.Foo:
>> class pkg.Child tried to access pkg.Parent._field1, a field of type
>> pkg.Foo, but pkg.Child and pkg.Parent see different definitions of
>> pkg.Foo
>> (pkg.Child in unnamed module of loader pkg.ClassLoaderForChildGrandFoo
>> @123, parent loader 'app'; pkg.Parent in unnamed module of loader
>> pkg.ClassLoaderForParentFoo @456, parent loader 'app')
>>
>> Below describes the preliminary proposal of the format.
>>
>> If the loader name is explicitly specified then
>>     ... of loader '<name>' @<id>
>>
>> If the loader name is not explicitly specified
>>     ... of loader <qualified-type-name> @<id>
>>
>> - single-quote denotes it's explicitly specified loader name.
>>     The loader name explicitly specified may contain whitespace.
>>
>> - @<id> can distinct different instances with the same loader name
>>     or of the same type.
>>
>> - omit @<id> for built-in loaders
>>
>> Your feedback is appreciated.
>>
>> Lois is on vacation.  I think it may be best for her to take this RFE
>> and implement the new format once we agree on the proposal as this
>> would need to update some exception wordings to make the problem
>> and reason very clear.
>>
>> Mandy