[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