implementation of record constructor auto-initialization [was Re: instance initializer]
Maurizio Cimadamore
maurizio.cimadamore at oracle.com
Fri Sep 6 08:54:25 UTC 2019
To be clear - if we wanted to allow 'return' with partially
un-initialized code, then Vicente's example would become:
record R(int i, int j) {
public R { // compact constructor
if (i < 0) {
this.i = -i;
return;
} else {
this.i = i;
return;
}
}
}
At which the compiler would have to say: hey, I have to init 'j', and
write this:
record R(int i, int j) {
public R { // compact constructor
if (i < 0) {
this.i = -i;
this.j = j; //synthetic
return;
} else {
this.i = i;
this.j = j; //synthetic
return;
}
}
}
I'm not super sure we need to go there - I could see an argument saying
that, if we do this, "return" doesn't mean "return".
But if we wanted to do this, I think the best trick javac could do is
something that Vicente has explored already:
record R(int i, int j) {
public R { // compact constructor
try {
if (i < 0) {
this.i = -i;
return;
} else {
this.i = i;
return;
}
} finally {
this.j = j;
}
}
}
That is, wrap the body in try/finally and assign remaining fields in the
finally - that takes care of the exit paths. But I have some reservation
(not related to the impl) that we really want this for auto
initialization, as that seems to change semantics for 'return' and 'throw'.
Maurizio
On 06/09/2019 09:45, Maurizio Cimadamore wrote:
>
>
> 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
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
More information about the amber-dev
mailing list