<div dir="ltr"><div class="gmail_default" style="font-family:arial,helvetica,sans-serif">OK, that's fair. But the sticking point to me is the documentation which says: "This call may be ignored if ClassFile.DebugElementsOption.DROP_DEBUG is set, or if any of the argument labels is not bound and ClassFile.DeadLabelsOption.DROP_DEAD_LABELS is set." Granted "may" is a wiggle word, but I think a reasonable reader would not interpret that as the methods taking a partial effect as they do currently; rather that it either is or is not ignored. So AFAICT *something* should be fixed, be it the code or the documentation.</div></div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Wed, Oct 1, 2025 at 8:18 AM Brian Goetz <<a href="mailto:brian.goetz@oracle.com">brian.goetz@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><u></u>
<div>
<font size="4" face="monospace">I'm going to object to the word
"fix", because I'm not convinced something is broken here. But we
can think about the implications of this change (and in the
meantime, you have an entirely suitable workaround.)</font><br>
<br>
<div>On 10/1/2025 9:12 AM, David Lloyd
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_default" style="font-family:arial,helvetica,sans-serif">Then perhaps
the cleaner fix is instead to have the default methods
delegate to
`LocalVariable.of(int,String,ClassDesc,Label,Label)` directly?</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Wed, Oct 1, 2025 at 7:52 AM
Brian Goetz <<a href="mailto:brian.goetz@oracle.com" target="_blank">brian.goetz@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div> <font size="4" face="monospace">After thinking about it
for a few more minutes, I'm less sure of this approach
(and not sure I'd even classify it as a bug.) While
locally this change is probably harmless, there are dozens
of uses of the idiom `constantPool().xxxEntry()` in this
class -- why would we switch just some of them over to
using the temp pool? And switching all of them over falls
into a more significant change in implementation approach,
for which the bar is surely higher. <br>
<br>
In any case, I think we don't need to change anything; the
code you want is already in the library. Just do<br>
<br>
builder.with(LocalVariable.of(slot, name, desc, start,
end))<br>
<br>
since that's exactly what LocalVariable::of does:<br>
<br>
</font>
<div style="background-color:rgb(255,255,255);color:rgb(8,8,8)">
<pre style="font-family:"JetBrains Mono",monospace;font-size:11.3pt"> <span style="color:rgb(0,51,179)">static </span><span style="color:rgb(0,0,0)">LocalVariable </span><span style="color:rgb(0,98,122)">of</span>(<span style="color:rgb(0,51,179)">int </span><span style="color:rgb(0,0,0)">slot</span>, <span style="color:rgb(0,0,0)">String name</span>, <span style="color:rgb(0,0,0)">ClassDesc descriptor</span>, <span style="color:rgb(0,0,0)">Label startScope</span>, <span style="color:rgb(0,0,0)">Label endScope</span>) {
<span style="color:rgb(0,51,179)">return </span><span style="font-style:italic">of</span>(<span style="color:rgb(0,0,0)">slot</span>,
<span style="color:rgb(0,0,0)">TemporaryConstantPool</span>.<span style="color:rgb(135,16,148);font-style:italic">INSTANCE</span>.utf8Entry(<span style="color:rgb(0,0,0)">name</span>),
<span style="color:rgb(0,0,0)">TemporaryConstantPool</span>.<span style="color:rgb(135,16,148);font-style:italic">INSTANCE</span>.utf8Entry(<span style="color:rgb(0,0,0)">descriptor</span>.descriptorString()),
<span style="color:rgb(0,0,0)">startScope</span>, <span style="color:rgb(0,0,0)">endScope</span>);
}
}
</pre>
</div>
<br>
<font size="4" face="monospace"><br>
<br>
<br>
<br>
<br>
</font><br>
<div>On 10/1/2025 8:31 AM, David Lloyd wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_default" style="font-family:arial,helvetica,sans-serif">OK,
shall I prepare a bug report and patch? Thanks.</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Tue, Sep 30, 2025
at 2:55 PM Chen Liang <<a href="mailto:chen.l.liang@oracle.com" target="_blank">chen.l.liang@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<div dir="ltr">
<div style="font-family:"Calibri Light","Helvetica Light",sans-serif;font-size:12pt;color:rgb(0,0,0)">
Hi David, this seems a legitimate bug.</div>
<div style="font-family:"Calibri Light","Helvetica Light",sans-serif;font-size:12pt;color:rgb(0,0,0)">
<br>
</div>
<div style="font-family:"Calibri Light","Helvetica Light",sans-serif;font-size:12pt;color:rgb(0,0,0)">
I think we can bypass this by using the
TemporaryConstantPool.INSTANCE to construct the
UTF-8 entries.</div>
<div style="font-family:"Calibri Light","Helvetica Light",sans-serif;font-size:12pt;color:rgb(0,0,0)">
<br>
</div>
<div style="font-family:"Calibri Light","Helvetica Light",sans-serif;font-size:12pt;color:rgb(0,0,0)">
Regards, Chen</div>
<hr style="display:inline-block;width:98%">
<div id="m_-6067371836316786644m_-3895197113871837706m_-3732124551141255202divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b>
classfile-api-dev <<a href="mailto:classfile-api-dev-retn@openjdk.org" target="_blank">classfile-api-dev-retn@openjdk.org</a>>
on behalf of David Lloyd <<a href="mailto:david.lloyd@redhat.com" target="_blank">david.lloyd@redhat.com</a>><br>
<b>Sent:</b> Tuesday, September 30, 2025 11:07
AM<br>
<b>To:</b> <a href="mailto:classfile-api-dev@openjdk.org" target="_blank">classfile-api-dev@openjdk.org</a>
<<a href="mailto:classfile-api-dev@openjdk.org" target="_blank">classfile-api-dev@openjdk.org</a>><br>
<b>Cc:</b> Ladislav Thon <<a href="mailto:lthon@redhat.com" target="_blank">lthon@redhat.com</a>><br>
<b>Subject:</b> DROP_DEBUG and the constant
pool</font>
<div> </div>
</div>
<div>
<div dir="ltr">
<div>
<div style="font-family:arial,helvetica,sans-serif">We've observed that when
using `DROP_DEBUG` in conjunction with
`CodeBuilder#localVariable` and/or
`localVariableType`, some (otherwise
useless) constant pool entries are still
being created (which contain, I believe,
both the variable name and descriptor).
This was observed using a backport of the
JDK classfile API based on JDK 25.</div>
<br clear="all">
</div>
<div>
<div style="font-family:arial,helvetica,sans-serif">Would this be expected
behavior? Is there a separate step needed
to clean the constant pool for cases like
this?</div>
</div>
<div style="font-family:arial,helvetica,sans-serif"><br>
</div>
<div style="font-family:arial,helvetica,sans-serif">It looks to me to be the
consequence of how the default methods for
local variable creation are implemented,
e.g.:</div>
<div style="font-family:arial,helvetica,sans-serif"><br>
</div>
<div style="font-family:arial,helvetica,sans-serif"> default CodeBuilder
localVariable(int slot, String name,
ClassDesc descriptor, Label startScope,
Label endScope) {<br>
return localVariable(slot,<br>
constantPool().utf8Entry(name),<br>
constantPool().utf8Entry(descriptor),<br>
startScope,
endScope);<br>
}<br>
</div>
<div><br>
</div>
<div>
<div style="font-family:arial,helvetica,sans-serif">The constant pool is
accessed even when `DROP_DEBUG` is
enabled, because that flag is used later
on in the process, and it seems that these
entries are never dropped, even if they
are unused.</div>
<br>
</div>
<span>-- </span><br>
<div dir="ltr">
<div dir="ltr">- DML • he/him<br>
</div>
</div>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<div><br clear="all">
</div>
<div><br>
</div>
<span class="gmail_signature_prefix">-- </span><br>
<div dir="ltr" class="gmail_signature">
<div dir="ltr">- DML • he/him<br>
</div>
</div>
</blockquote>
<br>
</div>
</blockquote>
</div>
<div><br clear="all">
</div>
<div><br>
</div>
<span class="gmail_signature_prefix">-- </span><br>
<div dir="ltr" class="gmail_signature">
<div dir="ltr">- DML • he/him<br>
</div>
</div>
</blockquote>
<br>
</div>
</blockquote></div><div><br clear="all"></div><div><br></div><span class="gmail_signature_prefix">-- </span><br><div dir="ltr" class="gmail_signature"><div dir="ltr">- DML • he/him<br></div></div>