-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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
8283700: Add final or sealed modifier to appropriate java.awt API classes #7998
Conversation
/csr |
👋 Welcome back prr! A progress list of the required criteria for merging this PR into |
@prrace has indicated that a compatibility and specification (CSR) request is needed for this pull request. |
I wonder do we have some javac checks which may show some warnings related to these updated classes like not all subclasses are covered by the switch/unnecessary default branch/etc? |
@mrserb, you mean to detect that you can not add a new subclass without breaking the source backward compatibility. |
I think Sergey is asking how javac identifies classes that are sub-classing the sealed class but not listed in the permits clause ? |
If a sealed class does not have a permits clause, the allowed subclasses must be in the same file as the class in question, e.g. nested classes that were also subclasses or auxiliary classes. This is described in the JEP. |
Thank you for clarification |
The CSR is now ready for review : https://bugs.openjdk.java.net/browse/JDK-8283813 |
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.
Suitable classes across the JDK are being retro-fitted to be sealed
How does this "suitability" check done? Are these exhaustive or more awt classes will be added?
Do we need to do the same for swing classes too?
I wrote an annotation processor to look for candidates to make sealed (public classes with no public or protected constructors and at least one package access constructor). Phil has filed bugs for swing (JDK-8283706) and other client areas. |
@prrace 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 36 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 e85fa2f.
Your commit was automatically rebased without conflicts. |
JDK 17 delivered JEP 409: Sealed Classes : https://openjdk.java.net/jeps/409
In essence this JEP allows a class to limit which classes can subclass it.
If a class has no subclasses and cannot be subclassed outside the JDK it can also be made final.
Suitable classes across the JDK are being retro-fitted to be sealed (or made final) where it can be done compatibly.
This fix takes care of several AWT classes which were identified.
If later we need to remove sealed or make a class non-final that will also be a compatible change,
so we aren't locking ourselves into this.
All our automated regression tests - plus JCK tests have been run.
This will require a CSR
Progress
Issues
Reviewers
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/7998/head:pull/7998
$ git checkout pull/7998
Update a local copy of the PR:
$ git checkout pull/7998
$ git pull https://git.openjdk.java.net/jdk pull/7998/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 7998
View PR using the GUI difftool:
$ git pr show -t 7998
Using diff file
Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/7998.diff