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

allow inline classes to have abstract supers #24

Closed
wants to merge 1 commit into from

Conversation

@hseigel
Copy link
Member

@hseigel hseigel commented Apr 20, 2020

Plesae review this change to allow inline classes to have super classes that are abstract classes. This fix does not test that the abstract classes are 'special'. That will be done in a future release.

Also, fix verification issue with assigning an inline type to an object of one of its super classes.

Tests will be added once javac support for inline types having abstract classes is merged in.

Thanks, Harold


Progress

  • Change must not contain extraneous whitespace

Reviewers

  • Frederic Parain (fparain - Committer)

Download

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

@bridgekeeper
Copy link

@bridgekeeper bridgekeeper bot commented Apr 20, 2020

👋 Welcome back hseigel! 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 openjdk bot commented Apr 20, 2020

@hseigel This change now passes all automated pre-integration checks, type /integrate in a new comment to proceed. After integration, the commit message will be:

allow inline classes to have abstract supers

Reviewed-by: fparain
  • If you would like to add a summary, use the /summary command.
  • To credit additional contributors, use the /contributor command.
  • To add additional solved issues, use the /solves command.

There are currently no new commits on the lworld branch since the last update of the source branch of this PR. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you would like to avoid potential automatic rebasing, specify the current head hash when integrating, like this: /integrate ff1b435ee9ddcca8f7d3bf8c06881e61efd5954d.

➡️ To integrate this PR with the above commit message to the lworld branch, type /integrate in a new comment.

@mlbridge
Copy link

@mlbridge mlbridge bot commented Apr 20, 2020

Webrevs

Copy link
Collaborator

@fparain fparain left a comment

Looks good to me.
Have you already created the follow-up CR to implement the support for inline friendly abstract classes?
Do you have an idea if changes lines 179-181 of verificationTypes.cpp will trigger more class loading then today? If yes, could this become an issue?

Fred

@hseigel
Copy link
Member Author

@hseigel hseigel commented Apr 20, 2020

@hseigel
Copy link
Member Author

@hseigel hseigel commented Apr 20, 2020

/integrate

@openjdk openjdk bot closed this Apr 20, 2020
@openjdk openjdk bot added integrated and removed ready labels Apr 20, 2020
@openjdk
Copy link

@openjdk openjdk bot commented Apr 20, 2020

@hseigel
Pushed as commit e4e93d7.

@openjdk openjdk bot removed the rfr label Apr 20, 2020
@fparain
Copy link
Collaborator

@fparain fparain commented Apr 20, 2020

Harold,

Thank you for the clarifications.

Fred

@mlbridge
Copy link

@mlbridge mlbridge bot commented Apr 20, 2020

Mailing list message from Harold Seigel on valhalla-dev:

Changeset: e4e93d7
Author: Harold Seigel <hseigel at openjdk.org>
Date: 2020-04-20 20:28:33 +0000
URL: https://git.openjdk.java.net/valhalla/commit/e4e93d77

allow inline classes to have abstract supers

Reviewed-by: fparain

! src/hotspot/share/classfile/classFileParser.cpp
! src/hotspot/share/classfile/verificationType.cpp
! test/hotspot/jtreg/runtime/valhalla/valuetypes/classfileparser/BadValueTypes.java

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