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

[Harold David Seigel]

Thanks Karen!

Harold


On 5/30/2018 9:17 AM, Karen Kinnear wrote:
> Harold,
>
> Looks good.
>
> thanks,
> Karen
>
>> On May 30, 2018, at 9:07 AM, Harold David Seigel 
>> <harold.seigel at oracle.com <mailto: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/%7Ehseigel/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 
>>>> <mailto:harold.seigel at oracle.com>>; hotspot-runtime-
>>>> dev at openjdk.java.net <mailto: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 <mailto: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 
>>>>> <mailto: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/ 
>>>>> <http://cr.openjdk.java.net/%7Ehseigel/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
>