On Tue, 3 Feb 2026 12:02:41 GMT, Dan Heidinga <[email protected]> wrote:
>> Paul Hübner has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 13 additional >> commits since the last revision: >> >> - Merge branch 'lworld' of https://github.com/openjdk/valhalla into >> JDK-8372954 >> - Missed obsolete test >> - Years >> - Merge branch 'lworld' of https://github.com/openjdk/valhalla into >> JDK-8372954 >> - substitutability contract changed >> - Legacy handling in ProcessHandleImpl >> - typo >> - Test flags >> - Dead stuff >> - Merge branch 'JDK-8372954' of github.com:Arraying/valhalla into >> JDK-8372954 >> - ... and 3 more: >> https://git.openjdk.org/valhalla/compare/e31f9f53...29344379 > > src/hotspot/share/classfile/classFileParser.cpp line 1399: > >> 1397: // one for the field the JVM injects when detecting an empty inline >> class >> 1398: const int total_fields = length + num_injected + (is_inline_type ? 2 >> : 0) >> 1399: + (is_value_class ? 1 : 0); > > Can you update the comment block above to include something like: > > // all value classes, even abstract ones, get an additional slot for the > acmp_maps field used by the substitutability check > > > We need something to make it clear why we're testing both inline type and > value class here and what the new field is Could you update the comment above, replacing the mention to the pre-allocated default value with the a mention to the null-reset value? This is not related to the removal of the old substitutability method, but it's an easy cleanup to do. Thanks. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2012#discussion_r2759263758
