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

Srikanth Adayapalam sadayapalam at openjdk.java.net
Fri May 1 11:31:38 UTC 2020


> 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) {
>     }
> }

Srikanth Adayapalam has updated the pull request with a new target base due to a merge or a rebase. The incremental
webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits
since the last revision:

 - Rebase + Remove stale golden file.
 - Fix trailing white spaces
 - 8237072: [lworld] Add support for denoting and deriving the reference projection
 - 8237072: [lworld] Add support for denoting and deriving the reference projection

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

Changes:
  - all: https://git.openjdk.java.net/valhalla/pull/32/files
  - new: https://git.openjdk.java.net/valhalla/pull/32/files/af7ce163..e3585a3b

Webrevs:
 - full: https://webrevs.openjdk.java.net/valhalla/32/webrev.01
 - incr: https://webrevs.openjdk.java.net/valhalla/32/webrev.00-01

  Stats: 79 lines in 4 files changed: 28 ins; 46 del; 5 mod
  Patch: https://git.openjdk.java.net/valhalla/pull/32.diff
  Fetch: git fetch https://git.openjdk.java.net/valhalla pull/32/head:pull/32

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


More information about the valhalla-dev mailing list