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

[Vicente Romero]

Ok can we then push this issue, if acceptable in its current state, an 
deal with the state capturing problem in a separate issue?

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/4064c953/attachment-0001.htm>