[lworld] RFR: Implement code generation support for value classes

[Maurizio Cimadamore]

On Tue, 18 Jan 2022 15:03:47 GMT, Srikanth Adayapalam <sadayapalam at openjdk.org> wrote:

>> src/jdk.compiler/share/classes/com/sun/tools/javac/code/Type.java line 1333:
>> 
>>> 1331:         @Override
>>> 1332:         public boolean isValueClass() {
>>> 1333:             return !isReferenceProjection() && tsym != null && tsym.isValueClass();
>> 
>> The new test for `isReferenceProjections` does not match my understanding of what a "value class" is. E.g. I believe a value class doesn't really have a reference projection (or, rather, doing vclass.ref is the same as doing String.ref - it's the identity). So it's not clear to me the rationale behind the new check.
>
> You are right, a B2 value class does not have a reference projection, in fact String.ref and ValueClass.ref will ATM result in a compile error. In the implementation .ref can be applied only to a primitive class (B3) - the spec may call for .ref to a total operation - but this is what we do at the moment awaiting spec clarification on the whole handling of .ref syntax. There is a ticket open for it - but at the moment I have trouble accessing JIRA
> 
> Now a B3 class can and does have a reference projection and every B3 class is a B2 class. So
> the receiver of the method isValueClass() could be a B3 class's primitive class type or its reference projection type . 
> This necessitates the new change to the method.
> 
> Consider this method from Gen:
> 
> public void visitDefaultValue(JCDefaultValue tree) {
>         if (tree.type.isValueClass()) {
>             code.emitop2(aconst_init, checkDimension(tree.pos(), tree.type), PoolWriter::putClass);
>         } else if (tree.type.isReference()) {
>             code.emitop0(aconst_null);
>         } else {
>             code.emitop0(zero(Code.typecode(tree.type)));
>         }
>         result = items.makeStackItem(tree.type);
>         return;
>     } 
> 
> Without the proposed change to isValueClass(), we would emit aconst_init instead of aconst_null for
> PrimitiveClass.ref.default
> 
> On a related note, (again hampered by lack of access to JBS), we have a ticket open for rationalization/cleanup of various internal APIs - I had put in some work into it - but that had to be halted because of large scale changes to landscape - (ref default classes being gone for example). This work needs to be taken up in the context of the new
> model to bring upto date the APIs. For example com.sun.tools.javac.code.Type.ClassType.Flavor is not current )
> 
> Let me know if this addresses this your question, Thanks for reviewing!

I'm still confused a bit. I get that this is all transitional, in a way - but I would expect javac to differentiate between B2 and B3 more. It is true, as you claim that `every B3 class is a B2 class`: some of the identity restrictions apply to both declarations. But in my opinion, the current javac support (as of before this PR) is much closer to B3 than it is to B2 declarations, at least from a type system perspective. That is, a `value class` should be, to javac, just a plain reference class, with no reference projection etc. So, if String.ref is an error, I'd expect vclass.ref to be an error in the same way (in this transitional world). 
>From a translation perspective I agree that B2 and B3 are not too different (no constructors, etc.) - but I'm a bit afraid that by blurring distinction between  B2 and B3 in the type system we might end up with problems further down the road.

That said, I understand that a lot of this is temporary, so if you think it would be more adequate I'm fine looking again at some point in the future.

-------------

PR: https://git.openjdk.java.net/valhalla/pull/604