RFR: JDK-8234445: spurious error message for record constructors with receiver parameters

[Vicente Romero]

Hi,

Thanks for the review,

On 12/19/19 11:09 AM, Maurizio Cimadamore wrote:
> Looks good - two questions:
>
> 1) do we really need to add this/super at different times? Perhaps we 
> could also add this/super in RecordsHeaderPhase for all classes (class 
> + records)
> 2) or, if not, I'd suggest moving the code which adds this/super in a 
> routine, and call the routine from where you need it

I think I prefer option 2 even if 1 is doable, I would prefer to let the 
RecordPhase to record stuff only, also I prefer to let the current flow 
stable and introduce a change to records only. So I have added a new 
routine and invoked it from two different places [1]. How does it look?

>
> Maurizio

Thanks,
Vicente

[1] http://cr.openjdk.java.net/~vromero/8234445/webrev.01/
>
> On 18/12/2019 18:24, Vicente Romero wrote:
>> Please review the fix for [1] at [2]. What happens here is that 
>> basically for records we are altering the order in which members are 
>> entered, this is because the compiler needs to check if a user 
>> provided constructor has the same erasure as the canonical 
>> constructor or not and depending on this generate one or not, or even 
>> issue an error. The only way to have access to this information is to 
>> enter all the constructors before the compiler gets to the point 
>> where it needs to decide if a canonical should be generated or not. 
>> The current implementation 'forgot' to add `this` and `super` to the 
>> scope before entering all the constructors. This is the reason for 
>> the spurious error message: because `this` was not entered before the 
>> user defined constructors were entered by the compiler
>>
>> Thanks,
>> Vicente
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8234445
>> [2] http://cr.openjdk.java.net/~vromero/8234445/webrev.00/