On Mon, 2 Feb 2026 23:03:40 GMT, Paul Hübner <[email protected]> wrote:
>> Hi all, >> >> This change removes the old substitutability test method. The new/alt >> substitutability test takes the place of the legacy test, and all >> identifiers with "alt" are removed. >> >> Testing: tiers 1-5 on Linux (x64, AArch64), macOS (x64, AArch64), Windows >> (x64). > > 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/f3a1a179...29344379 Apart from adding the comment, this looks good to me 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 ------------- PR Review: https://git.openjdk.org/valhalla/pull/2012#pullrequestreview-3744873855 PR Review Comment: https://git.openjdk.org/valhalla/pull/2012#discussion_r2758735750
