[lworld] RFR: 8266466: [lworld] Enhance javac to consume unified primitive class files generated under the option -XDunifiedValRefClass [v2]

[Srikanth Adayapalam]

On Fri, 21 May 2021 11:52:25 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

> Overall, a clever strategy.
> 
> I'm a bit doubtful that we have seen the end of the string here. In particular, when it comes to LFoo; ambiguities (is it a regular reference Foo or a projection Foo.ref of some primitive type?) I think the old and trusted signature attribute also has a role to play.
> 
> The signature attribute will in fact require updates also to support universal generics.

You are right that this is not the end of the story. The implementation deals with just a single bit ACC_PRIMITIVE and weaves that into the type system - so as of this patch, we can have plain old L_TypeOf_L, L_Typeof_Q and Q_TypeOf_Q as fully "evolved forms"  and X_Typeof_X, Q_Typeof_X and L_Typeof_X as the "larval forms".

BUT,  the ambiguity you right about i.e "In particular, when it comes to LFoo;  ... (is it a regular reference Foo or a projection Foo.ref of some primitive type?)" is handled already in this patch. The single bit ACC_PRIMITIVE is enough for this.

See com.sun.tools.javac.code.Type.ClassType#isReferenceProjection which disambiguates L_TypeOf_X into one of L_TypeOf_Q or L_TypeOf_L. 

I plan to take up work on JDK-8244231 right after this,  at which point, a larval form may see other evolutions.

> src/jdk.compiler/share/classes/com/sun/tools/javac/code/Type.java line 1033:
> 
>> 1031:          * viewed against the default nature of the associated class.
>> 1032:          */
>> 1033:         public enum Flavor {
> 
> Perhaps you could add a method which "upgrades" a flavor to a new one, given new info.
> 
> e.g.
> 
> 
> L_TypeOf_X.update(sym.isPrimitiveClass()) ->  L_TypeOf_Q/L_TypeOf_L (depending on isPrimitiveClass)
> 
> 
> This would allow you to rule out (e.g. throw) bad updates. E.g. X flavors can be updated, all the other "leaf" flavors can only be updated to self. 
> 
> I think a method such as this would remove a lot of conditional logic in the clients.

I'll take a look ...

> src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/ClassReader.java line 666:
> 
>> 664:                     } else {
>> 665:                         // We are seeing QFoo; or LFoo; The name itself does not shine any light on default val-refness
>> 666:                         flavor = prefix == 'L' ? Flavor.L_TypeOf_X : Flavor.L_TypeOf_X;
> 
> Is there a typo here - shouldn't be:
> 
> 
>                         flavor = prefix == 'L' ? Flavor.L_TypeOf_X : Flavor.Q_TypeOf_X;

Oops. Yes it should be as you say. I am puzzled because this was copy pasted code. Don't know how the error got introduced! Good catch! The other two places do the right thing. Hmm.

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

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