asmtools icon indicating copy to clipboard operation
asmtools copied to clipboard

7903806: Enhance jasm/jdis to support value classes and objects

Open dansmithcode opened this issue 1 year ago • 3 comments
trafficstars

Some changes to align Valhalla support in asmtools with the latest design for JEP 401.

Latest JVM specification can be found here: https://cr.openjdk.org/~dlsmith/jep401/jep401-20240624/specs/value-objects-jvms.html

I originally made these changes on a custom copy of asmtools, so this was mostly a matter of reviewing my original patch and making updates. There have been some changes, though, and it's quite possible I've missed something.

Some things to be aware of:

  • There are no longer any new opcodes or any changes to special methods (like <init>)
  • The ACC_IDENTITY flag, which replaces ACC_VALUE, is aliased with ACC_SUPER—this will create some awkwardness until the feature is final. In the mean time, I've had jdis just print super; I imagine someone more experienced with this code might find a clean way to print the right thing based on the version number
  • That said, I'm not sure what version number these features will be associated with—spec currently says 67.65535 (i.e., Java 23), but obviously that's going to change
  • The Preload attribute, which contained a list of CONSTANT_Classes, has been replaced with LoadableDescriptors, which contains a list of Utf8 descriptor strings
  • ACC_PERMITTED_SUBCLASSES and ACC_PRIMITIVE have been abandoned, can be deleted
  • The ACC_STRICT flag is now allowed on fields; the spec has some constraints on its use (e.g., the field must be final), but I didn't include those checks, because we have plans to expand the feature to other kinds of fields
  • I got this to compile, but I haven't tested anything. Not sure how to run the local tests; I'll do some targeted testing just to make sure the changes basically work.

Progress

  • [x] Change must not contain extraneous whitespace

Issue

  • CODETOOLS-7903806: Enhance jasm/jdis to support value classes and objects (Enhancement - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/asmtools.git pull/73/head:pull/73
$ git checkout pull/73

Update a local copy of the PR:
$ git checkout pull/73
$ git pull https://git.openjdk.org/asmtools.git pull/73/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 73

View PR using the GUI difftool:
$ git pr show -t 73

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/asmtools/pull/73.diff

Webrev

Link to Webrev Comment

dansmithcode avatar Aug 31 '24 00:08 dansmithcode

:wave: Welcome back dlsmith! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

bridgekeeper[bot] avatar Aug 31 '24 00:08 bridgekeeper[bot]

@dansmithcode This change now passes all automated pre-integration checks.

After integration, the commit message for the final commit will be:

7903806: Enhance jasm/jdis to support value classes and objects

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

openjdk[bot] avatar Aug 31 '24 00:08 openjdk[bot]

Webrevs

mlbridge[bot] avatar Aug 31 '24 00:08 mlbridge[bot]

@dansmithcode This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper[bot] avatar Sep 28 '24 06:09 bridgekeeper[bot]

@dansmithcode This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

bridgekeeper[bot] avatar Oct 26 '24 08:10 bridgekeeper[bot]