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

[Srikanth]

On Thursday 12 July 2018 04:45 PM, Srikanth wrote:

[...]

>>
>> - don't you need also to override visitApply in case the constructor 
>> is calling  an instance method on the value being constructed?
>
> This will be covered as part of JDK-8205910

Oops! I forgot to cover that under JDK-8205910. I have raised 
https://bugs.openjdk.java.net/browse/JDK-8208067 now.

Thanks
Srikanth

>>
>> * 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:
>
> I think this is a non-issue, anyway I have added the following as a 
> test FWIW.
>
>>
>> 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.
>>
>>
>> That's all I can think of.
>>
>> Cheers
>> Maurizio
>>
>>
>>
>> On 11/07/18 11:56, Srikanth wrote:
>>>
>>> Hi Maurizio,
>>>
>>> Please review the following patch that implements the changes 
>>> required for
>>> https://bugs.openjdk.java.net/browse/JDK-8198749 ([lworld] 
>>> Translation of value constructors in classic constructor notation)
>>>
>>> Webrev: http://cr.openjdk.java.net/~sadayapalam/JDK-8198749/webrev.00/
>>>
>>> Part of the work in supporting value construction in the classic 
>>> constructor notation
>>> will happen independently on behalf of 
>>> https://bugs.openjdk.java.net/browse/JDK-8205910(diagnose use of 
>>> 'this' with DU fields (for VTs and VBCs)).
>>>
>>> Normally I would have requested the review after a fair bit more of 
>>> testing and self-review, but given your proposed long absence, I am 
>>> requesting the review now. However, I am happy to note that the code 
>>> is in pretty decent shape for another person to study (known issues 
>>> at the bottom)
>>>
>>> I will continue with the testing and self review in parallel.
>>>
>>> If time is an issue, you can limit the review to the following 
>>> source files:
>>>
>>>     Gen.java
>>>     TransValues.java
>>>     LambdaToMethod.java
>>>
>>> and these test files: (these give an idea of what works already and 
>>> allows you to experiment by tweaking)
>>>
>>> test/langtools/tools/javac/valhalla/lworld-values/InnerValueNew.java
>>> test/langtools/tools/javac/valhalla/lworld-values/LocalValueNew.java
>>> test/langtools/tools/javac/valhalla/lworld-values/QualifiedThisTest.java 
>>>
>>> test/langtools/tools/javac/valhalla/lworld-values/ValueConstructorRef.java 
>>>
>>> test/langtools/tools/javac/valhalla/lworld-values/ValueNewReadWrite.java 
>>>
>>>
>>> Known issues:
>>>
>>> (1) make.at() calls may not be updated consistently/uniformly.
>>> (2) I need to double check that some subtree translations are not 
>>> skipped inadvertently.
>>> (3) Some of the other *older* modified tests need rewriting - they 
>>> compare javap output which is flaky due to constant pool offsets 
>>> changing anytime there is a change in code generation scheme.
>>> (4) Some code duplication can be avoided by creating utility routines.
>>>
>>> langtools tests are green (except for one non-material failure).
>>>
>>> Thanks!
>>> Srikanth
>>
>>
>