Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 5 commits into from

Conversation

sadayapalam
Copy link
Collaborator

@sadayapalam sadayapalam commented Apr 30, 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) {
    }
}

Progress

  • Change must not contain extraneous whitespace

Issue

  • JDK-8237072: [lworld] Add support for denoting and deriving the reference projection

Reviewers

  • JimLaskey (no known github.com user name / role)

Download

$ git fetch https://git.openjdk.java.net/valhalla pull/32/head:pull/32
$ git checkout pull/32

@bridgekeeper
Copy link

bridgekeeper bot commented Apr 30, 2020

👋 Welcome back sadayapalam! A progress list of the required criteria for merging this PR into lworld will be added to the body of your pull request.

@openjdk
Copy link

openjdk bot commented Apr 30, 2020

@sadayapalam This change is no longer ready for integration - check the PR body for details.

@mlbridge
Copy link

mlbridge bot commented Apr 30, 2020

Webrevs

@JimLaskey
Copy link
Member

JimLaskey commented Apr 30, 2020

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java:546
    `outermost' - mix of quotes.
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java:1319
    `other' projection: If `this' - mix of quotes.
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java:1669 
     // TODO: TYP?, CLINT? - are you tracking?
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java:1672
    I haven't seen enough Valhalla code to fully comment but 'r' and 'v' prefixes for 'ref' and 'val' might be too subtle. One lower case letter in an overloaded variable name is visually harder to discern. (Brevity is not the next readers friend.)
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java:1688
    I think this.flatname.append('$', this.name.table.names.ref); should be broken out into two? support methods.
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java: 1715
    The `other' projection: If `this' - mix of quotes.
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java:1805
   Just wonder if this common projection pattern (p ? p.x : null) could be broken out into support methods.
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java:1967
    The `other' projection: If `this' - mix of quotes (hate when cut and paste propagates the problem.)
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Type.java:1027
    `other' projection: If `this' - mix of quotes
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java:1048
   What if isValue(es) and !isValue(et)? Doesn't really follow the pattern of the old code.
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java:1234
   Same question as above.
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java:1627
   Tracking unspecified behaviour?
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java:2194
    Comment indent :-)
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java:4074
    Naively: What happens if rs.resolveIdent does raise an exception? sym undefined? exception propagated?
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java:4197
    Similarly to above.
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java:901
    Tracking tolerate Value.class for now?
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java:420
    Is there a plan to handle projection conversation without modifying the tree?

Copy link
Collaborator

@mcimadamore mcimadamore left a comment

Overall this looks very good. Note that changes to dot apply cleanly on latest lworld branch, so you might need some rebasing.

I'm impressed at how much we could get done with relatively small set of changes in the core classes. As discussed offline, my mine gripe with the proposed implementation approach is that, I think, it distorts the user model view a little. In my mental model:

  • there is ONE class in the langauge; the class might be inline or not. If it's inline, might be ref-default or not.
  • Every type defines a reference projection. If a type is a type that comes from an ordinary class, then the reference projection of that type is the type itself.
  • inline classes do not give birth to a single type (like normal classes do - e.g. think of j.l.String). But they can give birth to two distinct type (pretty much like a generic class can give birth to an infinite set of types)
  • to help translation, a single inline class will be translated into a ref/val classes pair - which means a single inline class will give rise to two runtime types

In other words, I think that appealing with symmetry with the generic use case, where a single declared class give rise to many types might be cleaner in the long run than having two declared classes (as in the proposed implementation).

This choice surfaces up in a bunch of places where we have relationship which are seemingly defined on symbols (e.g. Symbol::outermostClass, and friends) in which we have to always ask first whether the class symbol we are dealing with is a reference class symbol, and, if not, go back to that. This choice also backfires when it comes to keep the members of the two fictional symbols in sync - so there's a number of places where we have to update the scope of one class in the same way we update the scope of the other class.

Note: all these things are not blockers - but to me they might be signs that some alternate implementation might be possible.

The current approach will require adjustments in places where symbols are expected, but something like V.ref is found - for instance I put together this fictional example:

package a;

import a.V.ref;

inline class V {
   int x = 0;
}

