Re: RFR: 8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up to limits [v2]
On Wed, 7 Apr 2021 02:48:00 GMT, Ian Graves wrote: >> This fixes a bug where the formatting code for `%g` flags incorrectly tries >> to round `BigDecimal` after determining whether it should be a decimal >> numeric format or a scientific numeric format. The solution rounds before >> determining the correct format. > > Ian Graves has updated the pull request incrementally with one additional > commit since the last revision: > > Adding BigDecimal equivalents to tests Marked as reviewed by rriggs (Reviewer). src/java.base/share/classes/java/util/Formatter.java line 3826: > 3824: BigDecimal tenToTheNegFour = BigDecimal.valueOf(1, 4); > 3825: BigDecimal tenToThePrec = BigDecimal.valueOf(1, -prec); > 3826: value = value.round(new MathContext(prec)); While you are in the area, how about inlining the creation of the tenToTheNegFour and tenToThePrec values. They are used only once and may not be used at all. They don't appear to be needed except for the comparisons. - PR: https://git.openjdk.java.net/jdk/pull/3363
Re: RFR: 8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up to limits [v3]
> This fixes a bug where the formatting code for `%g` flags incorrectly tries > to round `BigDecimal` after determining whether it should be a decimal > numeric format or a scientific numeric format. The solution rounds before > determining the correct format. Ian Graves has updated the pull request incrementally with one additional commit since the last revision: Inlining some single use variables - Changes: - all: https://git.openjdk.java.net/jdk/pull/3363/files - new: https://git.openjdk.java.net/jdk/pull/3363/files/cf40421e..45522605 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3363&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3363&range=01-02 Stats: 4 lines in 1 file changed: 0 ins; 2 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/3363.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3363/head:pull/3363 PR: https://git.openjdk.java.net/jdk/pull/3363
Re: RFR: 8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up to limits [v2]
On Fri, 16 Apr 2021 14:26:58 GMT, Roger Riggs wrote: >> Ian Graves has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Adding BigDecimal equivalents to tests > > src/java.base/share/classes/java/util/Formatter.java line 3826: > >> 3824: BigDecimal tenToTheNegFour = BigDecimal.valueOf(1, 4); >> 3825: BigDecimal tenToThePrec = BigDecimal.valueOf(1, -prec); >> 3826: value = value.round(new MathContext(prec)); > > While you are in the area, how about inlining the creation of the > tenToTheNegFour and tenToThePrec values. > They are used only once and may not be used at all. They don't appear to be > needed except for the comparisons. Makes sense to me. - PR: https://git.openjdk.java.net/jdk/pull/3363
Re: RFR: 8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up to limits [v3]
On Fri, 16 Apr 2021 16:08:53 GMT, Ian Graves wrote: >> This fixes a bug where the formatting code for `%g` flags incorrectly tries >> to round `BigDecimal` after determining whether it should be a decimal >> numeric format or a scientific numeric format. The solution rounds before >> determining the correct format. > > Ian Graves has updated the pull request incrementally with one additional > commit since the last revision: > > Inlining some single use variables I think this looks all right. I assume you ran all the `java.math` tests. - Marked as reviewed by bpb (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/3363
Re: RFR: 8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up to limits [v3]
On Fri, 16 Apr 2021 16:08:53 GMT, Ian Graves wrote: >> This fixes a bug where the formatting code for `%g` flags incorrectly tries >> to round `BigDecimal` after determining whether it should be a decimal >> numeric format or a scientific numeric format. The solution rounds before >> determining the correct format. > > Ian Graves has updated the pull request incrementally with one additional > commit since the last revision: > > Inlining some single use variables That's correct. Passes `java.math` tests. Ran it again, just to be sure. - PR: https://git.openjdk.java.net/jdk/pull/3363
Integrated: 8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up to limits
On Tue, 6 Apr 2021 20:34:52 GMT, Ian Graves wrote: > This fixes a bug where the formatting code for `%g` flags incorrectly tries > to round `BigDecimal` after determining whether it should be a decimal > numeric format or a scientific numeric format. The solution rounds before > determining the correct format. This pull request has now been integrated. Changeset: 0bdc3e7a Author:Ian Graves Committer: Roger Riggs URL: https://git.openjdk.java.net/jdk/commit/0bdc3e7a Stats: 66 lines in 2 files changed: 62 ins; 1 del; 3 mod 8262744: Formatter '%g' conversion uses wrong format for BigDecimal rounding up to limits Reviewed-by: rriggs, bpb - PR: https://git.openjdk.java.net/jdk/pull/3363
RFR: 8265375: Bootcycle builds fail with StackOverflowError in cldrconverter
Please review the fix to the tier4 build failure. The piece of code that made into `CLDRLocaleProviderAdapter.java` was also needed in the build tool counterpart (`CLDRConverter`). - Commit messages: - 8265375: Bootcycle builds fail with StackOverflowError in cldrconverter Changes: https://git.openjdk.java.net/jdk/pull/3553/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3553&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8265375 Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/3553.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3553/head:pull/3553 PR: https://git.openjdk.java.net/jdk/pull/3553
Re: RFR: 8265375: Bootcycle builds fail with StackOverflowError in cldrconverter
On Fri, 16 Apr 2021 21:10:42 GMT, Naoto Sato wrote: > Please review the fix to the tier4 build failure. The piece of code that made > into `CLDRLocaleProviderAdapter.java` was also needed in the build tool > counterpart (`CLDRConverter`). Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/3553
Re: RFR: 8265375: Bootcycle builds fail with StackOverflowError in cldrconverter [v2]
> Please review the fix to the tier4 build failure. The piece of code that made > into `CLDRLocaleProviderAdapter.java` was also needed in the build tool > counterpart (`CLDRConverter`). Naoto Sato has updated the pull request incrementally with one additional commit since the last revision: Copyright year update. - Changes: - all: https://git.openjdk.java.net/jdk/pull/3553/files - new: https://git.openjdk.java.net/jdk/pull/3553/files/91326786..e54cb6ec Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=3553&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=3553&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/3553.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3553/head:pull/3553 PR: https://git.openjdk.java.net/jdk/pull/3553
Integrated: 8265375: Bootcycle builds fail with StackOverflowError in cldrconverter
On Fri, 16 Apr 2021 21:10:42 GMT, Naoto Sato wrote: > Please review the fix to the tier4 build failure. The piece of code that made > into `CLDRLocaleProviderAdapter.java` was also needed in the build tool > counterpart (`CLDRConverter`). This pull request has now been integrated. Changeset: ff499701 Author:Naoto Sato URL: https://git.openjdk.java.net/jdk/commit/ff499701 Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod 8265375: Bootcycle builds fail with StackOverflowError in cldrconverter Reviewed-by: joehw - PR: https://git.openjdk.java.net/jdk/pull/3553
