<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
        {font-family:Aptos;
        panose-1:2 11 0 4 2 2 2 2 2 4;}
@font-face
        {font-family:Consolas;
        panose-1:2 11 6 9 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0cm;
        font-size:11.0pt;
        font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:blue;
        text-decoration:underline;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;
        mso-ligatures:none;}
@page WordSection1
        {size:612.0pt 792.0pt;
        margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
        {page:WordSection1;}
/* List Definitions */
@list l0
        {mso-list-id:1614751998;
        mso-list-template-ids:-1945834490;}
@list l0:level1
        {mso-level-number-format:bullet;
        mso-level-text:;
        mso-level-tab-stop:36.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level2
        {mso-level-number-format:bullet;
        mso-level-text:;
        mso-level-tab-stop:72.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level3
        {mso-level-number-format:bullet;
        mso-level-text:;
        mso-level-tab-stop:108.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level4
        {mso-level-number-format:bullet;
        mso-level-text:;
        mso-level-tab-stop:144.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level5
        {mso-level-number-format:bullet;
        mso-level-text:;
        mso-level-tab-stop:180.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level6
        {mso-level-number-format:bullet;
        mso-level-text:;
        mso-level-tab-stop:216.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level7
        {mso-level-number-format:bullet;
        mso-level-text:;
        mso-level-tab-stop:252.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level8
        {mso-level-number-format:bullet;
        mso-level-text:;
        mso-level-tab-stop:288.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
@list l0:level9
        {mso-level-number-format:bullet;
        mso-level-text:;
        mso-level-tab-stop:324.0pt;
        mso-level-number-position:left;
        text-indent:-18.0pt;
        mso-ansi-font-size:10.0pt;
        font-family:Symbol;}
ol
        {margin-bottom:0cm;}
ul
        {margin-bottom:0cm;}
--></style>
</head>
<body lang="en-CZ" link="blue" vlink="purple" style="word-wrap:break-word">
<div class="WordSection1">
<div>
<p class="MsoNormal"><span lang="EN-US">Hi,</span></p>
<p class="MsoNormal"><span lang="EN-US">I’ve updated the <a href="https://github.com/openjdk/jdk/pull/17306">
PR</a> draft with removal of </span><span lang="EN-US" style="font-size:12.0pt;font-family:Consolas">operator</span><span lang="EN-US"> method as obsolete and</span></p>
<p class="MsoNormal"><span style="font-size:12.0pt;font-family:Consolas">newPrimitiveArray</span>,
<span style="font-size:12.0pt;font-family:Consolas">newReferenceArray</span><span lang="EN-US"> and
</span><span style="font-size:12.0pt;font-family:Consolas">newMultidimensionalArray</span><span lang="EN-US"> methods as duplicate.</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">I’ve also extended </span><span lang="EN-US" style="font-size:12.0pt;font-family:Consolas">conversion</span><span lang="EN-US"> method to support the whole matrix (including multi-step conversions, no-op conversions and
</span><span lang="EN-US" style="font-size:12.0pt;font-family:Consolas">BooleanType</span><span lang="EN-US"> conversions), see
<a href="https://github.com/openjdk/jdk/blob/027374004c9c9bf53a709c98593363043e8b7782/src/java.base/share/classes/java/lang/classfile/CodeBuilder.java#L548">
CodeBuilder.java#L548</a></span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">Regarding the </span><span lang="EN-US" style="font-size:12.0pt;font-family:Consolas">allocateLocal</span><span lang="EN-US"> I would slightly prefer to keep it, however
</span><span style="font-size:12.0pt;font-family:Consolas">allocateSlot</span> or
<span style="font-size:12.0pt;font-family:Consolas">allocateLocalSlot</span> or <span style="font-size:12.0pt;font-family:Consolas">
reserveLocalSlot</span><span lang="EN-US"> are also fine.</span></p>
<p class="MsoNormal"><span lang="EN-US">Prefix </span><span lang="EN-US" style="font-size:12.0pt;font-family:Consolas">new</span><span lang="EN-US"> would be confusing.</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US">Thanks,</span></p>
<p class="MsoNormal"><span lang="EN-US">Adam</span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"><span lang="EN-US"> </span></p>
<p class="MsoNormal"> </p>
<div id="mail-editor-reference-message-container">
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal" style="margin-bottom:12.0pt"><b><span style="font-size:12.0pt;font-family:"Aptos",sans-serif;color:black">From:
</span></b><span style="font-size:12.0pt;font-family:"Aptos",sans-serif;color:black">Brian Goetz <brian.goetz@oracle.com><br>
<b>Date: </b>Friday, 5 January 2024 at 17:33<br>
<b>To: </b>liangchenblue@gmail.com <liangchenblue@gmail.com>, classfile-api-dev <classfile-api-dev@openjdk.org><br>
<b>Cc: </b>Adam Sotona <adam.sotona@oracle.com><br>
<b>Subject: </b>Re: Revisit j.l.classfile.CodeBuilder API surface</span></p>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span style="font-size:13.5pt;font-family:"Courier New"">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. 
</span></p>
<div>
<p class="MsoNormal">On 1/5/2024 11:21 AM, - wrote:</p>
</div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">I love this simplification too! </p>
<div>
<p class="MsoNormal"> </p>
<div>
<p class="MsoNormal">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.</p>
</div>
<div>
<p class="MsoNormal"> </p>
</div>
<div>
<p class="MsoNormal">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.</p>
</div>
<div>
<p class="MsoNormal"> </p>
</div>
<div>
<p class="MsoNormal">In contrast, the more specific methods in CodeBuilder avoids these pitfalls.</p>
</div>
<div>
<p class="MsoNormal"> </p>
</div>
<div>
<p class="MsoNormal">After all, my rant above is not against this change, but instead are improvements that are enabled by this simplification.</p>
</div>
<div>
<p class="MsoNormal"> </p>
</div>
<div>
<p class="MsoNormal">Cheers,</p>
</div>
</div>
<div>
<p class="MsoNormal">Chen Liang</p>
</div>
</div>
<p class="MsoNormal"> </p>
<div>
<div>
<p class="MsoNormal">On Fri, Jan 5, 2024 at 9:52 AM Brian Goetz <<a href="mailto:brian.goetz@oracle.com" target="_blank">brian.goetz@oracle.com</a>> wrote:</p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0cm 0cm 0cm 6.0pt;margin-left:4.8pt;margin-top:5.0pt;margin-right:0cm;margin-bottom:5.0pt">
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span style="font-size:13.5pt;font-family:"Courier New"">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?</span></p>
</div>
</blockquote>
</div>
</blockquote>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</div>
</div>
</body>
</html>