and I got this:

error: import requires canonical name for V$ref
import a.V.ref;
          ^

which seems a bit confusing (more generally I note that the compiler is often emitting V$ref and V$val which should be normalized to the syntax used by developers - e.g. dropping the dollar sign).

I'm afraid that when the time will come and we will start looking at the annotation processing machinery, the choice of having two symbols (hence two Elements) for the val and ref projection might become untenable (since at that point I believe the annotation processor user would expect to get two different TypeMirrors for the projections which point to the same underlying declared Element). Since Element is 1-1 with Symbol, this might become a problem.

On a different topic, I noted something on name resolution:

inline class V {
   int y = 52;

   static class ref {
       int x;
   }

   void m(V.ref vref) {
       vref.x = 12;
   }
}

Which gives:

error: cannot find symbol
       vref.x = 12;
           ^
  symbol:   variable x
  location: variable vref of type V$ref

In other words, the compiler is always resolving .ref to a projection - which is fine for now, but there's a deeper question as to whether we're ok with this behavior and if we want to even allow defining a nested class called ref or val inside an inline class.

Another thing I noted when playing with ref projection is that we don't seem to be getting NPE at runtime when passing nulls back and forth:

static inline class V {
    int y = 52;
}

static void m(V.ref vr) {
    V v = vr;
}

public static void main(String[] args) {
     m(null);
}

This compiles and runs correctly, while I'd expect an NPE to be thrown inside m. I've inspected the bytecode:

         0: aload_0
         1: checkcast     #7                  // class "QV;"
         4: astore_1
         5: return

So, this might be an issue with the VM not doing the right thing with the cast (e.g. not ruling nulls out) - and ultimately related to this discussion:

https://mail.openjdk.java.net/pipermail/valhalla-spec-experts/2020-April/001288.html

We should check as to whether this behavior is expected, and if it should be rectified; or whether we should add an NPE check on our side.

This is all I have for now.

@fparain
Copy link
Collaborator

fparain commented Apr 30, 2020

I've tried to reproduce the checkcast issue mentioned by Mauricio with this code:

public class Test2 {
    static inline class V {
	int y = 52;
    }

    static void m(V.ref vr) {
	V v = vr;
    }

    public static void main(String[] args) {
	m(null);
    }
}

But I got an NPE where it was expected:

$ ./build/pull32/jdk/bin/java Test2
Exception in thread "main" java.lang.NullPointerException
	at Test2.m(Test2.java:7)
	at Test2.main(Test2.java:11)

@mcimadamore
Copy link
Collaborator

mcimadamore commented May 1, 2020

But I got an NPE where it was expected:

My bad - I realized that the test I put together was only compiling and not executing - sorry for the noise.

@mcimadamore
Copy link
Collaborator

mcimadamore commented May 1, 2020

// 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

For this one, I think something that an approach that could be discussed would be to consider the method reference V::m, where V is an inline class, in the same way as we consider List::add. That is, we have class name::member name.

In such cases we infer the right parameterization from the context; here it would be nice if we could infer the right val/ref from the context too.

@mcimadamore
Copy link
Collaborator

mcimadamore commented May 1, 2020

Another odd test:

static inline class V  {
        int y = 52;

        class Bar { }
        static class Baz { }
    }

    public static void main(String[] args) {
        new V.ref().new Bar();
        new V().new Bar();
        V.Baz baz1; 
        V.ref.Baz baz2;
    }

This gives:

error: cannot find symbol
        new V.ref().new Bar();
                        ^
  symbol:   class Bar
  location: class V$ref
error: cannot find symbol
        V.ref.Baz baz2;
             ^
  symbol:   class Baz
  location: class V$ref
2 errors

