jdk
jdk copied to clipboard
8342868: Errors related to unused code on Windows after 8339120 in core libs
After 8339120, gcc began catching many different instances of unused code in the Windows specific codebase. Some of these seem to be bugs. I've taken the effort to mark out all the relevant globals and locals that trigger the unused warnings and addressed all of them by commenting out the code as appropriate. I am confident that in many cases this simplistic approach of commenting out code does not fix the underlying issue, and the warning actually found a bug that should be fixed. In these instances, I will be aiming to fix these bugs with help from reviewers, so I recommend anyone reviewing who knows more about the code than I do to see whether there is indeed a bug that needs fixing in a different way than what I did
Progress
- [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
- [x] Change must not contain extraneous whitespace
- [x] Commit message must refer to an issue
Issue
- JDK-8342868: Errors related to unused code on Windows after 8339120 in core libs (Bug - P4)
Reviewing
Using git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/21654/head:pull/21654
$ git checkout pull/21654
Update a local copy of the PR:
$ git checkout pull/21654
$ git pull https://git.openjdk.org/jdk.git pull/21654/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 21654
View PR using the GUI difftool:
$ git pr show -t 21654
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/21654.diff
Webrev
:wave: Welcome back jwaters! 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.
@TheShermanTanker 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:
8342868: Errors related to unused code on Windows after 8339120 in core libs
Reviewed-by: naoto, jlu
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 3566 new commits pushed to the master branch:
- 08b1fa4cb39e56497052e3ee13e679c8734cf7c5: 8359972: Problem list TestStaticCallStub until JDK-8359963 is fixed
- 23e1e2ff4a4a75ec268c7925fb98d6b96a01bbcf: 8359180: Apply java.io.Serial annotations in java.instrument
- 5a62e99523904e89caf561d4c1068c1565a97450: 8357686: Remove unnecessary Map.get from AWTAutoShutdown.unregisterPeer
- ... and 3563 more: https://git.openjdk.org/jdk/compare/07f550b85a3910edd28d8761e2adfb8d6a1352f6...master
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 master branch, type /integrate in a new comment.
@TheShermanTanker The following labels will be automatically applied to this pull request:
core-libsi18nnet
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing lists. If you would like to change these labels, use the /label pull request command.
Bumping, please advise on whether the fixes are correct or not
Thanks for the reviews! I would delete them entirely, but I don't want to invalidate the existing reviews and force you guys to re-approve again :(
@TheShermanTanker 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!
Keep it open Skara, I'm waiting for the other Pull Requests to be approved so I can do them all at once
@TheShermanTanker 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!
Stop asking, I still need awt and accessibility to be approved :(
@TheShermanTanker 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!
Keep open please
@TheShermanTanker 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!
Keep open please
@TheShermanTanker 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!
Keep it open please. Life is not being kind to me at the moment, I apologize for the delays
@TheShermanTanker 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!
Stay open
@TheShermanTanker 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 issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
/touch
@TheShermanTanker The pull request is being re-evaluated and the inactivity timeout has been reset.
@TheShermanTanker 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 issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!
/touch
@TheShermanTanker The pull request is being re-evaluated and the inactivity timeout has been reset.
Hello @TheShermanTanker, I see that this PR is marked as ready for integration since several months. But there's also your comment which says:
I still need awt and accessibility to be approved
I don't see any client area changes in this PR. Is there anything that's needed to be done to move forward with integrating this? I don't mean that you should integrate it "now" - I see that this branch hasn't been merged against master branch for several months, so that would be the first thing to do.
Sorry for waiting so long. It's become clear that I won't be able to get awt and accessibility up to speed for a long time, so I will go ahead with this one first
/integrate
Going to push as commit bbc5c98b144014a0423d666f74c4a5a15b08a7c2.
Since your change was applied there have been 3762 commits pushed to the master branch:
- 563a3358f6f1ecff816318cbb32376487365c1fa: 8342682: Errors related to unused code on Windows after 8339120 in dt_shmem jdwp security and jpackage
- ec7c6be6a9e84c8cd2077fea07930592ddd13669: 8359388: Stricter checking for cipher transformations
- 197fde5363e314de7cd6090ecd77521f3a90c56d: 8361303: L10n comment for javac.opt.Xlint.desc.synchronization in javac.properties
- ... and 3759 more: https://git.openjdk.org/jdk/compare/07f550b85a3910edd28d8761e2adfb8d6a1352f6...master
Your commit was automatically rebased without conflicts.
@TheShermanTanker Pushed as commit bbc5c98b144014a0423d666f74c4a5a15b08a7c2.
:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.
@TheShermanTanker the commented out code really should have been deleted, not just left commented out. Please file anpther JBS issue to have this cleaned up so it is not forgotten. Thanks.