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

[Maurizio Cimadamore]

On 06/09/2019 09:40, Maurizio Cimadamore wrote:
>
>
> On 06/09/2019 09:26, Brian Goetz wrote:
>> This makes sense to me, with the proviso that “at the end” doesn’t 
>> necessarily mean one place, but all exit paths?
>
> Yes - that's what I meant.
>
Actually - not sure - what do you mean by "exit path"? We clearly can't 
have "returns" here. And, if we "throw" then we don't care much about 
initializing.

So, looking at the constructor as a whole, either a field has been 
assigned in _all_ paths, or it has only in _some_ paths, or it has not 
been initialized in _any_ path.

In the first case, no auto-generation is required. In the second case we 
give an error. In the third case we can add an initializer at the end of 
the constructor I think?

Maurizio

> Maurizio
>
>>
>>
>>
>> Sent from my iPad
>>
>> On Sep 6, 2019, at 4:04 AM, Maurizio Cimadamore 
>> <maurizio.cimadamore at oracle.com 
>> <mailto: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
>>>>>
>>>>>
>>>>>
>>>>>
>>>>