implementation of record constructor auto-initialization [was Re: instance initializer]

[Brian Goetz]

This makes sense to me, with the proviso that “at the end” doesn’t necessarily mean one place, but all exit paths?



Sent from my iPad

> On Sep 6, 2019, at 4:04 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
> 
> (moving to amber-dev, since I want to focus on implementation issues here)
> 
> Vicente,
> it is true that assessing whether a field is DA at the end of the constructor is a complex task. That said, we have machinery to do it in the compiler (Flow class).
> 
> My feeling here is that, perhaps, the compiler is attempting to do things too early in the pipeline; if the initialization code is added say, at Enter/TypeEnter, there's no way for the compiler to see what the initialization state of the fields actually is.
> 
> I think the best approach is to wait it out - that is, send the constructor unmodified through Attr, and then define some special logic in Flow that:
> 
> * issue errors if, at the end of record constructor there is some field that is neither DA nor DU
> * do not issue errors at the end of a record constructor for DU fields
> * keep a list of the DU fields for later
> 
> Then in Lower, we can look at the list, and add the missing initializers for the previously discovered DU fields. So, that cover the first part of your concerns.
> 
> The second, and more meaty part, is that if you have stuff like:
> 
> record R(int i, int j) {
>     public R {  // compact constructor
>         if (i < 0) {
>             this.i = -i;
>         } else {
>             this.j = j;
>         }
>     }
> }
> 
> You might be induced to think that the compiler has to put an initializer inside the if branch, and another inside the else (one for this.j, the other for this.i). But this is not actually the case; in this example, both this.i and this.j are neither DA nor DU at the end of the constructor -> error.
> 
> In other words, I think the rules stated by Brian already take into account the fact that the constructor can be messy, and that the only thing that auto-initialization is allowed to do, is to add a bunch of initializers _at the end_ of the constructor; if that doesn't work - error (because that means that there were some fields initialized in some paths but not in others).
> 
> Makes sense?
> 
> Maurizio
> 
>> On 06/09/2019 00:47, Vicente Romero wrote:
>> 
>> 
>>> On 9/5/19 6:14 PM, forax at univ-mlv.fr wrote:
>>> 
>>> 
>>> De: "Brian Goetz" <brian.goetz at oracle.com>
>>> À: "Tagir Valeev" <amaembo at gmail.com>, "Remi Forax" <forax at univ-mlv.fr>
>>> Cc: "amber-spec-experts" <amber-spec-experts at openjdk.java.net>
>>> Envoyé: Jeudi 5 Septembre 2019 23:26:34
>>> Objet: Re: Draft JLS spec for records
>>> 
>>> 
>>> I don't think so. A compact constructor (or require initializer, as you propose) could be not the only constructor. An instance initializer is convenient because it's added to every constructor, regardless of whether it's compact or not. So the new thing doesn't supersede the instance initializer and I see no good reason to explicitly disable it.
>>> 
>>> Vicente offered another reason why we might want to prohibit the instance initializer, if only out of expediency; it complicates the analysis of which fields are DA on all paths through the constructor (and therefore, do not need to be automatically initialized.)  If you have a record:
>>> 
>>>     record Foo(int i) { 
>>>         { this.i = 0; }
>>>     }
>>> 
>>> then the canonical constructor has to see that `i` is always initialized by the static init, and therefore should be not initialized.  Worse, if we have:
>>> 
>>>     record Foo(int i) { 
>>>         { if (tuesday) this.i = 0; }
>>>     }
>>> 
>>> then we have to issue a compilation error, since we have fields that are neither DA nor DU at the end of the initializer.
>>> 
>>> None of this is impossible to do, of course; it's just not clear whether it's worth it, given the limited utility of instance initializers in records (because we've already banned instance fields.)  
>>> 
>>> If we want to fix that with an the analysis, it will have to work on user-written code mixed with generated code,
>>> it doesn't seem to be a good idea.
>> 
>> right I was thinking about the case:
>> 
>> record R(int i, int j) {
>>     public R {  // compact constructor
>>         if (i < 0) {
>>             this.i = -i;
>>         } else {
>>             this.j = j;
>>         }
>>     }
>> }
>> 
>> which doesn't have instance initializer blocks. The thing is where to draw between cases where the compiler will just issue an error and cases in which it will generate the missing initializations. In this particular case the compiler will have to be very clever and generate:
>> 
>> public R {  // compact constructor
>>         if (i < 0) {
>>             this.i = -i;
>>             this.j = j;  // automatic code
>>         } else {
>>             this.i = i; // automatic code
>>             this.j = j;
>>         }
>>     }
>> 
>> we can do it, it will be complex though, but do we want to do it? Where to draw the line?
>> 
>> Thanks,
>> Vicente
>>> 
>>> so it's Ok to not support instance initializers in record.
>>> 
>>> Rémi
>>> 
>>> 
>>> 
>>> 
>>