-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
8302865: Illegal bytecode for break from if with instanceof pattern matching condition #14517
Conversation
👋 Welcome back jlahoda! A progress list of the required criteria for merging this PR into |
/backport jdk21 |
@lahodaj Backport for repo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks sensible
@lahodaj This change now passes all automated pre-integration checks. ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details. After integration, the commit message for the final commit will be:
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 122 new commits pushed to the
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details. ➡️ To integrate this PR with the above commit message to the |
/integrate |
Going to push as commit a15db1a.
Your commit was automatically rebased without conflicts. |
Creating backport for repo openjdk/jdk21 on branch master /backport openjdk/jdk21 master |
@openjdk[bot] @lahodaj the backport was successfully created on the branch lahodaj-backport-a15db1a5 in my personal fork of openjdk/jdk21. To create a pull request with this backport targeting openjdk/jdk21:master, just click the following link: The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:
If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk21:
|
Please see the CSR for the exact wording of the specification change:
https://bugs.openjdk.org/browse/JDK-8310016
There are two changes to flow scoping under this PR:
a) for code like:
s
won't be in the scope forSystem.err.println
anymore (as that leads to an invalid bytecode anyway). I was experimenting with several possibilities to fix this, and the final proposal is (when we are about to add some bindings to the scope), look for an immediately enclosing labelled statement(s), and there's some, look for a break to this statement(s).b) for code like:
javac implements a strict interpretation of the specification, which does not include
s
in the scope for theSystem.err.println
(due to thebreak
out of thewhile
). The specification is clarified, so that only breaks to the loop are considered (together with breaks to the enclosing labelled statements), so flow scoping handling for loops is adjusted to only consider breaks to the loop.Progress
Issues
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/14517/head:pull/14517
$ git checkout pull/14517
Update a local copy of the PR:
$ git checkout pull/14517
$ git pull https://git.openjdk.org/jdk.git pull/14517/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 14517
View PR using the GUI difftool:
$ git pr show -t 14517
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/14517.diff
Webrev
Link to Webrev Comment