RFR: 8152844: JVM InstanceKlass Methods For Obtaining Package/Module Should Be Moved to Klass

[Coleen Phillimore]

Great!  Sounds good (better hurry, JPRT is going down at 3)
Coleen

On 4/22/16 11:22 AM, Rachel Protacio wrote:
> Hi, Coleen,
>
> Thanks for the review! I'll remove the arrayKlass entries and, seeing 
> no other objections, will commit.
>
> Rachel
>
> On 4/22/2016 9:24 AM, Coleen Phillimore wrote:
>>
>> Hi,
>>
>> http://cr.openjdk.java.net/~rprotacio/8152844.00/src/share/vm/oops/arrayKlass.cpp.udiff.html 
>>
>>
>> I don't think you need module or package declared in arrayKlass.hpp 
>> since it should be abstract also, ie. there should be no instances of 
>> ArrayKlass.
>>
>> Otherwise, looks great.  I love seeing those InstanceKlass::casts go 
>> away.
>>
>> Coleen
>>
>>
>> On 4/21/16 10:26 PM, David Holmes wrote:
>>> Hi Rachel,
>>>
>>> On 22/04/2016 6:11 AM, Rachel Protacio wrote:
>>>> Hello,
>>>>
>>>> Please review this fix for the package() and module() functions, 
>>>> making
>>>> them pure virtual functions of Klass and implementing them in the
>>>> relevant child classes. This cleans up some verbose and repetitive 
>>>> code
>>>> that was casting and checking klass types. Also added test cases to
>>>> runtime/modules/getModuleJNI/GetModule.java, ensuring that all the
>>>> module() functions work as expected.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8152844
>>>> Open webrev: http://cr.openjdk.java.net/~rprotacio/8152844.00/
>>>
>>> The refactoring all looks good to me (though C++ annoys me in 
>>> needing to re-declare the functions in each subclass :) ).
>>>
>>> Cheers,
>>> David
>>>
>>>> Tested with jtreg tests in jdk/test (java/lang, java/io, 
>>>> java/security,
>>>> java/util, tools/jimage, tools/jlink, and tools/jmod), JPRT, and RBT
>>>> hotspot and non-colo tests.
>>>>
>>>> Thanks!
>>>> Rachel
>>
>