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

[Maurizio Cimadamore]

On 16/12/2019 15:28, Vicente Romero wrote:
> Ok can we then push this issue, if acceptable in its current state, an 
> deal with the state capturing problem in a separate issue?

That's fine by me

Cheers
Maurizio

>
> Thanks,
> Vicente
>
> On 12/12/19 1:38 PM, Maurizio Cimadamore wrote:
>>
>> 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/20191216/fa9b64ca/attachment.htm>