-
Notifications
You must be signed in to change notification settings - Fork 6.2k
8295715: Minimize disabled warnings in serviceability libs #10786
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
Conversation
|
👋 Welcome back ihse! A progress list of the required criteria for merging this PR into |
plummercj
left a comment
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.
Changes look good.
|
Is there any desire to clean up these warnings? I see back in 2015 there were a bunch of "Resolve disabled warnings for libXXX" bugs that you fixed. Any plans to make another pass. Seems like some of these warnings might be hiding real bugs. |
|
@magicus 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 9 new commits pushed to the
Please see this link for an up-to-date comparison between the source branch of this pull request and the ➡️ To integrate this PR with the above commit message to the |
|
@plummercj I agree that it could very well be that these warnings are signs of real bugs. And yes, I plan to open new bugs requesting for these warnings to be fixed, if possible, if not someone else beats me to it. However, it is mostly out of scope for me to fix, since domain knowledge by the respective component teams are usually required. And the old "Resolve disabled warnings for libXXX" JBS issues were created by me, but not fixed by me (in general). |
|
/integrate |
|
Going to push as commit faa6b66.
Your commit was automatically rebased without conflicts. |
After JDK-8294281, it is now possible to disable warnings for individual files instead for whole libraries. I used this opportunity to go through all disabled warnings in the serviceability native libraries.
Any warnings that were only triggered in a few files were removed from the library as a whole, and changed to be only disabled for those files.
Some warnings didn't trigger in any file anymore, and could just be removed.
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk pull/10786/head:pull/10786$ git checkout pull/10786Update a local copy of the PR:
$ git checkout pull/10786$ git pull https://git.openjdk.org/jdk pull/10786/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 10786View PR using the GUI difftool:
$ git pr show -t 10786Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/10786.diff