Now, I don't know what's the status of inner classes and values - but this seems to suggest that member types are not copied from one scope to another (since the nested names are only resolved from the val projection.

@sadayapalam
Copy link
Collaborator Author

sadayapalam commented May 1, 2020

Overall this looks very good. Note that changes to dot apply cleanly on latest lworld branch, so you might need some rebasing.

Thanks for the review, Maurizio!

(I will look into rebasing, At the time I raised the PR request it was even with openjdk/valhalla)

I'm impressed at how much we could get done with relatively small set of changes in the core classes. As discussed offline, my mine gripe with the proposed implementation approach is that, I think, it distorts the user model view a little. In my mental model:

[...]

  • to help translation, a single inline class will be translated into a ref/val classes pair - which means a single inline class will give rise to two runtime types

In other words, I think that appealing with symmetry with the generic use case, where a single declared class give rise to many types might be cleaner in the long run than having two declared classes (as in the proposed implementation).

I agree this suggestion if it is implementable, has promising upside potential to make the code relatively simpler.

I chose the present approach because:

  • some of the state that (naturally) today belong in the class symbol abstraction needs to be distinct between the two projections. (name, flags, type ...)

  • Various parts of javac code compare tsyms and assume the types are same if they are same (on the non-parameterized type path)

  • there are places where we operates only on symbols and it is not clear we will unambiguously know the symbol ownership in those places.

These may all be solvable or worked around satisfactorily.

I agree that upside potential is promising enough to explore the presently unknown downside potential - JDK-8244227 is raised to explore that, to prototype it so we can make an informed call.

This choice surfaces up in a bunch of places where we have relationship which are seemingly defined on symbols (e.g. Symbol::outermostClass, and friends) in which we have to always ask first whether the class symbol we are dealing with is a reference class symbol, and, if not, go back to that. This choice also backfires when it comes to keep the members of the two fictional symbols in sync - so there's a number of places where we have to update the scope of one class in the same way we update the scope of the other class.

I disagree with the vocabulary. It is not backfiring! It is additional work we knowingly sign up for :) But yes, it would be good if we don't have to.

I must mention here that the LW2 implementation actually used two ClassSymbol shells that shared the members_field. I had to opt for a deeper copy to solve some problem I ran into which I am unable to recollect at my advanced geriatric stage :)

Note: all these things are not blockers - but to me they might be signs that some alternate implementation might be possible.

The current approach will require adjustments in places where symbols are expected, but something like V.ref is found - for instance I put together this fictional example:

package a;

import a.V.ref;

inline class V {
   int x = 0;
}

and I got this:

error: import requires canonical name for V$ref
import a.V.ref;
          ^

which seems a bit confusing (more generally I note that the compiler is often emitting V$ref and V$val which should be normalized to the syntax used by developers - e.g. dropping the dollar sign).

I have added this case to JDK-8244229 Thanks!

I'm afraid that when the time will come and we will start looking at the annotation processing machinery, the choice of having two symbols (hence two Elements) for the val and ref projection might become untenable (since at that point I believe the annotation processor user would expect to get two different TypeMirrors for the projections which point to the same underlying declared Element). Since Element is 1-1 with Symbol, this might become a problem.

Hmm. As discussed offline, we may have to use some smoke and mirrors anyways to make them appear as one symbol when dealing with binary classes from classpath.

On a different topic, I noted something on name resolution:

inline class V {
   int y = 52;

   static class ref {
       int x;
   }

   void m(V.ref vref) {
       vref.x = 12;
   }
}

Which gives:

error: cannot find symbol
       vref.x = 12;
           ^
  symbol:   variable x
  location: variable vref of type V$ref

In other words, the compiler is always resolving .ref to a projection - which is fine for now, but there's a deeper question as to whether we're ok with this behavior and if we want to even allow defining a nested class called ref or val inside an inline class.

I have added this test case to JDK-8244229 Thanks!

@sadayapalam
Copy link
Collaborator Author

sadayapalam commented May 1, 2020

Another odd test:

static inline class V  {
        int y = 52;

        class Bar { }
        static class Baz { }
    }

    public static void main(String[] args) {
        new V.ref().new Bar();
        new V().new Bar();
        V.Baz baz1; 
        V.ref.Baz baz2;
    }

This gives:

error: cannot find symbol
        new V.ref().new Bar();
                        ^
  symbol:   class Bar
  location: class V$ref
error: cannot find symbol
        V.ref.Baz baz2;
             ^
  symbol:   class Baz
  location: class V$ref
2 errors

This is acknowledged as a known TODO in
com.sun.tools.javac.code.Symbol.ClassSymbol#referenceProjection

