On Fri, 26 Jul 2024 13:17:11 GMT, Chen Liang <li...@openjdk.org> wrote:
>> src/java.base/share/classes/jdk/internal/classfile/impl/BufferedCodeBuilder.java >> line 67: >> >>> 65: this.maxLocals = Util.maxLocals(methodInfo.methodFlags(), >>> methodInfo.methodTypeSymbol()); >>> 66: if (original != null) >>> 67: this.maxLocals = Math.max(this.maxLocals, >>> original.maxLocals()); >> >> `original::maxLocals`set the counter for `CodeBuilder::allocateLocal` >> By restricting calculation of maxLocals to "origin instanceof CodeAttribute" >> may cause invalid locals allocations for chained code builders. The number >> might not be exposed in the API, however we need to know it internally. > > I think we should just do `Util.maxLocals` without this `origin` check. Your > said problem can happen too if a DirectMethodBuilder transforms a > BufferedCodeBuilder.Model. > > Note that any code builder can receive a code model half way with > `cob.transform(anyModel, CodeTransform.ACCEPT_ALL)`, which the buffered code > builder initialization also uses. > I think we should just ask users to use `CodeLocalsShifter` in that case, > especially if the transform is inserting new locals. We cannot buffer > existing chained builders for performance reasons. Updated this to internal API that accesses the existing local variable management on BufferedCodeBuilder. We can discuss this consistency problem later. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/20338#discussion_r1694050104