RFR:JDK-8198749 Translation of value constructors in classic constructor notation

[Srikanth]

Thanks so much for testing this, Remi!

I raised https://bugs.openjdk.java.net/browse/JDK-8207330 and fixed it 
with a push of
http://hg.openjdk.java.net/valhalla/valhalla/rev/f0e1ddcd0054

(Same problem existed for synthetics storing captured outer locals - 
also fixed)

Thanks
Srikanth

On Sunday 15 July 2018 06:39 PM, Remi Forax wrote:
> Hi Srikanth,
>
> ----- Mail original -----
>> De: "Srikanth" <srikanth.adayapalam at oracle.com>
>> À: "Maurizio Cimadamore" <maurizio.cimadamore at oracle.com>, "valhalla-dev" <valhalla-dev at openjdk.java.net>
>> Envoyé: Jeudi 12 Juillet 2018 11:04:34
>> Objet: Re: RFR:JDK-8198749 Translation of value constructors in classic constructor notation
>> On Wednesday 11 July 2018 06:38 PM, Maurizio Cimadamore wrote:
>>> * tests - mayeb for later, and depending on whether we support inner
>>> values (seems to, looking at InnerValueNew) - one kind of super
>>> constructor call that might get you in trouble is the qualified super
>>> constructor call - e.g., with regular ordinary classes you can have this:
>>>
>>> class A {
>>>
>>>      class Inner { }
>>> }
>>>
>>>
>>> class B extends A.Inner {
>>>
>>>         B(A encl) {
>>>             enc.super();
>>>         }
>>> }
>>>
>>>
>>> It would be interesting to verify at some point that this kind of
>>> idiom works with your value desugaring too.
>>>
>>
>> Perhaps I am missing some nuance:
>>
>> Of the three classes involved, A, B and A.Inner, only A can be a value
>> class. B and A.Inner cannot be since values don't extend and can't be
>> extended.
>>
>> In that case, after the prevailing desugaring/lowering we end up with
>>
>> class B extends A$Inner {
>>
>>      B(A encl) {
>>          super(<*nullchk*>encl);
>>      }
>> }
>>
>> that is being fed as *input* to the new translator. The nullcheck is
>> bogus and should be eliminated for
>> value typed `encl' - but what other complication do you see ?
>>
>> I do have tests that verify that constructors that receive synthetics
>> such as enclosing instances and captured outer locals are translated
>> properly.
>>
>> Thanks
>> Srikanth
>
> I've just tested several inner classes cases and everything seems fine,
> Maurizio, most of the trouble comes as you said when you mix inheritance and inner classes, but a value type can only inherits from Object,
> so what you can do that may prevent the rewriting of the value type constructor is limited.
>
> I've still found a bug :)
> the field that reference the outer class is not marked as ACC_FLATENABLE even if the outer class is a value type.
>
> in this example,
> public __ByValue class Outer {
>    private final  int value;
>    
>    public Outer(int value) {
>      this.value = value;
>    }
>    
>    public __ByValue class Inner {
>      private final int value2;
>      
>      public Inner(int value2) {
>        this.value2 = value2;
>      }
>    }
> }
>
> javap says that inside Outer$Inner.class, this$0 is not ACC_FLATENABLE
>    final fr.umlv.valuetype.Outer this$0;
>      descriptor: Lfr/umlv/valuetype/Outer;
>      flags: (0x1010) ACC_FINAL, ACC_SYNTHETIC
>
>
>>
>>> That's all I can think of.
>>>
>>> Cheers
>>> Maurizio
>
> cheers,
> Rémi