RFR: JDK-8243057: compilation of annotated static record fields fails with NPE

Vicente Romero vicente.romero at oracle.com
Fri Jul 24 04:38:45 UTC 2020


Hi Jan,

I have updated the webrev as we discussed offline [1],

Thanks,
Vicente

[1] http://cr.openjdk.java.net/~vromero/8243057/webrev.03/

On 7/21/20 11:40 AM, Vicente Romero wrote:
> Hi Jan,
>
> Yep you are right but given that isRecordMember already have the info 
> about the record flag we can rewrite the condition as:
>
> boolean isRecordField = isRecordMember &&
>          declarationTree.hasTag(VARDEF) &&
>          s.owner.kind ==TYP;
> [1] http://cr.openjdk.java.net/~vromero/8243057/webrev.02/
>
> Sorry for the noise,
> Vicente
>
> On 7/21/20 9:57 AM, Vicente Romero wrote:
>> Hi Jan,
>>
>> Thanks for the review. the RECORD flag is applied to static fields 
>> too what about:
>>
>> boolean isRecordField = isRecordMember &&
>>          !s.isStatic() &&
>>          declarationTree.hasTag(VARDEF) &&
>>          s.owner.kind ==TYP;
>> ?
>> I have made this change at [1],
>>
>> Thanks,
>> Vicente
>>
>> [1] http://cr.openjdk.java.net/~vromero/8243057/webrev.01/
>>
>> On 7/21/20 4:50 AM, Jan Lahoda wrote:
>>> Hi Vicente,
>>>
>>> I am sorry, but saying "s.flags_field == $mask" feels somewhat 
>>> dangerous to me - what if there are other (valid) flags that can be 
>>> added to record components/fields?
>>>
>>> My understanding was that the record members are marked with the 
>>> RECORD flag, should it be enough to say:
>>> boolean isRecordField = declarationTree.hasTag(VARDEF) &&
>>>         (s.flags_field & RECORD) != 0 &&
>>>         s.owner.kind == TYP;
>>> ?
>>>
>>> Thanks,
>>>     Jan
>>>
>>> On 20. 07. 20 18:53, Vicente Romero wrote:
>>>> ping, it is a simple review,
>>>>
>>>> Vicente
>>>>
>>>> On 7/17/20 3:36 PM, Vicente Romero wrote:
>>>>> Please review fix for [1] at [2]. At method 
>>>>> Check::validateAnnotation the condition used to determine if a 
>>>>> field is or not a record field was wrong. As a consequence static 
>>>>> fields were considered as record fields. This doesn't have any 
>>>>> consequence unless they are annotated. This patch is fixing this 
>>>>> issue.
>>>>>
>>>>> Thanks,
>>>>> Vicente
>>>>>
>>>>> [1] https://bugs.openjdk.java.net/browse/JDK-8243057
>>>>> [2] http://cr.openjdk.java.net/~vromero/8243057/webrev.00/
>>>>
>>
>

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


More information about the compiler-dev mailing list