RFR: JDK-8235778: No compilation error reported when a record is declared in a local class

[Vicente Romero]

should the spec be more specific about local records, like mentioning 
that they can't capture state?

Vicente

On 12/12/19 7:34 AM, Maurizio Cimadamore wrote:
>
> The patch is ok, but I'm still not super convinced about treatment of 
> local records; example:
>
> $ cat TestLocalRecord.java
>
> class TestLocalRecord {
> void m() {
>    String s = "Hello!";
>    record A() {
>        void m() { System.out.println(s); }
>    }
>    new A().m();
> }
> }
>
> $ javap -s TestLocalRecord\$1A.class Compiled from "TestLocalRecord.java"
>
> final class TestLocalRecord$1A extends java.lang.Record {
>   final java.lang.String val$s;
>     descriptor: Ljava/lang/String;
>   public TestLocalRecord$1A();
>     descriptor: (Ljava/lang/String;)V
>
>   void m();
>     descriptor: ()V
>
>   public java.lang.String toString();
>     descriptor: ()Ljava/lang/String;
>
>   public final int hashCode();
>     descriptor: ()I
>
>   public final boolean equals(java.lang.Object);
>     descriptor: (Ljava/lang/Object;)Z
> }
>
> 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.
>
> Maurizio
>
>
> On 12/12/2019 00:40, Vicente Romero wrote:
>> I have uploaded a new iteration at [1],
>>
>> Thanks for your comments,
>> Vicente
>>
>> [1] http://cr.openjdk.java.net/~vromero/8235778/webrev.01/
>>
>> On 12/11/19 7:08 PM, Maurizio Cimadamore wrote:
>>>
>>> If sym.isLocal() returns true, is this check
>>>
>>> && (sym.owner.flags_field & STATIC) == 0)
>>> 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:
>>>
>>> void m() {
>>>    record A() {
>>>         record B() { }
>>>    }
>>> }
>>>
>>> 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?
>>>
>>> Maurizio
>>>
>>> On 11/12/2019 23:39, Vicente Romero wrote:
>>>> Hi,
>>>>
>>>> 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.
>>>>
>>>> Thanks,
>>>> Vicente
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8235778
>>>> http://cr.openjdk.java.net/~vromero/8235778/webrev.00/
>>>>
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/compiler-dev/attachments/20191212/27637057/attachment-0001.htm>