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

[Maurizio Cimadamore]

On Fri, 21 May 2021 11:28:55 GMT, Srikanth Adayapalam <sadayapalam at openjdk.org> wrote:

> Post the integration of https://bugs.openjdk.java.net/browse/JDK-8244227,
> the constructor of ClassType, takes a boolean isReferenceProjection as an additional
> parameter. Till now, it was feasible to upfront decide at the time of constructing
> a class type whether the proposed type is supposed to be a reference projection or not.
> 
> This is no longer possible under the unified class file generation scheme we are
> moving to. When we see LFoo; in field/method signatures/descriptors, we don't know
> whether the type is Foo.ref of a primitive class Foo or a plain old reference type.
> 
> Likewise when we see QFoo; we have an ambiguity whether this is the primitive class
> type Foo or is Foo.val type of a ref-default primitive class Foo. Till a class is
> fully built ("completed"), we don't know whether it is a primitive class and if so,
> what its ref-val defaultness is.
> 
> Completing class Foo every time we see a type descriptor LFoo; or QFoo; is wasteful
> of resources. Nor is it possible as it is, since it would call for the ClassReader's
> code to be reentered even as some other class is being read in: So the proposed patch
> implements an incremental piece meal augmentation of attributes.
> 
> I suggest the review be started with Type.java in ClassType#Flavor enumeration to gather
> a high level picture of the abstractions put in place to characterize a class type in
> an incremental basis. After Type.java, Attr.java, ClassReader.java and Symbol.java in
> that order provide a good order for review.
> 
> Please note, while this patch puts together the infrastructure and layes the groundwork
> for modelling ref-default classes and thereby enable VBC migration work, such work will
> arrive separately at a later time.
> 
> As of now javac knows only about primitive class and their reference projections and
> plain old reference types. There is no notion of ref-val defaultness yet, Nor a way
> to declare them.

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.

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.

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;

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

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