<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    Hi Maurizio,<br>
    <br>
    I have uploaded another iteration [1], some comments inlined below.<br>
    <br>
    <div class="moz-cite-prefix">On 10/27/19 6:33 PM, Maurizio
      Cimadamore wrote:<br>
    </div>
    <blockquote type="cite"
      cite="mid:415e23ea-8414-5964-3d5a-66a7e33cb8e8@oracle.com">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <p>Also some comments on error messages (some feedback from
        Alex/Jon would also be appreciated here; error messages are also
        crucial to convey important bits of the programming model, so I
        think we should try to get them right):</p>
      <pre><span class="new">+compiler.err.record.cant.declare.duplicate.fields=\</span>
<span class="new">+    records cannot declare components with the same name</span>
<span class="new">+</span></pre>
      <div class="moz-cite-prefix"><br>
      </div>
      <div class="moz-cite-prefix">I think this should be less record
        specific - it's just a duplicate declaration - so we should
        issue same error as with method/field duplicate decl?</div>
    </blockquote>
    <br>
    not sure what to do here, there are several error messages about
    duplicated elements that are also pretty specific, see for modules,
    annotations etc.<br>
    <blockquote type="cite"
      cite="mid:415e23ea-8414-5964-3d5a-66a7e33cb8e8@oracle.com">
      <div class="moz-cite-prefix"><br>
        <pre><span class="new">compiler.err.record.cant.declare.field.modifiers=\</span>
<span class="new">+    record components can not have modifiers


</span></pre>
      </div>
      <div class="moz-cite-prefix">"can not" -> "cannot"</div>
    </blockquote>
    <br>
    done<br>
    <br>
    <blockquote type="cite"
      cite="mid:415e23ea-8414-5964-3d5a-66a7e33cb8e8@oracle.com">
      <div class="moz-cite-prefix"><br>
      </div>
      <div class="moz-cite-prefix">
        <pre><span class="new">+compiler.err.record.fields.must.be.in.header=\</span>
<span class="new">+    instance fields in a record must be declared in the header


</span></pre>
      </div>
      <div class="moz-cite-prefix">Should it say that fields should be
        in the header, or should it say that a record cannot declare
        fields?</div>
    </blockquote>
    <br>
    yep fixed<br>
    <br>
    <blockquote type="cite"
      cite="mid:415e23ea-8414-5964-3d5a-66a7e33cb8e8@oracle.com">
      <div class="moz-cite-prefix"><br>
        Invalid field declaration in record</div>
      <div class="moz-cite-prefix">(consider replacing field with record
        component)</div>
      <div class="moz-cite-prefix"><br>
      </div>
      <div class="moz-cite-prefix">Or something like that.</div>
      <div class="moz-cite-prefix"><br>
      </div>
      <div class="moz-cite-prefix">
        <pre><span class="new">+compiler.err.canonical.constructor.must.be.public=\</span>
<span class="new">+    canonical constructor must be public

</span><span class="new"></span>
</pre>
        I think I'd prefer to qualify "canonical _record_ constructor"<br>
      </div>
    </blockquote>
    <br>
    done<br>
    <br>
    <blockquote type="cite"
      cite="mid:415e23ea-8414-5964-3d5a-66a7e33cb8e8@oracle.com">
      <div class="moz-cite-prefix">
        <pre><span class="new"></span><span class="new"></span> <span class="new">+</span>
<span class="new">+compiler.err.canonical.with.name.mismatch=\</span>
<span class="new">+    constructor with same signature as canonical does not match by parameter names</span></pre>
      </div>
      <div class="moz-cite-prefix">This is a tricky one. Perhaps let's
        try to simplify:</div>
      <div class="moz-cite-prefix"><br>
      </div>
      <div class="moz-cite-prefix">"Invalid parameter names in record
        constructor"<br>
        (parameter names must match the names of the declared record
        components)<br>
      </div>
    </blockquote>
    <br>
    done<br>
    <br>
    <blockquote type="cite"
      cite="mid:415e23ea-8414-5964-3d5a-66a7e33cb8e8@oracle.com">
      <div class="moz-cite-prefix"> </div>
      <div class="moz-cite-prefix"><br>
      </div>
      <div class="moz-cite-prefix">Or something similar.</div>
      <div class="moz-cite-prefix"><br>
      </div>
      <div class="moz-cite-prefix">
        <pre><span class="new">+compiler.err.accessor.return.type.doesnt.match=\</span>
