[lworld] RFR: 8237072: [lworld] Add support for denoting and deriving the reference projection
Srikanth Adayapalam
sadayapalam at openjdk.java.net
Fri May 1 10:58:14 UTC 2020
On Thu, 30 Apr 2020 17:19:23 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/jvm/ClassReader.java line 2605:
>
>> 2604: readClassFileInternal(c);
>> 2605: if (c.isValue()) {
>> 2606: /* http://cr.openjdk.java.net/~briangoetz/valhalla/sov/04-translation.html
>
> This approach leads to very compact code - but in javac-land it's a bit odd that we have to prune away members at this
> late stage. I wonder if some of the pruning could happen earlier (e.g. in TransValues?)
Actually, this code is trying to remove V$ref.class from its owner (the package symbol). The class finder would have
from the class path found V.class and V$ref.class and added them to the known classes. When V.class is completed and we
find that it is a value class, V.class has a "wrong" super type from javac pov. It needs to read V$ref.class and pick
up the "real" super type from there. In that process it needs to remove V$ref.class from the package owner as it not a
top level class at all (very similar to what happens for nested classes)
> test/langtools/tools/javac/valhalla/lworld-values/InferredValueParameterizationTest.java line 2:
>
>> 1: /*
>> 2: * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
>> 3: * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>
> The golden file seems to still be there - even though this is now a positive test
Thanks, will remove it!
-------------
PR: https://git.openjdk.java.net/valhalla/pull/32
More information about the valhalla-dev
mailing list