RFR add lworld specific value type checks to classFileParser

[harold seigel]

Thanks Karen!

Harold

On 3/22/2018 12:12 PM, Karen Kinnear wrote:
>
> Harold,
>
> Thanks for the updates and sharing the webrev.
>
> #1: it is not the case that “Arrays are always flattenable”. It is the 
> case that “Arrays of value classes are always flattenable”.
> Would you mind changing that again?
> #3 - thank you for the comment update
> #4 - looks good.
>
> thanks,
> Karen
>
> p.s. I don’t need to see it again :-)
>
>
>> On Mar 22, 2018, at 10:08 AM, harold seigel <harold.seigel at oracle.com 
>> <mailto:harold.seigel at oracle.com>> wrote:
>>
>> Hi Karen,
>>
>> Thanks for reviewing this!
>>
>> Please review this updated webrev.  It addresses the comments 1, 3, 
>> and 4 that you made below.
>>
>>     http://cr.openjdk.java.net/~hseigel/valueTypes_lworld.cfp.2/webrev/index.html
>>
>> Additionally, I opened bug JDK-8200069 
>> <https://bugs.openjdk.java.net/browse/JDK-8200069> for comment 2.
>>
>>
> Thank you - I like Mr Simms’ approach to filing rfes and bugs with the 
> lworld label.
>>
>> Thanks, Harold
>>
>>
>> On 3/21/2018 2:48 PM, Karen Kinnear wrote:
>>> Harold,
>>>
>>> Thank you so much for all of these changes.Code looks good. Glad jcod worked for you.
>>>
>>> Minor comments:
>>> 1. classFileParser.cpp line 1622 “Arrays are not flattenable” -> actually value type arrays
>>> are always flattenable with our prototype, so perhaps “Value Type arrays are always flattenable, ACC_FLATTENABLE not allowed”
>>> or something. Would require a change to line 7 in the test as well
>>>
>>> 2. Thanks for commenting out the <init> is not ok in a value class - could you possibly file an rfe
>>> so we remember to clean this up when this is decided?
>>>
>>> 3. lines 4013-4016
>>> Thank you for the comments as to why we can check flattenable_field circularity and superclass circularity
>>> independently.
>>> Could you also add a comment as to why we can check flattenable_field circularity and superinterface
>>> circularity separately? That is because interfaces can not be value types.
>>>
>>> 4. systemDictionary.cpp
>>> resolve_flattenable_field_or_fail
>>>    I don’t think you need to do the register_loader here - I think resolve_or_fail will do that for you - but
>>> you can double-check with Coleen.
>>>
>>> thanks!
>>> Karen
>>>
>>>
>>>
>>>> On Mar 19, 2018, at 4:11 PM, harold seigel<harold.seigel at oracle.com>  wrote:
>>>>
>>>> Hi,
>>>>
>>>> Please review the following webrev:
>>>>
>>>>    http://cr.openjdk.java.net/~hseigel/valueTypes_lworld.cfp.Mar19/
>>>>
>>>> The webrev adds checks to the JVM classFileParser.cpp source for the following:
>>>>
>>>> * A class with ACC_VALUE and ACC_INTERFACE or ACC_ABSTRACT or ACC_ENUM
>>>>    is invalid
>>>> * java.lang.Object is the only valid super type for value types
>>>> * A class with ACC_VALUE must be final
>>>> * All instance fields in an ACC_VALUE class must be marked ACC_FINAL
>>>> * A class with ACC_VALUE cannot have a method called <init>
>>>> * A class with ACC_VALUE cannot have any synchronized instance methods
>>>> * A field marked ACC_FLATTENABLE cannot be an array
>>>>
>>>> Also, all ACC_FLATTENABLE fields are now pre-loaded and checked for circularity.
>>>>
>>>> (Note that the change to test ValueOops.java is just to remove a <tab> character.)
>>>>
>>>> The webrev was tested with hotspot/jtreg tests and many jdk/jtreg tests.
>>>>
>>>> Thanks, Harold
>>
>