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

[Maurizio Cimadamore]

I believe so - Remi started a thread on this topic on amber-spec-experts

Maurizio

On 12/12/2019 17:49, Vicente Romero wrote:
> 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/b8679480/attachment.htm>