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

8294982: Implementation of Classfile API #10982

Closed
wants to merge 187 commits into from

Conversation

asotona
Copy link
Member

@asotona asotona commented Nov 4, 2022

This is root pull request with Classfile API implementation, tests and benchmarks initial drop into JDK.

Following pull requests consolidating JDK class files parsing, generating, and transforming (JDK-8294957) will chain to this one.

Classfile API development is tracked at:
https://github.com/openjdk/jdk-sandbox/tree/classfile-api-branch

Development branch of consolidated JDK class files parsing, generating, and transforming is at:
https://github.com/openjdk/jdk-sandbox/tree/classfile-api-dev-branch

Classfile API JEP and online API documentation is also available.

Please take you time to review this non-trivial JDK addition.

Thank you,
Adam


Progress

  • Change must not contain extraneous whitespace
  • Commit message must refer to an issue
  • Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

Reviewers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 10982

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10982.diff

asotona and others added 30 commits June 10, 2022 14:20
…lang.reflect.AccesFlag in Classfile API and tests
…andleInfo

implemented RebuildingTransformation and added to Transforms and CorpusTest
reduced CorpusTestHelper output and adjusted TEST.properties
MethodParameterInfo name parameter changed to Optional
added MethodParameterInfo::ofParameter(Optional<String>,int)
implemented TemporaryConstantPool::stringEntry
adjusted BoundAttribute and RebuildingTransformation test helper
EnclosingMethodAttribute factory method changed to accept Optionals
added EnclosingMethodAttribute::of(ClassDesc,Optional<String>,Optional<MethodTypeDesc>)
added EnclosingMethodAttribute accessor methods
InnerClassInfo all factory methods changed to accept Optionals
added NestHostAttribute::of(ClassDesc)
added SourceIDAttribute::of(String)
changes reflected in BoundAttribute and RebuildTransformation test helper
* added TypeAnnotation factory methods accepting ClassDesc and AnnotationElement...
AnnotationValue.OfConstant sub-classed to allow switch pattern matching
RebuildingTransformation test helper adjusted

* added TypeAnnotation.TargetInfo factory methods with validity checking for multi-target types
adjusted RebuildTransformation test helper
…Symbol (openjdk#13)

refactored to FieldModel::fieldTypeSymbol and MethodModel::methodTypeSymbol (openjdk#13)
added round testing of signatures in RebuildTransformation test helper
… a frame type.

 Doing so, make the chop size available to consumers of frames.
Copy link
Contributor

@mcimadamore mcimadamore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done. The API does a great job at mastering the complexity (and, at time, ad-hocness) of the Java bytecode, and expose it in a way that is principled and useful to developers.

@asotona
Copy link
Member Author

asotona commented Mar 9, 2023

/integrate

@openjdk
Copy link

openjdk bot commented Mar 9, 2023

Going to push as commit 4655b79.
Since your change was applied there have been 36 commits pushed to the master branch:

  • 1e9942a: 8303881: Mixed, minor cleanup in jdk.compiler
  • 7e01534: 8303467: Serial: Refactor reference processor
  • 713def0: 8303105: LoopRangeStrideTest fails IR verification on x86
  • 34a9246: 8274264: Not all of G1 young collection verification honors the verification type
  • a7e308a: 8303576: addIdentitiesToKeystore in KeystoreImpl.m needs CFRelease call in early potential CHECK_NULL return
  • dd79410: 8303509: Socket setTrafficClass does not work for IPv4 connections when IPv6 enabled
  • dc523a5: 8300258: C2: vectorization fails on simple ByteBuffer loop
  • 5e232cf: 8303564: C2: "Bad graph detected in build_loop_late" after a CMove is wrongly split thru phi
  • 8cfd74f: 8302976: C2 intrinsification of Float.floatToFloat16 and Float.float16ToFloat yields different result than the interpreter
  • 02875e7: 8171156: Class java.util.LocaleISOData has outdated information for country Code NP
  • ... and 26 more: https://git.openjdk.org/jdk/compare/45a616a891e4a4b0e77b1f2fa040522f4a99d172...master

Your commit was automatically rebased without conflicts.

@openjdk openjdk bot added the integrated Pull request has been integrated label Mar 9, 2023
@openjdk openjdk bot closed this Mar 9, 2023
@openjdk openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Mar 9, 2023
@openjdk
Copy link

openjdk bot commented Mar 9, 2023

@asotona Pushed as commit 4655b79.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

@Glavo
Copy link
Contributor

Glavo commented Mar 14, 2023

I ported the Classfile API implemented in this PR back to Java 17 and published it to Maven Central as a standalone library: https://github.com/Glavo/classfile

Some of my senior developer friends are trying to replace ASM with it. If I get any feedback, I'll report it to you.

static ArgumentConstantInstruction ofArgument(Opcode op, int value) {
Util.checkKind(op, Opcode.Kind.CONSTANT);
if (op != Opcode.BIPUSH && op != Opcode.SIPUSH)
throw new IllegalArgumentException(String.format("Wrong opcode specified; found %s, expected BIPUSH or SIPUSH", op, op.kind()));
Copy link
Member

@turbanoff turbanoff Jun 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IDEA shows warning here.
Too many arguments for format string (found: 2, expected: 1)

Seems op.kind() parameter is unused in the format string.

And the same issue in jdk.internal.classfile.instruction.ConstantInstruction#ofLoad

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll fix it, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build build-dev@openjdk.org core-libs core-libs-dev@openjdk.org integrated Pull request has been integrated