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

[Maurizio Cimadamore]

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/acaeeff0/attachment.htm>