<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>The patch is ok, but I'm still not super convinced about
treatment of local records; example:</p>
<p>$ cat TestLocalRecord.java <br>
</p>
<p>class TestLocalRecord {<br>
void m() {<br>
  String s = "Hello!";<br>
  record A() {<br>
      void m() { System.out.println(s); }<br>
  }<br>
  new A().m();<br>
}<br>
}<br>
</p>
<p>$ javap -s TestLocalRecord\$1A.class Compiled from
"TestLocalRecord.java"<br>
</p>
<p>final class TestLocalRecord$1A extends java.lang.Record {<br>
 final java.lang.String val$s;<br>
   descriptor: Ljava/lang/String;<br>
 public TestLocalRecord$1A();<br>
   descriptor: (Ljava/lang/String;)V<br>
<br>
 void m();<br>
   descriptor: ()V<br>
<br>
 public java.lang.String toString();<br>
   descriptor: ()Ljava/lang/String;<br>
<br>
 public final int hashCode();<br>
   descriptor: ()I<br>
<br>
 public final boolean equals(java.lang.Object);<br>
   descriptor: (Ljava/lang/Object;)Z<br>
}<br>
</p>
<p>Note the mismatch between the descriptor of the canonical
constructor and the source signature of the same. This record
seems not to be "the whole state and nothing but the state"
because of the presence of captured fields in there.</p>
<p>Maurizio<br>
</p>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">On 12/12/2019 00:40, Vicente Romero
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:671447a4-d93f-05a5-d26e-d538115b28a9@oracle.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
I have uploaded a new iteration at [1],<br>
<br>
Thanks for your comments,<br>
Vicente<br>
<br>
[1] <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~vromero/8235778/webrev.01/"
moz-do-not-send="true">http://cr.openjdk.java.net/~vromero/8235778/webrev.01/</a><br>
<br>
<div class="moz-cite-prefix">On 12/11/19 7:08 PM, Maurizio
Cimadamore wrote:<br>
</div>
<blockquote type="cite"
cite="mid:cfd47ff1-4152-5962-78c5-71fea5580ef4@oracle.com">
<meta http-equiv="Content-Type" content="text/html;
charset=UTF-8">
<p>If sym.isLocal() returns true, is this check</p>
<pre><span class="new">&& (sym.owner.flags_field & STATIC) == 0) </span></pre>
<div class="moz-cite-prefix">Needed? Aren't we inside a record
declaration that is contained in some local context (e.g.
within a method body), whose immediate enclosing type is a
type T? If so, isn't T always non-static? I guess yes, unless
T is a record itself, like:</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">void m() {<br>
  record A() {<br>
       record B() { }<br>
  }<br>
}</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">The patch seems to be biased in
favor of this - is it deliberate? (also there's no test around
that). Should the spec say something?<br>
</div>
<div class="moz-cite-prefix"><br>
Maurizio<br>
</div>
<div class="moz-cite-prefix"><br>
</div>
<div class="moz-cite-prefix">On 11/12/2019 23:39, Vicente Romero
wrote:<br>
</div>
<blockquote type="cite"
cite="mid:7f09a74c-cce5-21db-485f-de75a6ee8ed2@oracle.com">Hi,
<br>
<br>
Please review the fix for [1] at [2]. Records are not allowed
to be defined inside inner classes. This patch extends the
check to local inner classes which was missing. <br>
<br>
Thanks, <br>
Vicente <br>
<br>
<a class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8235778"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8235778</a>
<br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~vromero/8235778/webrev.00/"
moz-do-not-send="true">http://cr.openjdk.java.net/~vromero/8235778/webrev.00/</a>
<br>
<br>
</blockquote>
</blockquote>
<br>
</blockquote>
</body>
</html>