RFR: 8003557: NPG: Klass* const k should be const Klass* k.

Yumin Qi yumin.qi at oracle.com
Thu May 9 20:18:56 PDT 2013


Vladimir, Thanks for the review!

Yumin
On 5/9/2013 7:25 PM, Vladimir Kozlov wrote:
> Looks good. Thank you for doing this clean up.
>
> Vladimir
>
> On 5/9/13 5:13 PM, Yumin Qi wrote:
>> Thanks, Coleen
>>
>> On 5/9/2013 2:36 PM, Coleen Phillimore wrote:
>>>
>>> Yumin,
>>>
>>> This isn't as big of a change than I feared.
>>>
>>> http://cr.openjdk.java.net/~minqi/8003557/webrev/src/share/vm/oops/method.cpp.udiff.html 
>>>
>>>
>>>
>>> I don't think the compiler is making you make this change or the other
>>> changes to make local variables const just because they call a const
>>> member function.
>>>
>> The local consts are OK, they are pointers which treat objects as
>> constants. Compiler does not complain this.
>>
>>> http://cr.openjdk.java.net/~minqi/8003557/webrev/src/share/vm/classfile/verifier.cpp.udiff.html 
>>>
>>>
>>> http://cr.openjdk.java.net/~minqi/8003557/webrev/src/share/vm/memory/heapInspection.cpp.udiff.html 
>>>
>>>
>>>
>>> There were two const casts - can you make the target function const
>>> instead?  And see how many lines have to change from that. Otherwise,
>>> make KlassInfoBucket::lookup not take const Klass*.
>>>
>> One reversed, since if set as const, it will chain to set member
>> variable Klass* to be const, then all the functions related to it to be
>> const, which is bad.
>> The other one, ExceptionTable  ctor can take const as input, so no
>> problem here. Also add 'const' to KlassInfoEntry's member functions
>> which are reading only.
>>
>> new webrev:
>>   http://cr.openjdk.java.net/~minqi/8003557/webrev1
>>
>> Thanks
>> Yumin
>>
>>> The verifier one doesn't seem like it'd be that bad.
>>>
>>> Thanks,
>>> Coleen
>>>
>>>
>>> On 05/09/2013 04:10 PM, Yumin Qi wrote:
>>>> Please have codereview for
>>>> 8003557: NPG: Klass* const k should be const Klass* k.
>>>>
>>>> Description: This is leftover from NPG, in which converted const
>>>> KlassOop to Klass* const which did not keep the original intention.
>>>> As a def, const KlassOop is not as it is defined. In this webrev, I
>>>> tried to find/change as many as possible such places, the remaining
>>>> of this work may go on with other fixes in future as discovered.
>>>>
>>>> http://cr.openjdk.java.net/~minqi/8003557/webrev/
>>>>
>>>> Thanks
>>>> Yumin
>>>
>>



More information about the hotspot-runtime-dev mailing list