On Thu, 28 Apr 2022 11:05:45 GMT, Kim Barrett <kbarr...@openjdk.org> wrote:
>> Albert Mingkun Yang has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - comment >> - Rework reference type initialization >> >> Signed-off-by: Albert Yang <albert.m.y...@oracle.com> > > src/hotspot/share/classfile/classFileParser.cpp line 6098: > >> 6096: >> 6097: return _super_klass->reference_type() != REF_NONE; >> 6098: } > > Stylistically, I'd prefer an if-ladder here. I might also swap the > reference-type check and the name check, so the for-bootstrapping name check > case being last (with a comment to that effect). These if-checks more or less mirror the type hierarchy, `Object -> Reference -> Soft|Weak... -> ...`. Then, walking down the type hierarchy using the early-return style here makes more sense to me. > src/hotspot/share/oops/instanceKlass.cpp line 497: > >> 495: _nest_host_index(0), >> 496: _init_state(allocated), >> 497: _reference_type(REF_NONE), > > This is initializing `_reference_type` to the wrong value for a > `InstanceRefKlass` object, which then needs to reset it in the derived > constructor. Why not get the reference type from the parser? The (currently > file-scoped static) determine_reference_type function in instanceRefKlass.cpp > doesn't have any dependency on the klass object being constructed, just the > parser. The current approach limits the knowledge of non-strong ref types to `instanceRefKlass` file. ------------- PR: https://git.openjdk.java.net/jdk/pull/8332
