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

Reply via email to