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

[Srikanth Adayapalam]

On Tue, 18 Jan 2022 16:03:00 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> 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.

It is a fact that quite a bit of changes are in the pipeline, particularly with respect to internal API rationalization/cleanup, but lest I get too comfortable sweeping the present point of discussion under that carpet, here are some clarifications,

> 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. 

Yes, the API points isPrimitiveClass() will answer true only for B3 while isValueClass() will answer true for both B2 and B3. One of the next steps is to go through with a fine toothed comb every use of isPrimitiveClass in the code base
to make sure that it is guarding treatment specifc to B3 classes - everything else should have been pushed down to isValueClass() gate.

> 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).

Yes, String.ref is an error and given value class Point, Point.ref is an error today.

> 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.

I would say there is no blurring of distinction - the internal APIs that were evolving for a set of needs haven't fully been custom fit for the new world yet.

Yes, within the next couple of weeks, I expect to have futher changes that would have removed certain vestiges, retooled others to fit the current model and I will invite you to review that.

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

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