RFR: 8266822: Rename MetaspaceShared::is_old_class to be more explicit about what "old" means [v6]

[David Holmes]

On 17/05/2021 4:39 pm, Ioi Lam wrote:
> On Tue, 11 May 2021 22:47:44 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:
> 
>>> Please review this simple patch for renaming the function from `MetaspaceShared::is_old_class` to `MetaspaceShared::has_old_class_version`. Also added some comment to the function.
>>>
>>> Tests:
>>> - [x] tier1, 2
>>
>> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
>>
>>    declare has_old_class_version as a const method
> 
>> _Mailing list message from [David Holmes](mailto:david.holmes at oracle.com) on [hotspot-runtime-dev](mailto:hotspot-runtime-dev at mail.openjdk.java.net):_
>>
>> On 15/05/2021 7:14 am, Ioi Lam wrote:
>>
>>> On Tue, 11 May 2021 22:47:44 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:
>>>>> Please review this simple patch for renaming the function from `MetaspaceShared::is_old_class` to `MetaspaceShared::has_old_class_version`. Also added some comment to the function.
>>>>> Tests:
>>>>> - [x] tier1, 2
>>>>
>>>>
>>>> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
>>>> declare has_old_class_version as a const method
>>>
>>>
>>>> _Mailing list message from [David Holmes](mailto:david.holmes at oracle.com) on [hotspot-runtime-dev](mailto:hotspot-runtime-dev at mail.openjdk.java.net):_
>>>> Hi Calvin,
>>>> On 12/05/2021 2:23 am, Calvin Cheung wrote:
>>>>> Please review this simple patch for renaming the function from `MetaspaceShared::is_old_class` to `MetaspaceShared::has_old_class_version`. Also added some comment to the function.
>>>>
>>>>
>>>> I guess you missed the fact that I had changed the bug synopsis. Why not
>>>> call this needs_old_verifier (or something like that) so that the exact
>>>> meaning of this method is more clear?
>>>
>>>
>>> needs_old_verifier isn't clear, either. `has_old_class_version(K)` returns true if K is "new" but one of K's supertypes is "old". K itself does not "need the old verifier".
>>
>> and k itself does not "have an old class version" so the current method
>> is even more misleadingly named!
>>
>> How about inverting it and having has_post_Java_6_hierarchy: return true
>> if k and all its supertypes are post-Java-6 classfile version.
> 
> It's unclear whether hierarchy refers to the super types, subtypes, or both.

The hierarchy from a given class is always up.

>> Seriously we must be able to name this method in a more meaningful way. :(
> 
> Trying to describe what the function does in a short and meaningful name is not always possible.
> 
> In this case, I think it's better to say what the function intends to do. `InstanceKlass::can_be_verified_at_dumptime()` will probably suffice. It's unlikely that this function will be used for any other purpose.

That works for me.

Thanks,
David

> 
> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/3983
>