Re: RFR: 8286378: Address possibly lossy conversions in java.base [v3]
On Fri, 13 May 2022 14:18:04 GMT, Roger Riggs wrote: >> Well spotted, I don't think that change was intentionally. > > Ouch; Will fix: > > I took Alan's earlier comment literally: > > "This one can also be changed to: > > to =- (int) TimeUnit.NANOSECONDS.toMillis(adjust);" @RogerRiggs This already got fixed by @jaikiran in [GH‑8693]. [GH‑8693]: https://github.com/openjdk/jdk/pull/8693 - PR: https://git.openjdk.java.net/jdk/pull/8642
Re: RFR: 8286378: Address possibly lossy conversions in java.base [v3]
On Fri, 13 May 2022 05:54:15 GMT, Alan Bateman wrote: >> src/java.base/linux/classes/sun/nio/ch/EPollSelectorImpl.java line 128: >> >>> 126: // timed poll interrupted so need to adjust timeout >>> 127: long adjust = System.nanoTime() - startTime; >>> 128: to =- (int) TimeUnit.NANOSECONDS.toMillis(adjust); >> >> This will now always assign a negative number to `to`. >> >> >> >> `=-` is not a compound assignment, it’s negation followed by a normal >> assignment. > > Well spotted, I don't think that change was intentionally. Ouch; Will fix: I took Alan's earlier comment literally: "This one can also be changed to: to =- (int) TimeUnit.NANOSECONDS.toMillis(adjust);" - PR: https://git.openjdk.java.net/jdk/pull/8642
Re: RFR: 8286378: Address possibly lossy conversions in java.base [v3]
On Fri, 13 May 2022 04:41:03 GMT, ExE Boss wrote: >> Roger Riggs has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Updated copyrights >> Fixed cast style to add a space after cast, (where consistent with file >> style) >> Improved code per review comments in PollSelectors. > > src/java.base/linux/classes/sun/nio/ch/EPollSelectorImpl.java line 128: > >> 126: // timed poll interrupted so need to adjust timeout >> 127: long adjust = System.nanoTime() - startTime; >> 128: to =- (int) TimeUnit.NANOSECONDS.toMillis(adjust); > > This will now always assign a negative number to `to`. > > > > `=-` is not a compound assignment, it’s negation followed by a normal > assignment. Well spotted, I don't think that change was intentionally. - PR: https://git.openjdk.java.net/jdk/pull/8642
Re: RFR: 8286378: Address possibly lossy conversions in java.base [v3]
On Wed, 11 May 2022 16:30:41 GMT, Roger Riggs wrote: >> PR#8599 8244681: proposes to add compiler warnings for possible lossy >> conversions >> From the CSR: >> >> "If the type of the right-hand operand of a compound assignment is not >> assignment compatible with the type of the variable, a cast is implied and >> possible lossy conversion may silently occur. While similar situations are >> resolved as compilation errors for primitive assignments, there are no >> similar rules defined for compound assignments." >> >> This PR anticipates the warnings and adds explicit casts to replace the >> implicit casts. >> In most cases, the cast matches the type of the right-hand side to type of >> the compound assignment. >> Since these casts are truncating the value, there might be a different >> refactoring that avoids the cast >> but a direct replacement was chosen here. >> >> (Advise and suggestions will inform similar updates to other OpenJDK >> modules). > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Updated copyrights > Fixed cast style to add a space after cast, (where consistent with file > style) > Improved code per review comments in PollSelectors. src/java.base/linux/classes/sun/nio/ch/EPollSelectorImpl.java line 128: > 126: // timed poll interrupted so need to adjust timeout > 127: long adjust = System.nanoTime() - startTime; > 128: to =- (int) TimeUnit.NANOSECONDS.toMillis(adjust); This will now always assign a negative number to `to`. `=-` is not a compound assignment, it’s negation followed by a normal assignment. - PR: https://git.openjdk.java.net/jdk/pull/8642
Re: RFR: 8286378: Address possibly lossy conversions in java.base [v3]
On Wed, 11 May 2022 16:30:41 GMT, Roger Riggs wrote: >> PR#8599 8244681: proposes to add compiler warnings for possible lossy >> conversions >> From the CSR: >> >> "If the type of the right-hand operand of a compound assignment is not >> assignment compatible with the type of the variable, a cast is implied and >> possible lossy conversion may silently occur. While similar situations are >> resolved as compilation errors for primitive assignments, there are no >> similar rules defined for compound assignments." >> >> This PR anticipates the warnings and adds explicit casts to replace the >> implicit casts. >> In most cases, the cast matches the type of the right-hand side to type of >> the compound assignment. >> Since these casts are truncating the value, there might be a different >> refactoring that avoids the cast >> but a direct replacement was chosen here. >> >> (Advise and suggestions will inform similar updates to other OpenJDK >> modules). > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Updated copyrights > Fixed cast style to add a space after cast, (where consistent with file > style) > Improved code per review comments in PollSelectors. Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8642
Re: RFR: 8286378: Address possibly lossy conversions in java.base [v3]
On Wed, 11 May 2022 16:30:41 GMT, Roger Riggs wrote: >> PR#8599 8244681: proposes to add compiler warnings for possible lossy >> conversions >> From the CSR: >> >> "If the type of the right-hand operand of a compound assignment is not >> assignment compatible with the type of the variable, a cast is implied and >> possible lossy conversion may silently occur. While similar situations are >> resolved as compilation errors for primitive assignments, there are no >> similar rules defined for compound assignments." >> >> This PR anticipates the warnings and adds explicit casts to replace the >> implicit casts. >> In most cases, the cast matches the type of the right-hand side to type of >> the compound assignment. >> Since these casts are truncating the value, there might be a different >> refactoring that avoids the cast >> but a direct replacement was chosen here. >> >> (Advise and suggestions will inform similar updates to other OpenJDK >> modules). > > Roger Riggs has updated the pull request incrementally with one additional > commit since the last revision: > > Updated copyrights > Fixed cast style to add a space after cast, (where consistent with file > style) > Improved code per review comments in PollSelectors. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8642
Re: RFR: 8286378: Address possibly lossy conversions in java.base [v3]
> PR#8599 8244681: proposes to add compiler warnings for possible lossy > conversions > From the CSR: > > "If the type of the right-hand operand of a compound assignment is not > assignment compatible with the type of the variable, a cast is implied and > possible lossy conversion may silently occur. While similar situations are > resolved as compilation errors for primitive assignments, there are no > similar rules defined for compound assignments." > > This PR anticipates the warnings and adds explicit casts to replace the > implicit casts. > In most cases, the cast matches the type of the right-hand side to type of > the compound assignment. > Since these casts are truncating the value, there might be a different > refactoring that avoids the cast > but a direct replacement was chosen here. > > (Advise and suggestions will inform similar updates to other OpenJDK modules). Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Updated copyrights Fixed cast style to add a space after cast, (where consistent with file style) Improved code per review comments in PollSelectors. - Changes: - all: https://git.openjdk.java.net/jdk/pull/8642/files - new: https://git.openjdk.java.net/jdk/pull/8642/files/7ff806a5..a6679ce7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8642=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8642=01-02 Stats: 41 lines in 24 files changed: 0 ins; 0 del; 41 mod Patch: https://git.openjdk.java.net/jdk/pull/8642.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8642/head:pull/8642 PR: https://git.openjdk.java.net/jdk/pull/8642
