<!DOCTYPE html><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<font size="4" face="monospace">One problem with `newLocalSlot` is
that it now conflicts with the new instructions like `newObject`,
and thus makes it less clear whether the method is _generating_
bytecode or merely setting up for a future operation. But perhaps
`allocateSlot` or `allocateLocalSlot` or `reserveLocalSlot` is
more clear. </font><br>
<br>
<div class="moz-cite-prefix">On 1/5/2024 11:21 AM, - wrote:<br>
</div>
<blockquote type="cite" cite="mid:CABe8uE0JsAS7fv6wDBmgfYz5dAQFExO8DhMU_rW2pH97LiS=2A@mail.gmail.com">
<div dir="ltr">I love this simplification too!
<div><br>
<div>One nitpick is that now we have `loadLocal` and
`storeLocal`, maybe we should rename existing
`allocateLocal` to something like `newLocalSlot` ("allocate"
is disliked by panama devs too), so it's clear it returns a
slot index like `parameterSlot` or `receiverSlot` do.</div>
<div><br>
</div>
<div>In addition, the previous "instruction family" APIs have
their downfalls too: some families are not complete, and may
throw exceptions upon combination of certain arguments but
are not well documented. For example, a
`conversion(TypeKind.LongType, TypeKind.ByteType)` would
fail, because bytecode actually uses 2 instructions: l2i and
i2b. (maybe upgrading this `conversion` to emit multiple
bytecode is a solution, too) Same for calling
`newPrimitiveArray(TypeKind.VoidType)` etc. Similarly, the
`operator(Opcode)` is just a watered-down version of ASM's
`visitInsn`, yet is more risky as you can pass in other
no-operand opcodes that fail at runtime without compile-time
hints.</div>
<div><br>
</div>
<div>In contrast, the more specific methods in CodeBuilder
avoids these pitfalls.</div>
<div><br>
</div>
<div>After all, my rant above is not against this change, but
instead are improvements that are enabled by this
simplification.</div>
<div><br>
</div>
<div>Cheers,</div>
</div>
<div>Chen Liang</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Fri, Jan 5, 2024 at 9:52 AM
Brian Goetz <<a href="mailto:brian.goetz@oracle.com" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">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">Let me fill in some
history here. <br>
<br>
Much of the design process for this library could be
described as an iterated game of "find the primitive."
The first version of the API had a zillion ad-hoc methods,
with little structural relation. At some point we hit
upon a model-driven analysis, and broke down instructions
into families (loads, stores, etc), and at that point, we
thought methods like loadInstruction were the primitives.
But we hadn't actually hit bottom yet; after many rounds
of refactoring, it became clear that there was one
primitive for the builder hierarchy, now called `with`,
and all of the builder methods now bottom out at that. <br>
<br>
At the same time, we added a number of "convenience"
methods such as aload_0() and aload(int n) (which now just
delegate to with() or loadInstruction()), and over time,
we found that most generative use cases used these methods
much more than the primitives (which is fine.) <br>
<br>
At this point, I think many of the xxxInstruction (and its
rigid naming convention) are vestiges of a previous
attempt to organize the API. Some still have uses, though
should be renamed to reflect that they are mere
conveniences (such as arrayLoadInstruction to arrayLoad).
<br>
<br>
(Does anyone use operator(opcode), or can we drop that one
too?)<br>
<br>
So I support this simplification. <br>
<br>
Having done the refactor, were there any surprises in the
usages of various CodeBuilder methods?<br>
<br>
</font><br>
<div>On 1/5/2024 10:38 AM, Adam Sotona wrote:<br>
</div>
<blockquote type="cite">
<div>
<p class="MsoNormal"><span lang="EN-US">Hi,</span></p>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white">j.l.classfile.CodeBuilder
API consist of </span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US">more than </span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white">230
methods.</span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US"> </span></p>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white">Existing
ClassFile API use cases proved the concept </span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US">of </span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white">one
big CodeBuilder is comfortable. However there are
some redundancies, glitches in the naming, some
frequently used methods are hard to find and some
methods have low practical use.</span></p>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white"> </span></p>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US">Majority</span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white">
of the methods may be </span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US">divided</span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white">
into three main </span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US">levels</span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white">:</span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77)"><br>
</span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US">1.</span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white">
methods building low level bytecode instructions
according to JVMS chapter 6.5 (aaload, aastore,
aconst_null...)</span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77)"><br>
</span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US">2.</span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white">
methods reconstructing individual subtypes of
j.l.classfile.Instruction from given arguments
(loadInstruction, storeInstruction,
incrementInstruction, branchInstruction...)</span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77)"><br>
</span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US">3.</span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white">
methods modeling high level code blocks (block,
ifThen, ifThenElse, trying...)</span></p>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white"> </span></p>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US">Many methods from level 2 (with suffix
`Instruction`) seem to be obsolete or misplaced.
Some of them are duplicates of methods from level 1,
some are obsolete and some are very useful, however
a bit hidden. </span> <span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77)" lang="EN-US">The API should be cleaned a bit while
in preview.<span style="background:white"></span></span></p>
<p style="margin-right:0cm;margin-bottom:0cm;margin-left:0cm"> <span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US">I would like to open a discussion on
the following proposed </span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77)">changes
in </span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77)" lang="EN-US">the </span><span style="font-size:10pt;font-family:"Roboto Mono";color:rgb(23,43,77)">CodeBuilder</span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77)"> methods</span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77)" lang="EN-US">:</span></p>
<ul type="disc">
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">incrementInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> remove as
duplicate of </span><span style="font-size:10pt;font-family:"Roboto Mono"">iinc</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">lookupSwitchInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> remove as
duplicate of </span><span style="font-size:10pt;font-family:"Roboto Mono"">lookupswitch</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">tableSwitchInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> remove as
duplicate of </span><span style="font-size:10pt;font-family:"Roboto Mono"">tableswitch</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">throwInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> remove as
duplicate of </span><span style="font-size:10pt;font-family:"Roboto Mono"">athrow</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">invokeDynamicInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> remove as
duplicate of </span><span style="font-size:10pt;font-family:"Roboto Mono"">invokedynamic</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">stackInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> remove as
obsolete with suggested replacements: </span><span style="font-size:10pt;font-family:"Roboto Mono"">with(StackInstruction.of(...))</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">monitorInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> remove as
obsolete with suggested replacements: </span><span style="font-size:10pt;font-family:"Roboto Mono"">monitorenter</span><span style="font-size:10.5pt;font-family:Arial,sans-serif">, </span><span style="font-size:10pt;font-family:"Roboto Mono"">monitorexit</span><span style="font-size:10.5pt;font-family:Arial,sans-serif">, or </span><span style="font-size:10pt;font-family:"Roboto Mono"">with(MonitorInstruction.of(...))</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">nopInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> remove as
duplicate of </span><span style="font-size:10pt;font-family:"Roboto Mono"">nop</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">typecheckInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> remove as
obsolete with suggested replacements: </span><span style="font-size:10pt;font-family:"Roboto Mono"">checkcast</span><span style="font-size:10.5pt;font-family:Arial,sans-serif">, </span><span style="font-size:10pt;font-family:"Roboto Mono"">instanceOf</span><span style="font-size:10.5pt;font-family:Arial,sans-serif">, or </span><span style="font-size:10pt;font-family:"Roboto Mono"">with(TypeCheckInstruction.of(...))</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">loadInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">loadLocal</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">storeInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">storeLocal</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">branchInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">branch</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">invokeInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">invoke</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">newObjectInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">newObject</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">newPrimitiveArrayInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">newPrimitiveArray</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">newReferenceArrayInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">newReferenceArray</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">newMultidimensionalArrayInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">newMultidimensionalArray</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">arrayLoadInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">arrayLoad</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">arrayStoreInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">arrayStore</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">convertInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">conversion</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">operatorInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">operator</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">constantInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">loadConstant</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">fieldInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">fieldAccess</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">instanceof_</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">instanceOf</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
<li class="MsoNormal" style="color:rgb(23,43,77)"> <span style="font-size:10pt;font-family:"Roboto Mono"">returnInstruction</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"> rename to </span><span style="font-size:10pt;font-family:"Roboto Mono"">return_</span><span style="font-size:10.5pt;font-family:Arial,sans-serif"></span></li>
</ul>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US">Here is the related RFE: </span><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:black;background:white" lang="EN-US"><a href="https://bugs.openjdk.org/browse/JDK-8323058" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">https://bugs.openjdk.org/browse/JDK-8323058</a></span><span style="font-size:10.5pt;font-family:Arial,sans-serif;background:white" lang="EN-US"></span></p>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:black;background:white" lang="EN-US">Draft of the CSR (no spec yet): <a href="https://bugs.openjdk.org/browse/JDK-8323067" target="_blank" moz-do-not-send="true" class="moz-txt-link-freetext">https://bugs.openjdk.org/browse/JDK-8323067</a></span><span style="font-size:10.5pt;font-family:Arial,sans-serif;background:white" lang="EN-US"></span></p>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US">And draft of the Pull Request: <a href="https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/17282__;!!ACWV5N9M2RV99hQ!K4zh9o6NcHVlQHameNjhwvRGo_HnBCYhzTH9PQDru-SZu5QkkI1NPfSZHyiCN_7l2PlyEU41N_Fp-LCMfbzTFjDa$" target="_blank" moz-do-not-send="true">https://github.com/openjdk/jdk/pull/17282</a></span><span style="font-size:10.5pt;font-family:Arial,sans-serif;background:white" lang="EN-US"></span></p>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:Arial,sans-serif;background:white" lang="EN-US"> </span></p>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US">Any comments are welcome.</span></p>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US"> </span></p>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US">Thank you,</span></p>
<p class="MsoNormal"><span style="font-size:10.5pt;font-family:Arial,sans-serif;color:rgb(23,43,77);background:white" lang="EN-US">Adam</span></p>
</div>
</blockquote>
<br>
</div>
</blockquote>
</div>
</blockquote>
<br>
</body>
</html>