RFR: 8190837: BasicType and BasicTypeSize should refer to HotSpot values

[Chris Plummer]

On 11/28/17 4:51 PM, Yasumasa Suenaga wrote:
> Hi Chris,
>
> 2017-11-29 5:32 GMT+09:00 Chris Plummer <chris.plummer at oracle.com>:
>> Hi Yasumasa,
>>
>> This isn't code I know very well, and I'm not a Reviewer. Just a couple of
>> observations.
>>
>> I wonder if the person who originally suggested this change realized the
>> disruption it would have to existing switch statements. I'm not saying the
>> change shouldn't be done for this reason, but it is something to at least
>> consider.
> According to JLS, `case` label need to have constant expression.
> We cannot set `static final` to the field which is set at initialize().
>
> https://docs.oracle.com/javase/specs/jls/se9/html/jls-14.html#jls-14.11
I understood the reason for getting rid of the case statements. I was 
just wondering if you weighed this code disruption vs. the value of what 
you are fixing.
>
>
>> Your coding pattern for the following differs from the existing 200+
>> instances of VM.registerVMInitializedObserver() calls already in place. I
>> suggest you be consistent with existing code.
>>
>>    71   static {
>>    72     VM.registerVMInitializedObserver(
>>    73         (o, d) -> initialize(VM.getVM().getTypeDataBase()));
>>    74   }
> This style has been used in JavaThreadsPanel.java .
Ah, I missed that one case, but then it's one that you added. :)
> I like it because it is simple.
>
> I will change it to traditional style if other reviewer(s) suggest it.
I think consistency is important.

thanks,

Chris
>
>
> Thanks,
>
> Yasumasa
>
>
>> thanks,
>>
>> Chris
>>
>>
>> On 11/27/17 11:49 PM, Yasumasa Suenaga wrote:
>>
>> Hi all,
>>
>> Enum values in BasicType and BasicTypeSize are declared as const
>> values. However, it makes error prone when existing enum values
>> change.
>> They should refer to HotSpot values via VMStructs.
>>
>> This issue has been pointed by Jini [1].
>>
>> I uploaded webrev for this issue. Could you review it?
>>
>>    http://cr.openjdk.java.net/~ysuenaga/JDK-8190837/webrev.00/
>>
>> I cannot access JPRT. So I need a sponsor.
>>
>>
>> Thanks,
>>
>> Yasumasa
>>
>>
>> [1]
>> http://mail.openjdk.java.net/pipermail/serviceability-dev/2017-October/021965.html
>>
>>