I have raised JDK-8244233 to cover it,

@sadayapalam
Copy link
Collaborator Author

sadayapalam commented May 4, 2020

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java:546
    `outermost' - mix of quotes.
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java:1319
    `other' projection: If `this' - mix of quotes.
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java: 1715
    The `other' projection: If `this' - mix of quotes.
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java:1967
    The `other' projection: If `this' - mix of quotes (hate when cut and paste propagates the problem.)
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Type.java:1027
    `other' projection: If `this' - mix of quotes

I have cleaned up these, but this is a common coding/commenting convention in Javac. See that in the same files Symbol.java & Type.java, there are about 20 other unrelated places where quotes in this `style' exist :)

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java:1669
// TODO: TYP?, CLINT? - are you tracking?

Yes: JDK-8244233

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java:1672
I haven't seen enough Valhalla code to fully comment but 'r' and 'v' prefixes for 'ref' and 'val' might be too subtle. One lower case letter in an overloaded variable name is visually harder to discern. (Brevity is not the next readers friend.)

Agreed it is poor choice. Fixed by expanding to ref* and val*

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java:1688
I think this.flatname.append('$', this.name.table.names.ref); should be broken out into two? support methods.
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Symbol.java:1805
Just wonder if this common projection pattern (p ? p.x : null) could be broken out into support methods.

I have left these as is for now, as it is not clear what exactly is being suggested. If the original code is not clear enough, do outline what you have in mind. TIA

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java:1048
What if isValue(es) and !isValue(et)? Doesn't really follow the pattern of the old code.
src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java:1234
Same question as above.

Basically, we are checking here if T[] <: S[]. This is true if T.ref <: S. Example of such a check being is Point [] <: Object []

If the polarity is reversed and we are checking if Object[] <: Point [], we would come up with
es = Point
et = Object

and return isSubtype(object, Point)

which will return false which is indeed the right answer for if Object[] <: Point []

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java:1627
Tracking unspecified behaviour?

Two clarifications: (1) there is no "behavior" here. it is only a comment I have left in for our reference as we are going to hit it the moment we are going to add support for inline type arguments. (2) And because we are going to hit it most definitely as soon as we start working on generics support that there is no ticket for that per se.

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java:2194
Comment indent :-)

Fixed.

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java:4074
Naively: What happens if rs.resolveIdent does raise an exception? sym undefined? exception propagated?
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java:4197
Similarly to above.

The top level exception handling really happens in com.sun.tools.javac.main.Main#compile(java.lang.String[], com.sun.tools.javac.util.Context)
in the various catch blocks between lines 330-356.

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java:901
Tracking tolerate Value.class for now?

I have fixed the comment to just say: "tolerate Value.class" dropping the "for now" - The comment can be confusing. We should always tolerate and not report error on Value.class - thanks for catching this. (the "for now" originally meant "may be there are other such cases we will discover we may have need to support also" but we have't found anything else that must be tolerated)

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java:420
Is there a plan to handle projection conversation without modifying the tree?

No, this may not be doable - it is the intrinsic/organic cost of saying inline types are islands (i.e they have no supertypes of any kind in the lang model including jlO and any declared super types). The best that can be done is to ensure (a) the swicheroo happens for the narrowest window needed (b) the tree is restored in a finally block

Many many thanks for the review Jim!

@sadayapalam
Copy link
Collaborator Author

sadayapalam commented May 5, 2020

/integrate

@openjdk openjdk bot closed this May 5, 2020
@openjdk openjdk bot added integrated and removed ready rfr labels May 5, 2020
@openjdk
Copy link

openjdk bot commented May 5, 2020

@sadayapalam The following commits have been pushed to lworld since your change was applied:

  • 6742fab: 8244339: [lworld] JIT support for inline types with abstract class supertypes

Your commit was automatically rebased without conflicts.

Pushed as commit a3a846d.

@openjdk
Copy link

openjdk bot commented May 5, 2020

@sadayapalam this pull request can not be integrated into lworld due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout JDK-8237072
git fetch https://git.openjdk.java.net/valhalla lworld
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge lworld"
git push

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants