On Tue, 9 May 2023 11:59:08 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org> wrote:
>> Adam Sotona 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 16 additional >> commits since the last revision: >> >> - fixed StackCounter >> - Merge branch 'master' into JDK-8305990-debug-info-strip-fail >> - implemented failover stackmap generation for class version 50 >> fixed StackMapGenerator error debug print + added clone constructor to >> SplitConstantPool >> adjusted and extended related tests >> - Apply suggestions from code review >> >> Thanks for review. >> >> Co-authored-by: liach <7806504+li...@users.noreply.github.com> >> - Merge branch 'master' into JDK-8305990-debug-info-strip-fail >> - Update StackCounter.java >> - added comments to StackCounter about maxStack upper bounds calculation >> for JSR/RET instructions >> - fixed stack counting of JSR instructions >> - implemented StackCounter >> - Making some BufWriter fields final >> - ... and 6 more: https://git.openjdk.org/jdk/compare/1302ccc8...5db0ed01 > > Seems like GitHub UI does not let me add comments (as GitHub seems to be > experiencing some issues). Here's what I added: > > * StackCounter: `stack` and `local` are different in spirit. One adds new > stack slots. The other ensure there's at least enough local slots available. > As such I'd suggest a renaming `addStackSlot` and `ensureLocalSlot`. > > * StackMapGeneration/DirectClassBuilder: the exception type seems a bit loose > and we might end up catching more than is thrown by the stackmap generator. @mcimadamore thanks for review. I've fixed `stack` and `local`. Exception consolidation is already a part of #11411 and this catch will be changed to `IAE` only. Actually it would have to catch `ISE`, `IAE`, `IOOBE` and probably also `NPE`. ------------- PR Comment: https://git.openjdk.org/jdk/pull/13478#issuecomment-1540021763
