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

[Harold David Seigel]

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

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