Hi Chris, 2017-11-29 5:32 GMT+09:00 Chris Plummer <chris.plum...@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 > 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 . I like it because it is simple. I will change it to traditional style if other reviewer(s) suggest it. 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 > >
