[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