[lworld] RFR: 8237072: [lworld] Add support for denoting and deriving the reference projection

Srikanth Adayapalam sadayapalam at openjdk.java.net
Fri May 1 10:50:13 UTC 2020


On Thu, 30 Apr 2020 17:11:08 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Hello Maurizio & Jim,
>> 
>>     May I request you to please review this patch for valhalla inline types support
>> under the new scheme ? TIA.
>> 
>> I.   What does this patch do?
>> II.  What can be skipped in the review?
>> III. Recommended order for review.
>> IV.  Known problems, limitations and deferred tasks.
>> V.   Report to language designers on open problems.
>> 
>> I. What does this patch do:
>> 
>>     - From every inline class declaration V, derive two classes V and V.ref
>>     - These two are subtypes at the VM level, but are related (only) by inline
>>       widening conversion and narrowing conversion at the language level.
>>     - The two types have the same fields and methods with the same shape and
>>       accessibility rules.
>>     - Add source level support for V.ref and V.val notation.
>>     - Get rid of the experimental support for generics over values. Until we
>>       figure out the full story with generics and inlines, carrying along this
>>       experimental support is becoming untenable/unsustainable.
>>     - Get rid of the experimental support added for noncovariant value arrays
>>     - Get rid of LW2's "Nullable Projections" including the V? syntax
>> 
>> II. These files can be skipped as they simply revert change and align
>>     with origin/master: (roll back V and V? nullable projections of LW2)
>> 
>>     JavacParser.java
>>     AttrContext.java
>>     JCTree.java
>>     Pretty.java
>>     Printer.java
>>     RichDiagnosticsFormatter.java
>>     TaskFactory.java
>>     TreeCopier.java
>>     TypePrinter.java
>> 
>> III. Recommended order for review:
>> 
>>     - Type.java, Symbol.java
>> 
>>             Look at the new internal APIs to query/compute various projections.
>>             Every class type, class symbol, method and field support an API
>>             to return the "other" projection i.e its doppleganger in the alternate
>>             universe.
>> 
>>             Most crucially ClassSymbol.referenceProjection()
>> 
>>     - Attr.java:
>> 
>>             Source support for .ref/.val
>> 
>>     - MemberEnter.java:
>>     - TypeEnter.java:
>>     - TransTypes.java:
>> 
>>             Co-evolve V.val and V.ref in lock step (also TransValues.java)
>> 
>>     - TransValues.java:
>> 
>>             Fold all V.ref.member accesses to ((V) V.ref).member access.
>> 
>>     - Resolve.java:
>> 
>>             Changes to make sure method look up works OK in a world where values are islands.
>> 
>>     - Types.java:
>>   
>>             Implement inline widening/narrowing relationship.
>> 
>>     - ProjectionRelationsTest.java:
>> 
>>             Verify all relationships between V and V.ref and V[] and V.ref[]
>> 
>>     - ClassWriter.java:
>>             
>>             Dual class generation scheme with subtyping relationship at the binary/VM level.
>> 
>>     - ClassReader.java:
>> 
>>             Merge the dual classes back, sever the subtyping relationship and make
>>             them operate as a pair of classes that can convert to each other.
>> 
>>     - New tests:
>>     
>>             Most importantly ProjectionRelationsTest.java
>> 
>>     - Other files.
>> 
>>  
>> IV. There are many known issues that have been deliberately deferred to a later iteration:
>> 
>>     - With Brian's consent I am using a simpler translation strategy than what is
>>       outlined in the SoV documents. These are good enough for now, but when
>>       VBC migration is undertaken, will have to enhanced.
>>     - No support for ref-default and val-default classes yet.
>>     - No support for class hierarchy sealing.
>>     - You can't do new V.ref() (V.ref is abstract) although SoV calls for it.
>>     - Handling of .ref and .val is quick and dirty; Need revisit.
>>     - The nest mate related attributes are not fully properly emitted as of now
>>     - Need to verify that attributes from V are not carried over inadvertantly to
>>       V$ref.class
>>     - Class hierarchy walking in a world where inline types are islands calls for
>>       a type switch. I have done this only in places uncovered by existing tests.
>>       We need to go through various utility methods (similar to what is done in
>>       Symbol.java and Resolve.java) to make sure these changes are consistently
>>       applied.
>>     - Diamond inference with value classes crashes now (implementation creates factory
>>       methods, projections are missing and need to be created)
>> 
>> 
>> V. Problems in the interplay of inlines types with generics and inference:
>> 
>>     Removing the experimental support for generics over values results in several
>> constructs that used to compile earlier (albeit only with -XDallowGenericsOverValues)
>> not compiling anymore.
>> 
>>     These expose some issues that feed into the language design. We need to evolve
>> a short term answer (so that the JDK components tests that are impacted don't get
>> blocked) and a long term one (the real solution)
>> 
>> Here is a snippet that captures these problems:
>> import java.util.Arrays;
>> 
>> interface I {}
>> 
>> public inline class X implements I {
>> 
>>     int x = 10;
>> 
>>     void problem_01() {
>>         X [] a1 = new X[0];
>>         X [] a2 = Arrays.copyOf(a1, a2.length, X[].class);
>>       
>> /*
>> error: incompatible types: inference variable T has incompatible bounds
>>         X [] a2 = Arrays.copyOf(a1, a2.length, X[].class);
>>                                          ^
>>     lower bounds: X,Object
>>     lower bounds: X$ref
>>   where T,U are type-variables:
>>     T extends Object declared in method <T,U>copyOf(U[],int,Class<? extends T[]>)
>>     U extends Object declared in method <T,U>copyOf(U[],int,Class<? extends T[]>)
>> 1 error
>> */
>>     }
>> 
>>     void foo(Class<? extends I> p) {}
>> 
>>     void problem_02() {
>>         foo(X.class);
>> /*
>> error: incompatible types: Class<X> cannot be converted to Class<? extends I>
>>         foo(X.class);
>> */
>>     }
>> 
>>     String data() {
>>         return null;
>>     }
>> 
>>     // Problem: 3, we infer a stream of X.ref's causing
>>     // the method reference to not type check.
>>     void unbound_lookup_with_projected_receiver() {
>>         X [] a = new X[0];
>>         Arrays.stream(a).map(X::data).filter(p -> p != null).forEach(System.out::println);
>> /*
>> error: incompatible types: invalid method reference
>>         Arrays.stream(a).map(X::data).filter(p -> p != null).forEach(System.out::println);
>>                              ^
>>     method data in class X cannot be applied to given types
>>       required: no arguments
>>       found:    X$ref
>>       reason: actual and formal argument lists differ in length
>> */
>>     }
>> 
>>     void problem_04() {
>> /*
>>     test/hotspot/jtreg/runtime/valhalla/valuetypes/UnsafeTest.java:125: warning: [removal] putObject(Object,long,Object) in
>>     Unsafe has been deprecated and marked for removal
>>             U.putObject(v, off_o, List.of("Value1", "Value2", "Value3"));
>>              ^
>> /home/srikanth/valhalla/test/valhalla/test/hotspot/jtreg/runtime/valhalla/valuetypes/UnsafeTest.java:127: error: method
>> valueHeaderSize in class Unsafe cannot be applied to given types;
>>             U.putInt(v, off_v + off_i - U.valueHeaderSize(Value2.class), 999);
>>                                          ^
>>   required: Class<V>
>>   found:    Class<Value2>
>>   reason: inference variable V has incompatible bounds
>>     equality constraints: Value2
>>     lower bounds: Object
>>   where V is a type-variable:
>>     V extends Object declared in method <V>valueHeaderSize(Class<V>)
>> */
>>     }
>> 
>>     void problem_05() {
>> /*
>> test/hotspot/jtreg/compiler/valhalla/valuetypes/TestIntrinsics.java:119: error: incomparable types: Class<CAP#1> and
>> Class<MyValue1$ref>
>>         boolean check2 = MyValue1.class.asIndirectType().getSuperclass() == MyValue1.ref.class;
>>                                                                          ^
>>   where CAP#1 is a fresh type-variable:
>>     CAP#1 extends Object super: MyValue1 from capture of ? super MyValue1
>> */
>>     }
>> 
>>     public static void main(String [] args) {
>>     }
>> }
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java line 441:
> 
>> 440:     public Symbol referenceProjection() {
>> 441:         return null;
>> 442:     }
> 
> How much would it help to have this return `this` ? It seems to me that there a lot of `if <value> ...
> referenceProjection()` going around.

It could result in cleaner code. I have this one JDK-8244232 to cover it.

> src/jdk.compiler/share/classes/com/sun/tools/javac/code/Type.java line 1197:
> 
>> 1196:         public Type valueProjection() {
>> 1197:             if (!isReferenceProjection() || projection !=  null)
>> 1198:                 return projection;
> 
> Question here - since you have two symbols, getting a reference projection and value projection shouldn't be as simple
> as doing `tsym.XYZprojection().type` ? Why do we need to create new types here?

FTR, this is to preserve any parameterizations. The value projection of V.ref<String> is V.val<String> and not V.val<T>

> src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java line 1760:
> 
>> 1759:                     if (isValue(t)) {
>> 1760:                         // (s) Value ? == (s) Value.ref
>> 1761:                         t = t.referenceProjection();
> 
> I didn't get these comments

It is saying the predicate is a value type is castable to a type s is the same as the predicate is its reference
projection castable to s.

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

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


More information about the valhalla-dev mailing list