<span class="new">+    type returned by the accessor is not the same as the type of the corresponding record component\n\</span>
<span class="new">+    required: {0}\n\</span>
<span class="new">+    found:    {1}\n\</span>
<span class="new">+

</span></pre>
      </div>
      <p>Again, maybe this can be simpler:</p>
      <p>"Unexpected type in record accessor<span class="new"><br>
          + required: {0}\n\</span><span class="new"><br>
          + found: {1}\n\<br>
          "<br>
        </span></p>
    </blockquote>
    <br>
    yep agreed<br>
    <br>
    <blockquote type="cite"
      cite="mid:415e23ea-8414-5964-3d5a-66a7e33cb8e8@oracle.com">
      <p><span class="new"> </span></p>
      <pre><span class="new">+compiler.err.illegal.record.component.name=\</span>
<span class="new">+    record {0}, declares an illegal record component name: {1}</span></pre>
      <p><br>
        Better to flip the sentence around?</p>
      <p>Illegal record component name {1}<br>
        (record components cannot have same name as Object methods, ...)</p>
      <p>Maybe the first line could be enough, but if we could list the
        names that are 'reserved' that could be good too, assuming we
        can find a nice way to do that)\\</p>
    </blockquote>
    <br>
    I would say lets let only the first part: "Illegal record component
    name {1}" because there are other serialization related methods and
    fields that cant be used as component names. Listing all the
    possible sources would be muddy business I think.<br>
    <br>
    <blockquote type="cite"
      cite="mid:415e23ea-8414-5964-3d5a-66a7e33cb8e8@oracle.com">
      <pre><span class="new">+compiler.err.invalid.supertype.record=\</span>
<span class="new">+    no class can explicitly extend java.lang.Record</span></pre>
      <p>-> "Cannot extend j.l.Record"</p>
    </blockquote>
    done<br>
    <blockquote type="cite"
      cite="mid:415e23ea-8414-5964-3d5a-66a7e33cb8e8@oracle.com">
      <p><br>
      </p>
      <pre><span class="new">+compiler.err.canonical.cant.have.return.statement=\</span>
<span class="new">+    canonical constructor can not have return statements</span></pre>
      <p>"can not" -> "cannot"</p>
    </blockquote>
    fixed<br>
    <blockquote type="cite"
      cite="mid:415e23ea-8414-5964-3d5a-66a7e33cb8e8@oracle.com">
      <p><br>
      </p>
      <p>Maurizio<br>
      </p>
    </blockquote>
    <br>
    Thanks,<br>
    Vicente<br>
    <br>
    [1]
    <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~vromero/records.review/compiler/webrev.02/">http://cr.openjdk.java.net/~vromero/records.review/compiler/webrev.02/</a><br>
    <br>
    <blockquote type="cite"
      cite="mid:415e23ea-8414-5964-3d5a-66a7e33cb8e8@oracle.com">
      <p> <span class="new"></span> </p>
      <pre><span class="new"></span></pre>
      <div class="moz-cite-prefix">On 27/10/2019 22:09, Maurizio
        Cimadamore wrote:<br>
      </div>
      <blockquote type="cite"
        cite="mid:c93c974e-92c1-e4a3-c3a7-0ea5c5f26c2f@oracle.com">
        <meta http-equiv="Content-Type" content="text/html;
          charset=UTF-8">
        <p>Hi Vicente, looks a lot better, thanks.</p>
        <p>Some comments:</p>
        <p>Attr.java - Errors.FirstStatementMustBeCallToCanonical - not
          sure about this message , but in general, all we require is
          that a constructor delegates to another constructor - it can
          also be something other than the canonical - e.g contsructor1
          -> constructor2 -> canonical</p>
        <p>Attr.java - the above situation seems to also affect some
          other code in Attr - where you actually check that the invoked
          constructor in 'this' has same signature of the canonical
          constructor. I think these checks should be removed - the
          important thing is that the canonical will be called _somehow_
          - but can also be call indirectly - at least this was my
          understanding<br>
        </p>
        <p>Attr.java - not sure about the tree.sym.isRecord in the
          visitMethod - it seems like the same flag is used for both
          record (class) and canonical constructor? I'm ok with sharing
          the flag, but the method name looks a bit confusing when
          applied to a method symbol. I suggest putting 'isRecord'
          inside ClassSymbol, and 'isCanonicalRecordConstructor' on
          MethodSymbol, so that client code cannot make mistakes.</p>
        <p>Attr.java/TypeEnter.java - overall, the checks here look
          nice, and they 'blend' in with existing code nicely, I think.
          Well done!</p>
        <p>Enter.java - I guess my question here on records and
          treatment with STATIC is a general question as to why isn't
          the code doing the same thing for records and enums - aren't
          the rules similar? (e.g. enums must be static, etc)? Enums are
          not set STATIC by default in javac parser, and that is, I
          think the difference here. What pushed you in this direction?</p>
        <p>TypeEnter.java: I suggest renaming "getCanonicalInitDecl" to
          "getCanonicalConstructorDecl"<br>
          <br>
          TypeEnter.java - it seems like the result of
          getCanonicalInitDecl is always given the RECORD flag - but
          later on (in Lower) I saw a comment which says that only
          auto-generated symbols have the RECORD flag set. Which one is
          it?</p>
        <p>TypeEnter.java - I think the isUnchecked methods are a
          leftover from previous code - now they seem to be in Attr.java
          (btw, I think we must have those checks somewhere else in
          javac, look in Check::isUnchecked)</p>
        <p>Names.java - I think this bunch of fields is also no longer
          used?</p>
        <pre><span class="new">+    public final Name where;</span>
