RFR:JDK-8198749 Translation of value constructors in classic constructor notation (original) (raw)

Srikanth srikanth.adayapalam at oracle.com
Thu Jul 12 09:04:34 UTC 2018

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

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

More information about the valhalla-dev mailing list