RFR 8202913: loader constraint message for fields specifies incorrect referring class

[Karen Kinnear]

Harold,

Looks good.

thanks,
Karen

> On May 30, 2018, at 9:07 AM, Harold David Seigel <harold.seigel at oracle.com> wrote:
> 
> Hi Goetz,
> 
> Thanks for reviewing this change!
> 
> Please review this updated webrev for this bug:
> 
>   http://cr.openjdk.java.net/~hseigel/bug_8202913.2/webrev/index.html <http://cr.openjdk.java.net/~hseigel/bug_8202913.2/webrev/index.html>
> 
> The new exception message for the included test looks like this:
> 
>     loader constraint violation: when resolving field "_field1" of
>   type pkg.Foo, the class loader "<unnamed>" (instance of
>   pkg.ClassLoaderForChildGrandFoo, child of "app"
>   jdk.internal.loader.ClassLoaders$AppClassLoader) of the current
>   class, pkg.Child, and the class loader "<unnamed>" (instance of
>   pkg.ClassLoaderForParentFoo, child of "app"
>   jdk.internal.loader.ClassLoaders$AppClassLoader) for the field's
>   defining type, pkg.Parent, have different Class objects for type pkg.Foo
> 
> Please also see comments in-line.
> 
> 
> On 5/25/2018 10:23 AM, Lindenmaier, Goetz wrote:
>> Hi,
>> 
>>> I think this would read better if you replace 'type' by what is returned by
>>> Klass::external_kind().
>> correcting myself, you can just say 'class' ... interfaces don't define fields ...
> Karen pointed out that interfaces can have public static fields. So, I'll keep it as 'type', not 'class'.
>> 
>> Best regards,
>>   Goetz.
>> 
>> 
>>> -----Original Message-----
>>> From: Lindenmaier, Goetz
>>> Sent: Freitag, 25. Mai 2018 16:18
>>> To: 'Harold David Seigel' <harold.seigel at oracle.com>; hotspot-runtime-
>>> dev at openjdk.java.net
>>> Subject: RE: RFR 8202913: loader constraint message for fields specifies
>>> incorrect referring class
>>> 
>>> Hi Harold,
>>> 
>>> Thanks for adding this further improvement over my change.
>>> 
>>>> ... AppClassLoader) for the field's
>>>>  defining type, Parent, have different Class objects for type Foo
>>> I think this would read better if you replace 'type' by what is returned by
>>> Klass::external_kind().
>>> 
>>> Actually I would prefer to read
>>> ... AppClassLoader) for the class|abstractclass|interface test.Parent defining
>>> this field,
>>> have different Class objects for type Foo
> The message is already too wordy.  I'd prefer to not add this additional text because it is not related to the reason for the exception.
>>> 
>>> Could you please add the test files into some package so that you
>>> can assure class names are printed as test.Parent and not test/Parent?
>>> There is external_name() to get the proper names from classes, I don't
>>> know for symbols.
>>> Related tests are in runtime/LoaderConstraint. Please move your test
>>> there.
> Done.
> 
> Thanks, Harold
>>> 
>>> Best regards,
>>>   Goetz.
>>> 
>>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: hotspot-runtime-dev [mailto:hotspot-runtime-dev-
>>>> bounces at openjdk.java.net] On Behalf Of Harold David Seigel
>>>> Sent: Freitag, 25. Mai 2018 15:29
>>>> To: hotspot-runtime-dev at openjdk.java.net
>>>> Subject: RFR 8202913: loader constraint message for fields specifies
>>> incorrect
>>>> referring class
>>>> 
>>>> Hi,
>>>> 
>>>> Please review this change to correct and simplify the error message
>>>> displayed when a loader constraint check fails when trying to access a
>>>> field.
>>>> 
>>>> The old message (for this test case):
>>>> 
>>>>       loader constraint violation: when resolving field "_field1" the
>>>>     class loader "<unnamed>" (instance of ClassLoaderForChildGrandFoo,
>>>>     child of "app" jdk.internal.loader.ClassLoaders$AppClassLoader) of
>>>>     the referring class, Parent, and the class loader "<unnamed>"
>>>>     (instance of ClassLoaderForParentFoo, child of "app"
>>>>     jdk.internal.loader.ClassLoaders$AppClassLoader) for the field's
>>>>     resolved type, Foo, have different Class objects for that type
>>>> 
>>>> The new message:
>>>> 
>>>>     loader constraint violation: when resolving field "_field1" of type
>>>>     Foo, the class loader "<unnamed>" (instance of
>>>>     ClassLoaderForChildGrandFoo, child of "app"
>>>>     jdk.internal.loader.ClassLoaders$AppClassLoader) of the current
>>>>     class, Child, and the class loader "<unnamed>" (instance of
>>>>     ClassLoaderForParentFoo, child of "app"
>>>>     jdk.internal.loader.ClassLoaders$AppClassLoader) for the field's
>>>>     defining type, Parent, have different Class objects for type Foo
>>>> 
>>>> Open Webrev: http://cr.openjdk.java.net/~hseigel/bug_8202913/webrev/
>>>> 
>>>> JBS Bug:  https://bugs.openjdk.java.net/browse/JDK-8202913
>>>> 
>>>> This fix was tested with Mach5 tiers 1 and 2 tests and builds on
>>>> Linux-X64, Windows, Solaris Sparc, and Mac OS X, with tiers 3-5 tests on
>>>> Linux-x64, and with JCK-11 Lang and VM tests.
>>>> 
>>>> Thanks, Harold