<span class="new">+    public final Name non;</span>
<span class="new">+    public final Name ofLazyProjection;</span>
<span class="new">+</span></pre>
        <p><br>
          I have not checked the j.l.m API.</p>
        <p>Thanks<br>
          Maurizio<br>
        </p>
        <p><br>
        </p>
        <p><br>
        </p>
        <p><br>
        </p>
        <div class="moz-cite-prefix">On 26/10/2019 21:14, Vicente Romero
          wrote:<br>
        </div>
        <blockquote type="cite"
          cite="mid:b072fc50-43e3-ae97-3cb6-c4c49e60edc0@oracle.com">
          <meta http-equiv="Content-Type" content="text/html;
            charset=UTF-8">
          I forgot to mention that I have added javax.lang.model code to
          this iteration that wasn't part of the first iteration. I was
          planning to publish it as part of a different review but I
          realized that there was some code affected, tests, API
          implementation etc, which belonged to the compiler code. So I
          added that code in this iteration,<br>
          <br>
          Thanks,<br>
          Vicente<br>
          <br>
          <div class="moz-cite-prefix">On 10/26/19 3:55 PM, Vicente
            Romero wrote:<br>
          </div>
          <blockquote type="cite"
            cite="mid:c265f401-36a9-dba7-5e0d-e95a11a3820f@oracle.com">
            <meta http-equiv="Content-Type" content="text/html;
              charset=UTF-8">
            Hi Maurizio,<br>
            <br>
            Thanks again for the comments I have published another
            iteration at [1]. I focused on this iteration more on the
            implemenation than on the tests to first have an agreement
            on the implementation, timing, etc. Some comments inlined
            below.<br>
            <br>
            [1] <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~vromero/records.review/compiler/webrev.01/"
              moz-do-not-send="true">http://cr.openjdk.java.net/~vromero/records.review/compiler/webrev.01/</a><br>
            <br>
            <div class="moz-cite-prefix">On 10/21/19 2:01 PM, Maurizio
              Cimadamore wrote:<br>
            </div>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <meta http-equiv="Content-Type" content="text/html;
                charset=UTF-8">
              <p>Hi Vicente,<br>
                I did a pretty thorough pass on most of the code. I
                didn't look at tests, and I also didn't look at Lower.
                Comments below:<br>
              </p>
              <p>* Flags.java - VARARGS flag for records components; I
                wonder, instead of a new flag, can we use the internal
                VARARGS flag we have for methods, and attach that to the
                record symbol? That should also lead to more direct code
                in TypeHelper<br>
              </p>
            </blockquote>
            removed<br>
            <br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p> </p>
              <p>* Symbol.java - I think the override for 'erasure' is
                redundant - isn't that the impl from supertype?</p>
            </blockquote>
            yep, removed too<br>
            <br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">*
              Symbol.java - I wonder if accessor list with Pair<Kind,
              Symbol> isn't a premature generalization; we should
              just add a getter symbol and that's it </blockquote>
            <br>
            agreed, done<br>
            <br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* Attr.java - I think we might want to leave the door
                open for a check which forces all constructors of a
                record to go through the canonical one (depending on
                where the spec lands)</p>
            </blockquote>
            implemented in this iteration<br>
            <br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* Check.java - understanding checkpoint: when we see an
                annotation on a record component, first we check it's
                one of the kinds which are allowed (if not, error), and,
                if it's allowed, we add all record component annotations
                to record component elements, and we also filter away
                all annotations that have nothing to do with the element
                in which they appear. If my understanding is correct, I
                think this logic should be documented more clearly; I
                found the comment after the "if (isRecordField)" to be a
                bit obscure.</p>
            </blockquote>
            yes that's the idea, annotations that are originally applied
            to record components are pushed down to all generated
            elements in TypeEnter, and then in Check the ones that are
            off-site are removed<br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* Enter.java - why are you removing the static flag on
                records? I don't see anything similar around for enums.</p>
            </blockquote>
            <br>
            the static flag is added to all records but if the record is
            a top level class, it is not needed, that's why that code is
            there<br>
            <br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* Flow.java - not sure I get the changes to checkInit;
                typically checkInit is called at the use-site of DA/DU
                variables. Here it seems you suppress some of the errors
                emitted for accessing record fields inside the canonical
                constructor - but I hope that code like this<br>
                <br>
                record Foo(int x) {<br>
                Â Â  Foo(int x) {<br>
                Â Â Â Â Â Â  print(this.x);<br>
                Â Â  }<br>
                }</p>
              <p>Still give errors?</p>
            </blockquote>
            <br>
            yes it gives an error<br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p> If this works correctly, which errors does the 'guard'
                around the error generation is supposed to protect
                against?</p>
            </blockquote>
            <br>
            checkInit it not used only for what you mentioned above but
            also, see AssignAnalyzer::visitMethodDef, to check if an
            initial constructor, as the canonical constructor is in
            records, have initialized all the fields. The guard is there
            to don't issue an error if a canonical constructor hasn't
            initialized some fields, as the compiler will generate code
            to initialize those fields later in Lower<br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* MemberEnter.java - why the filter for HYPOTHETICAL ?
                It's only used here...</p>
            </blockquote>
            removed<br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* TypeEnter.java - implicit super calls are added in
                Attr::visitMethod for regular calls; we should do the
                same for records (or add all in TypeEnter - that is
                records and class should be consistent)</p>
            </blockquote>
            <br>
            right we should be consistent, I have moved that code to
            Attr<br>
            <br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* TypeEnter.java - on finishClass - you are calling
                memberEnter on record fields, which I think you already
                did in the new RecordsPhase</p>
            </blockquote>
            <br>
            nope, what I'm doing there is invoking MemberEnter to enter
            the members that hasn't been entered so far. Anyway that
            code changed a bit because I'm entering the constructors now
            at RecordPhase too but I have changed the code a bit to make
            more clear what is happening.<br>
            <br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* TypeEnter.java - (stylistic) addRecordsMemberIfNeeded
                should deal with _all_ record members (e.g. including
                accessors), not just some?</p>
            </blockquote>
            yep changed that too<br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* TypeEnter.java - checkForSerializationMember should
                probably be moved to MemberEnter::visitVar, or even to
                Attr (note that the code for the check is doing a little
                visit :-))</p>
            </blockquote>
            moved to Attr<br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* TypeEnter.java - again on check timings; while it's
                ok for the code in here to add new synthetic members, I
                think it's less ok to add more global error checks (such
                as make sure that the canonical declaration whose
                parameter names match the record components in order);
                these should live in Attr. More generally, I think that
                we should only check stuff here if we think that the
                check will add any value to annotation processing. Every
                other check can be deferred, and take place in a more
                'deterministic' part of javac.</p>
            </blockquote>
            <br>
            moved to Attr<br>
            <br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* TypeEnter.java - I think finishClass should be a bit
                better at determining as to whether default constructor
                is needed or not - for instance, this check:</p>
              <pre>if ((sym.flags() & INTERFACE) == 0 &&
 928                 !TreeInfo.hasConstructors(tree.defs)) {</pre>
              <p>Should be generalized to something that works for both
                classes and records; for classes you need to check if
                there's no other constructor; for records you need to
                check if there's no other constructor _with same
                signature_ as the canonical one. Then you can simplify
                addRecordMembers and remove the dependency on the
                boolean 'generatedConstructor' parameter. In other words
                the code should:</p>
              <p>1) check if default/canonical constructor generation is
                required<br>
                2) if so, use the appropriate helper to generate the
                code<br>
                3) at the end, add the remaining record members (under
                the assumption that the canonical constructor has
                already been added in (1), if that was missing)</p>
            </blockquote>
            <br>
            done, in order to do that I had to enter constructors at the
            RecordPhase, as mentioned earlier.<br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>*TypeEnter.java - addAccessor can be simplified if we
                only worry about getters. Again, the checks in here feel
                more Attr check than MemberEnter checks.</p>
            </blockquote>
            agreed, done<br>
            <br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>*TypeEnter.java - in addRecordMembersIfNeeded, I don't
                get why we create a tree for a member, and then we visit
                the member tree with memberEnter, just to add it to the
                scope. I understand that, currently addEnumMembers does
                the same, but this looks very roundabout; I wonder if
                there's a way to make all this process a bit simpler -
                create a symbol and add that to the scope. Or are there
                important checks in MemberEnter that we would lose?<br>
              </p>
            </blockquote>
            <br>
            yes there are several checks we would lose, plus we would
            lose consistency, but I tried to do that and several things
            fell apart, we need to enter not only the generated method,
            also it's parameters etc, which is what MemberEnter is
            doing.<br>
            <br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p> </p>
              <p>*JCTree.java/TreeMaker.java - I don't think there's any
                need to store accessors in the field AST; these are only
                used from TypeEnter, and TypeEnter can do whatever it
                does by looking at which record components there are in
                the record class, and add a getter for each. Let's make
                the code simpler and more direct</p>
            </blockquote>
            yep removed<br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* ClassReader.java - should we just silently ignore
                record attributes when not in preview mode - or should
                we issue classfile errors?</p>
              <p>* ClassReader.java - what kind of validation should we
                do on record attributes? Currently javac does nothing.
                Should we check that we have (i) getters (ii)
                toString/hashCode/equals implementations and (iii) a
                canonical constructor (ad fail if we don't) ? At the
                very least I would add code to _parse_ the attribute,
                even if we do nothing with it, so that at least we throw
                a classfile error if the attribute is badly broken</p>
            </blockquote>
            on ClassReader, we can discuss what to do in a language
            meeting, I don't have any strong preference<br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* Tokens.java - for "var", "yields" and other
                context-dependent keywords we never added a token. We
                just handled that in JavacParser. Why the difference
                here? I think it's best to stick to current style and
                maybe fix all of them (assuming that's what we want to
                do) in a followup cleanup. Actually, after looking at
                parser, it seems like you already handle that manually,
                so I just suggest to revert the changed to Tokens<br>
              </p>
            </blockquote>
            <br>
            I added the token to add it as a parameter to an error
            message, but I removed the token and now I'm passing a
            string<br>
            <br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p> </p>
              <p>* TreeInfo.java - how is 'isCanonicalConstructor' not
                returning 'true' for all constructors inside a record,
                as opposed to only return true for the canonical one?</p>
            </blockquote>
            I have added a comment to clarify what this method is doing<br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* TreeInfo.java - There is some code reuse possible
                between "recordFieldTypes" and "recordFields"</p>
            </blockquote>
            yep done<br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* Names.java - what is 'oldEquals' ?</p>
            </blockquote>
            removed, old code<br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* JavacParser.java - timing of checks; I don't think we
                should check for illegal record component names in here</p>
            </blockquote>
            removed from there<br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p>* JavacParser.java - code can be simplified somewhat by
                getting rid of accessors in VarDef AST.<br>
              </p>
            </blockquote>
            <br>
            done<br>
            <br>
            Thanks again for taking the time to do this long review,
            will answer the other mails separately<br>
            <br>
            Vicente<br>
            <blockquote type="cite"
              cite="mid:0be9bfc0-70b1-c1f8-5af1-5c88913dbb9d@oracle.com">
              <p> </p>
              <p><br>
              </p>
              <p><br>
              </p>
              <p><br>
              </p>
              <div class="moz-cite-prefix">On 21/10/2019 13:31, Vicente
                Romero wrote:<br>
              </div>
              <blockquote type="cite"
                cite="mid:1cfed5da-a0eb-be75-9008-8a0a5666a6f9@oracle.com">Hi,
                <br>
                <br>
                Please review the compiler code for JEP 359 (Records)
                [1] <br>
                <br>
                Thanks in advance for the feedback, <br>
                Vicente <br>
                <br>
                [1] <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~vromero/records.review/compiler/webrev.00/"
                  moz-do-not-send="true">http://cr.openjdk.java.net/~vromero/records.review/compiler/webrev.00/</a>
                <br>
              </blockquote>
            </blockquote>
            <br>
          </blockquote>
          <br>
        </blockquote>
      </blockquote>
    </blockquote>
    <br>
  </body>
</html>