Integrated: 8283001: windows-x86-cmp-baseline fails in some jvmti native libs
On Tue, 10 May 2022 20:23:01 GMT, Leonid Mesnik wrote: > 8283001: windows-x86-cmp-baseline fails in some jvmti native libs This pull request has now been integrated. Changeset: 82d25700 Author:Leonid Mesnik URL: https://git.openjdk.java.net/jdk/commit/82d257006d6337920bbc78a33cf61c3cb809ce4b Stats: 85 lines in 41 files changed: 41 ins; 2 del; 42 mod 8283001: windows-x86-cmp-baseline fails in some jvmti native libs Reviewed-by: sspitsyn, amenkov - PR: https://git.openjdk.java.net/jdk/pull/8641
Re: RFR: 8283001: windows-x86-cmp-baseline fails in some jvmti native libs
On Tue, 10 May 2022 20:23:01 GMT, Leonid Mesnik wrote: > 8283001: windows-x86-cmp-baseline fails in some jvmti native libs Marked as reviewed by amenkov (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8641
Integrated: 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach()
On Tue, 10 May 2022 04:00:29 GMT, Ioi Lam wrote: > The `mode` parameter for ` jdk.internal.perf.Perf.attach()` has never > supported the value `"rw"` since the source code was imported to the openjdk > repo more than 15 years ago. In fact HotSpot throws > `IllegalArgumentException` when such a mode is specified. > > It's unlikely such a mode will be required for future enhancements. Support > for `"rw"` is removed. The "mode" parameter is also removed, since now `"r"` > is the only supported mode. > > I also cleaned up related code in the JDK and HotSpot. > > Testing: > - Passed tiers 1 ~ 5 This pull request has now been integrated. Changeset: fcf49f42 Author:Ioi Lam URL: https://git.openjdk.java.net/jdk/commit/fcf49f42cef4ac3e50b3b480aecf6fa38cf5be00 Stats: 205 lines in 15 files changed: 3 ins; 153 del; 49 mod 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach() Reviewed-by: redestad, alanb - PR: https://git.openjdk.java.net/jdk/pull/8622
Re: RFR: 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach()
On Tue, 10 May 2022 20:03:45 GMT, Alan Bateman wrote: >> The `mode` parameter for ` jdk.internal.perf.Perf.attach()` has never >> supported the value `"rw"` since the source code was imported to the openjdk >> repo more than 15 years ago. In fact HotSpot throws >> `IllegalArgumentException` when such a mode is specified. >> >> It's unlikely such a mode will be required for future enhancements. Support >> for `"rw"` is removed. The "mode" parameter is also removed, since now `"r"` >> is the only supported mode. >> >> I also cleaned up related code in the JDK and HotSpot. >> >> Testing: >> - Passed tiers 1 ~ 5 > > I skimmed through the changes and I think they look okay. In the distant past > there were tools outside of the JDK that used the jvmstat API directly. It's > possible that VisualVM still does but it would only compile/run if > --add-exports is used to export the sun.jvmstat.* packages. So it might be > that dropping the parameter from a method in RemoteHost is noticed and I > think that is okay because this package is not exported and is not meant to > be used by code outside of the JDK. Thanks to @AlanBateman and @cl4es for the review. - PR: https://git.openjdk.java.net/jdk/pull/8622
Re: RFR: 8286441: Remove mode parameter from jdk.internal.perf.Perf.attach() [v2]
On Wed, 11 May 2022 02:43:21 GMT, Ioi Lam wrote: >> The `mode` parameter for ` jdk.internal.perf.Perf.attach()` has never >> supported the value `"rw"` since the source code was imported to the openjdk >> repo more than 15 years ago. In fact HotSpot throws >> `IllegalArgumentException` when such a mode is specified. >> >> It's unlikely such a mode will be required for future enhancements. Support >> for `"rw"` is removed. The "mode" parameter is also removed, since now `"r"` >> is the only supported mode. >> >> I also cleaned up related code in the JDK and HotSpot. >> >> Testing: >> - Passed tiers 1 and 2 >> - Tiers 3, 4, 5 are in progress > > Ioi Lam has updated the pull request incrementally with one additional commit > since the last revision: > > review comments Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/8622
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v5]
> Please review this patch adding new lint option, **lossy-conversions**, to > javac to warn about type casts in compound assignments with possible lossy > conversions. > > The new lint warning is shown if the type of the right-hand operand of a > compound assignment is not assignment compatible with the type of the > variable. > > The implementation of the warning is based on similar check performed to emit > "possible lossy conversion" compilation error for simple assignments. > > Proposed patch also include complex matrix-style test with positive and > negative test cases of lossy conversions in compound assignments. > > Proposed patch also disables this new lint option in all affected JDK modules > and libraries to allow smooth JDK build. Individual cases to address possibly > lossy conversions warnings in JDK are already addressed in a separate > umbrella issue and its sub-tasks. > > Thanks for your review, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: enabled lossy-conversions warnings for jdk.jfr and jdk.management.jfr - Changes: - all: https://git.openjdk.java.net/jdk/pull/8599/files - new: https://git.openjdk.java.net/jdk/pull/8599/files/a59dfa4f..32282515 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8599=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8599=03-04 Stats: 2 lines in 2 files changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8599.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8599/head:pull/8599 PR: https://git.openjdk.java.net/jdk/pull/8599
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v3]
On Wed, 11 May 2022 13:31:16 GMT, Roger Riggs wrote: >> Thanks for quick reaction. >> I'll keep my eyes on this race of patches and update this pull request >> accordingly or create a new PR. > > I put out a PR for java.base, but thought I'd wait until the javac fixe were > pushed before integrating and would re-enable the warnings at the same time. Feel free to go ahead with the java.base PR as this one still needs CSR. - PR: https://git.openjdk.java.net/jdk/pull/8599
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v4]
> Please review this patch adding new lint option, **lossy-conversions**, to > javac to warn about type casts in compound assignments with possible lossy > conversions. > > The new lint warning is shown if the type of the right-hand operand of a > compound assignment is not assignment compatible with the type of the > variable. > > The implementation of the warning is based on similar check performed to emit > "possible lossy conversion" compilation error for simple assignments. > > Proposed patch also include complex matrix-style test with positive and > negative test cases of lossy conversions in compound assignments. > > Proposed patch also disables this new lint option in all affected JDK modules > and libraries to allow smooth JDK build. Individual cases to address possibly > lossy conversions warnings in JDK are already addressed in a separate > umbrella issue and its sub-tasks. > > Thanks for your review, > Adam Adam Sotona has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision: - Merge branch 'openjdk:master' into JDK-8244681 - 8244681: Add a warning for possibly lossy conversion in compound assignments recommended correction of the warning description - 8244681: Add a warning for possibly lossy conversion in compound assignments recommended correction of the warning wording fixed typo in test method name - Merge branch 'openjdk:master' into JDK-8244681 - 8244681: Add a warning for possibly lossy conversion in compound assignments jdk.internal.le make patch to disable warnings - 8244681: Add a warning for possibly lossy conversion in compound assignments - Changes: - all: https://git.openjdk.java.net/jdk/pull/8599/files - new: https://git.openjdk.java.net/jdk/pull/8599/files/f0729396..a59dfa4f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8599=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8599=02-03 Stats: 9179 lines in 255 files changed: 5253 ins; 2422 del; 1504 mod Patch: https://git.openjdk.java.net/jdk/pull/8599.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8599/head:pull/8599 PR: https://git.openjdk.java.net/jdk/pull/8599
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v3]
On Wed, 11 May 2022 13:27:38 GMT, Adam Sotona wrote: >> That's good to know. I think the tricky part is mostly about keeping track >> of all these disabled warnings, so they are not kept around longer than >> necessary. And that needs coordination with all the subtasks of the umbrella >> issue. > > Thanks for quick reaction. > I'll keep my eyes on this race of patches and update this pull request > accordingly or create a new PR. I put out a PR for java.base, but thought I'd wait until the javac fixe were pushed before integrating and would re-enable the warnings at the same time. - PR: https://git.openjdk.java.net/jdk/pull/8599
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v3]
On Wed, 11 May 2022 13:10:10 GMT, Magnus Ihse Bursie wrote: >> I agree, but if it doesn't happen, I can follow up with a separate PR where >> I remove the disablement. > > That's good to know. I think the tricky part is mostly about keeping track of > all these disabled warnings, so they are not kept around longer than > necessary. And that needs coordination with all the subtasks of the umbrella > issue. Thanks for quick reaction. I'll keep my eyes on this race of patches and update this pull request accordingly or create a new PR. - PR: https://git.openjdk.java.net/jdk/pull/8599
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v3]
On Wed, 11 May 2022 13:05:45 GMT, Erik Gahlin wrote: >> make/modules/jdk.jfr/Java.gmk line 26: >> >>> 24: # >>> 25: >>> 26: DISABLED_WARNINGS_java += exports lossy-conversions >> >> Note that with the fix of JDK-8286392 (and JDK-8286396) the >> `lossy-conversions` warning should not be disabled for the JFR code. >> >> In general, you need to check which of the subtasks of JDK-8286374 that has >> been fixed, and adjust the makefiles accordingly, before pushing this fix. >> >> (In the future, it might be easier to push the fix which disables the >> warnings first, and then file follow-up bugs on aa per-component basis, and >> remind them to remove the disabling in the makefile. That way there won't be >> a race between individual fixes and a "master" bug like this.) > > I agree, but if it doesn't happen, I can follow up with a separate PR where I > remove the disablement. That's good to know. I think the tricky part is mostly about keeping track of all these disabled warnings, so they are not kept around longer than necessary. And that needs coordination with all the subtasks of the umbrella issue. - PR: https://git.openjdk.java.net/jdk/pull/8599
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v3]
On Wed, 11 May 2022 12:59:49 GMT, Magnus Ihse Bursie wrote: >> Adam Sotona has updated the pull request incrementally with one additional >> commit since the last revision: >> >> 8244681: Add a warning for possibly lossy conversion in compound >> assignments >> recommended correction of the warning description > > make/modules/jdk.jfr/Java.gmk line 26: > >> 24: # >> 25: >> 26: DISABLED_WARNINGS_java += exports lossy-conversions > > Note that with the fix of JDK-8286392 (and JDK-8286396) the > `lossy-conversions` warning should not be disabled for the JFR code. > > In general, you need to check which of the subtasks of JDK-8286374 that has > been fixed, and adjust the makefiles accordingly, before pushing this fix. > > (In the future, it might be easier to push the fix which disables the > warnings first, and then file follow-up bugs on aa per-component basis, and > remind them to remove the disabling in the makefile. That way there won't be > a race between individual fixes and a "master" bug like this.) I agree, but if it doesn't happen, I can follow up with a separate PR where I remove the disablement. - PR: https://git.openjdk.java.net/jdk/pull/8599
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v3]
On Wed, 11 May 2022 07:45:39 GMT, Adam Sotona wrote: >> Please review this patch adding new lint option, **lossy-conversions**, to >> javac to warn about type casts in compound assignments with possible lossy >> conversions. >> >> The new lint warning is shown if the type of the right-hand operand of a >> compound assignment is not assignment compatible with the type of the >> variable. >> >> The implementation of the warning is based on similar check performed to >> emit "possible lossy conversion" compilation error for simple assignments. >> >> Proposed patch also include complex matrix-style test with positive and >> negative test cases of lossy conversions in compound assignments. >> >> Proposed patch also disables this new lint option in all affected JDK >> modules and libraries to allow smooth JDK build. Individual cases to address >> possibly lossy conversions warnings in JDK are already addressed in a >> separate umbrella issue and its sub-tasks. >> >> Thanks for your review, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > 8244681: Add a warning for possibly lossy conversion in compound assignments > recommended correction of the warning description Lossy conversion issues for jdk.jfr and jdk.management.jfr. have been fixed. - PR: https://git.openjdk.java.net/jdk/pull/8599
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v3]
On Wed, 11 May 2022 07:45:39 GMT, Adam Sotona wrote: >> Please review this patch adding new lint option, **lossy-conversions**, to >> javac to warn about type casts in compound assignments with possible lossy >> conversions. >> >> The new lint warning is shown if the type of the right-hand operand of a >> compound assignment is not assignment compatible with the type of the >> variable. >> >> The implementation of the warning is based on similar check performed to >> emit "possible lossy conversion" compilation error for simple assignments. >> >> Proposed patch also include complex matrix-style test with positive and >> negative test cases of lossy conversions in compound assignments. >> >> Proposed patch also disables this new lint option in all affected JDK >> modules and libraries to allow smooth JDK build. Individual cases to address >> possibly lossy conversions warnings in JDK are already addressed in a >> separate umbrella issue and its sub-tasks. >> >> Thanks for your review, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > 8244681: Add a warning for possibly lossy conversion in compound assignments > recommended correction of the warning description Check updates on JDK-8286374 subtasks. make/modules/jdk.jfr/Java.gmk line 26: > 24: # > 25: > 26: DISABLED_WARNINGS_java += exports lossy-conversions Note that with the fix of JDK-8286392 (and JDK-8286396) the `lossy-conversions` warning should not be disabled for the JFR code. In general, you need to check which of the subtasks of JDK-8286374 that has been fixed, and adjust the makefiles accordingly, before pushing this fix. (In the future, it might be easier to push the fix which disables the warnings first, and then file follow-up bugs on aa per-component basis, and remind them to remove the disabling in the makefile. That way there won't be a race between individual fixes and a "master" bug like this.) - Changes requested by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8599
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v3]
> Please review this patch adding new lint option, **lossy-conversions**, to > javac to warn about type casts in compound assignments with possible lossy > conversions. > > The new lint warning is shown if the type of the right-hand operand of a > compound assignment is not assignment compatible with the type of the > variable. > > The implementation of the warning is based on similar check performed to emit > "possible lossy conversion" compilation error for simple assignments. > > Proposed patch also include complex matrix-style test with positive and > negative test cases of lossy conversions in compound assignments. > > Proposed patch also disables this new lint option in all affected JDK modules > and libraries to allow smooth JDK build. Individual cases to address possibly > lossy conversions warnings in JDK are already addressed in a separate > umbrella issue and its sub-tasks. > > Thanks for your review, > Adam Adam Sotona has updated the pull request incrementally with one additional commit since the last revision: 8244681: Add a warning for possibly lossy conversion in compound assignments recommended correction of the warning description - Changes: - all: https://git.openjdk.java.net/jdk/pull/8599/files - new: https://git.openjdk.java.net/jdk/pull/8599/files/6b3942b8..f0729396 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=8599=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8599=01-02 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/8599.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/8599/head:pull/8599 PR: https://git.openjdk.java.net/jdk/pull/8599
Re: RFR: 8244681: Add a warning for possibly lossy conversion in compound assignments [v2]
On Tue, 10 May 2022 09:07:44 GMT, Adam Sotona wrote: >> Please review this patch adding new lint option, **lossy-conversions**, to >> javac to warn about type casts in compound assignments with possible lossy >> conversions. >> >> The new lint warning is shown if the type of the right-hand operand of a >> compound assignment is not assignment compatible with the type of the >> variable. >> >> The implementation of the warning is based on similar check performed to >> emit "possible lossy conversion" compilation error for simple assignments. >> >> Proposed patch also include complex matrix-style test with positive and >> negative test cases of lossy conversions in compound assignments. >> >> Proposed patch also disables this new lint option in all affected JDK >> modules and libraries to allow smooth JDK build. Individual cases to address >> possibly lossy conversions warnings in JDK are already addressed in a >> separate umbrella issue and its sub-tasks. >> >> Thanks for your review, >> Adam > > Adam Sotona has updated the pull request incrementally with one additional > commit since the last revision: > > 8244681: Add a warning for possibly lossy conversion in compound assignments > recommended correction of the warning wording > fixed typo in test method name src/jdk.compiler/share/classes/com/sun/tools/javac/resources/javac.properties line 210: > 208: > 209: javac.opt.Xlint.desc.lossy-conversions=\ > 210: Warn about compiler possible lossy conversions. I like this warning. But the documentation doesn't even parse as English. I suggest this, as more grammatical and precise: > Warn about possible lossy conversions in compound assignments - PR: https://git.openjdk.java.net/jdk/pull/8599
