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

Backport 8259922: MethodHandles.collectArguments does not throw IAE #560

Merged
merged 1 commit into from
Apr 14, 2022

Conversation

babsingh
Copy link
Member

@babsingh
Copy link
Member Author

fyi @pshipton Identical to ibmruntimes/openj9-openjdk-jdk16#29. For supporting OJDK MHs.

@pshipton
Copy link
Member

ibmruntimes/openj9-openjdk-jdk16@f6e2b25af55d44 also added a test MethodHandlesCollectArgsTest.java, I think we want that as well?

@pshipton
Copy link
Member

Maybe we'd have to exclude the test until we switch?

@pshipton
Copy link
Member

We're backporting an OpenJDK change, so we should backport the Oracle copyright, not add an IBM copyright.

@pshipton
Copy link
Member

Why do we need to backport this if OpenJDK hasn't bothered to backport to 8, 11?

@babsingh
Copy link
Member Author

babsingh commented Apr 14, 2022

Why do we need to backport this if OpenJDK hasn't bothered to backport to 8, 11?

Because there are tests in OpenJ9, which will fail without this fix, once OJDK MHs are enabled: eclipse-openj9/openj9#11922.

We're backporting an OpenJDK change, so we should backport the Oracle copyright, not add an IBM copyright.

Updated Oracle copyright and removed IBM copyright.

ibmruntimes/openj9-openjdk-jdk16@f6e2b25af55d44 also added a test MethodHandlesCollectArgsTest.java, I think we want that as well?

This test won't work with JDK8 because MethodHandles.empty is not available in JDK8. We can either update the test or rely on the existing OJ9 tests for coverage in JDK8. For JDK11, I have included the test in ibmruntimes/openj9-openjdk-jdk11#502.

Maybe we'd have to exclude the test until we switch?

We do not need to exclude the test because the test should correctly work with OJ9 MHs

@pshipton
Copy link
Member

pshipton commented Apr 14, 2022

jenkins test sanity,extended alinux jdk8

@pshipton
Copy link
Member

Oops, PR testing isn't useful here since the class being modified isn't used atm. Luckily it failed anyway because I got the platform name wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants