8200473: cond? true : false patterns in openjdk#30486
8200473: cond? true : false patterns in openjdk#30486shunohta wants to merge 3 commits intoopenjdk:masterfrom
Conversation
|
Hi @shunohta, welcome to this OpenJDK project and thanks for contributing! We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user shunohta" as summary for the issue. If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing |
|
❗ This change is not yet ready to be integrated. |
|
@shunohta The following labels will be automatically applied to this pull request:
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. |
47efc5b to
2a4586d
Compare
2a4586d to
615e8df
Compare
|
@shunohta, the OpenJDK Generative AI policy https://openjdk.org/legal/ai doesn't allow for content generated from LLM to be accepted as a contribution. I see that in your PR description you confirm (through the checkbox) that the content in this PR wasn't generated using LLM, yet the PR description and the changes appear to contradict the claim. These changes cannot be accepted in the current form. Please refer to the https://openjdk.org/legal/ai document for what's allowed and what's not allowed as a contribution. |
|
I acknowledge that I used an LLM to generate the descriptions and identify patterns, but I manually reviewed all changes. |
|
Thank you @shunohta. Contributions to the OpenJDK project are always welcome including fixes for starter issues like this one (the project has accepted several such fixes over the years even before Generative AI contributions became common). So feel free to continue to contribute to the project. Apart from the new Generative AI guidelines, the OpenJDK dev guide https://openjdk.org/guide/ has some general guidelines which provide help on how to contribute. |
Problem:
<cond> ? true : falseand<cond> ? false : trueexist in the codebaseboolean, the ternary operator is unnecessaryAnalysis:
Redundant patterns by module
src/— 74 occurrences: java.xml (31), java.desktop (20), java.base (8), others (15)test/— 138 occurrences: test/hotspot (76), test/jdk (52), others (10)Solution:
java.base, the core module of the JDKChanges in java.base (8 files)
share/classes/sun/net/www/protocol/http/NTLMAuthenticationProxy.javashare/classes/sun/security/provider/X509Factory.javashare/classes/sun/text/DictionaryBasedBreakIterator.javashare/classes/jdk/internal/event/SocketReadEvent.javashare/classes/java/text/CollationElementIterator.javaunix/classes/sun/nio/fs/UnixPath.javashare/classes/sun/security/validator/Validator.javashare/classes/jdk/internal/icu/lang/UCharacter.javaTesting:
This PR performs semantically equivalent cleanups with no behavioral changes. I have verified that all relevant existing
jtregtests pass.Test mapping
NTLMAuthenticationProxy.javatest/jdk/sun/net/www/protocol/http/NoNTLM.javaX509Factory.javatest/jdk/sun/security/provider/X509Factory/BadPem.javaDictionaryBasedBreakIterator.javatest/jdk/java/text/BreakIterator/BreakIteratorTest.javaetc.SocketReadEvent.javatest/jdk/jdk/jfr/event/io/TestSocketEvents.javaetc.CollationElementIterator.javatest/jdk/java/text/Collator/IteratorTest.javaetc.UnixPath.javatest/jdk/java/nio/file/Path/PathOps.javaetc.Validator.javatest/jdk/sun/security/validator/EndEntityExtensionCheck.javaetc.UCharacter.javaNotes on verification
While verifying the changes against the existing tests, I noticed the following points and addressed them as part of this PR:
startsWith(), I added test cases toPathOps.javato cover edge cases like"//"or"///"(whichnormalizeAndCheck()normalizes to/).typeis aStringfield, the original code used==for reference comparison. I have updated this to use!Objects.equals()for a proper value comparison.Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/30486/head:pull/30486$ git checkout pull/30486Update a local copy of the PR:
$ git checkout pull/30486$ git pull https://git.openjdk.org/jdk.git pull/30486/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 30486View PR using the GUI difftool:
$ git pr show -t 30486Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/30486.diff
Using Webrev
Link to Webrev Comment