<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body>
    <font size="4"><font face="monospace">Some review comments on your
        patch:<br>
        <br>
         - I'd prefer for all of the static methods in Classfile to go
        away, though I can imagine choosing to leave the most basic
        version (no options) of build/parse around as conveniences.  If
        we leave these, the Javadoc should make it clear that these are
        equivalent to Context.of().parse(...).  <br>
         - I am OK with calling the context class "Classfile" for the
        time being; this makes the diff easier to follow as it makes it
        clear that the static methods are being promoted to instance
        methods, and we can discuss name later.  <br>
         - I see you cache the Options in
        BufWriterImpl/BufferedCodeBuilder/etc, but we should probably be
        caching the full context (impl) object instead, and fetching
        options out of the context.  <br>
         - (Syntax nit) Some enum options are fully descriptive of what
        they are (DROP_DEBUG_ELEMENTS), and some are implicit
        (ALWAYS_GENERATE -- generate what?).  I think I would prefer to
        err on the side of fully descriptive (and people can use static
        imports to drop the qualifier if they like), but we should be
        consistent.  There should be a convention of what the default is
        (like "first is always default") for options that are enums.  <br>
        <br>
      </font></font><br>
    <div class="moz-cite-prefix">On 5/26/2023 11:18 AM, Adam Sotona
      wrote:<br>
    </div>
    <blockquote type="cite" cite="mid:CY4PR1001MB2150855058CA47CCA8532FE58C479@CY4PR1001MB2150.namprd10.prod.outlook.com">
      
      <meta name="Generator" content="Microsoft Word 15 (filtered
        medium)">
      <style>@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;}p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0cm;
        font-size:10.0pt;
        font-family:"Calibri",sans-serif;}a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:#0563C1;
        text-decoration:underline;}span.EmailStyle19
        {mso-style-type:personal-reply;
        font-family:"Calibri",sans-serif;
        color:windowtext;}.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;
        mso-ligatures:none;}div.WordSection1
        {page:WordSection1;}</style>
      <div class="WordSection1">
        <p class="MsoNormal"><span style="font-size:11.0pt;mso-fareast-language:EN-US" lang="EN-US">This is a raw prototype of Classfile.Context
            implementation and the new options:<o:p></o:p></span></p>
        <p class="MsoNormal"><span style="font-size:11.0pt;mso-fareast-language:EN-US" lang="EN-US"><a href="https://github.com/openjdk/jdk/pull/14180/files" moz-do-not-send="true" class="moz-txt-link-freetext">https://github.com/openjdk/jdk/pull/14180/files</a><o:p></o:p></span></p>
        <p class="MsoNormal"><span style="font-size:11.0pt;mso-fareast-language:EN-US" lang="EN-US"><a href="https://github.com/openjdk/jdk/blob/4456ca78bc67138930dc313f71aba58ef1cf5f13/src/java.base/share/classes/jdk/internal/classfile/Classfile.java" moz-do-not-send="true" class="moz-txt-link-freetext">https://github.com/openjdk/jdk/blob/4456ca78bc67138930dc313f71aba58ef1cf5f13/src/java.base/share/classes/jdk/internal/classfile/Classfile.java</a><o:p></o:p></span></p>
        <p class="MsoNormal"><span style="font-size:11.0pt;mso-fareast-language:EN-US" lang="EN-US"><o:p> </o:p></span></p>
        <p class="MsoNormal"><span style="font-size:11.0pt;mso-fareast-language:EN-US" lang="EN-US">It is far from finished, however before going
            further I would appreciate your comments (mainly on options
            naming).<o:p></o:p></span></p>
        <p class="MsoNormal"><span style="font-size:11.0pt;mso-fareast-language:EN-US" lang="EN-US"><o:p> </o:p></span></p>
        <p class="MsoNormal"><span style="font-size:11.0pt;mso-fareast-language:EN-US" lang="EN-US">Thanks,<o:p></o:p></span></p>
        <p class="MsoNormal"><span style="font-size:11.0pt;mso-fareast-language:EN-US" lang="EN-US">Adam<o:p></o:p></span></p>
        <p class="MsoNormal"><span style="font-size:11.0pt;mso-fareast-language:EN-US" lang="EN-US"><o:p> </o:p></span></p>
        <p class="MsoNormal"><span style="font-size:11.0pt;mso-fareast-language:EN-US"><o:p> </o:p></span></p>
        <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;color:black">From:
              </span></b><span style="font-size:12.0pt;color:black">Brian
              Goetz <a class="moz-txt-link-rfc2396E" href="mailto:brian.goetz@oracle.com"><brian.goetz@oracle.com></a><br>
              <b>Date: </b>Thursday, 25 May 2023 20:31<br>
              <b>To: </b>Adam Sotona <a class="moz-txt-link-rfc2396E" href="mailto:adam.sotona@oracle.com"><adam.sotona@oracle.com></a>,
              <a class="moz-txt-link-abbreviated" href="mailto:liangchenblue@gmail.com">liangchenblue@gmail.com</a> <a class="moz-txt-link-rfc2396E" href="mailto:liangchenblue@gmail.com"><liangchenblue@gmail.com></a>,
              classfile-api-dev <a class="moz-txt-link-rfc2396E" href="mailto:classfile-api-dev@openjdk.org"><classfile-api-dev@openjdk.org></a><br>
              <b>Subject: </b>Re: Planned features of a classfile
              context object<o:p></o:p></span></p>
        </div>
        <p class="MsoNormal" style="margin-bottom:12.0pt"><span style="font-size:13.5pt;font-family:"Courier
            New",serif">A model created within a context can retain
            a reference to its creating context.  (We do a lot of this
            sort of thing already, where MethodModel retains a reference
            to ClassModel, and where CPBuilder retains a reference to
            Options.)  So for operations that require a context, a model
            derived from a classfile can use the context that created
            it.<br>
            <br>
            Methods like parse/build/transform will be moved to the
            classfile context object. 
          </span><span style="font-size:11.0pt"><o:p></o:p></span></p>
        <div>
          <p class="MsoNormal"><span style="font-size:11.0pt">On
              5/25/2023 2:02 PM, Adam Sotona wrote:<o:p></o:p></span></p>
        </div>
        <blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
          <p class="MsoNormal"><span style="font-size:11.0pt;mso-fareast-language:EN-US" lang="EN-US">I’m trying to figure out how the context will
              be passed down to the models (if it suppose to be detached
              from the models).</span><o:p></o:p></p>
          <p class="MsoNormal"><span style="font-size:11.0pt" lang="EN-US">Do we plan to enforce another CC argument to
              all models methods requiring context?</span><o:p></o:p></p>
          <p class="MsoNormal"><span style="font-size:11.0pt" lang="EN-US">Or do we plan to strip down all
              context-sensitive methods from all models and attach them
              to a kind of context-wrappers?</span><o:p></o:p></p>
          <p class="MsoNormal"><span style="font-size:11.0pt" lang="EN-US">For example
              codeModel.forEach(Classfile.Context.of(…), …) or
              Classfile.Context.of(…).forEach(codeModel, …) ?</span><o:p></o:p></p>
          <p class="MsoNormal"><span style="font-size:11.0pt" lang="EN-US"> </span><o:p></o:p></p>
          <p class="MsoNormal"><span style="font-size:11.0pt" lang="EN-US">Thanks,</span><o:p></o:p></p>
          <p class="MsoNormal"><span style="font-size:11.0pt" lang="EN-US">Adam</span><o:p></o:p></p>
        </blockquote>
        <p class="MsoNormal"><span style="font-size:11.0pt"><o:p> </o:p></span></p>
      </div>
    </blockquote>
    <br>
  </body>
</html>