RFR: JDK-8243057: compilation of annotated static record fields fails with NPE
Jan Lahoda
jan.lahoda at oracle.com
Fri Jul 24 08:27:29 UTC 2020
Looks good to me.
Thanks,
Jan
On 24. 07. 20 6:38, Vicente Romero wrote:
> 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/
>>>>>
>>>
>>
>
More information about the compiler-dev
mailing list