Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v10]
> Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon 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 11 additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into JDK-8267670 - Merge remote-tracking branch 'origin/master' into JDK-8267670 - Merge remote-tracking branch 'origin/master' into JDK-8267670 - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Added missing brace - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Removed trailing whitespace - 8267670: Remove redundant code from switch - 8267670: Updated code to use yield - Merge remote-tracking branch 'origin/master' into JDK-8267670 - ... and 1 more: https://git.openjdk.java.net/jdk/compare/4c495d3b...44f86889 - Changes: - all: https://git.openjdk.java.net/jdk/pull/4182/files - new: https://git.openjdk.java.net/jdk/pull/4182/files/44886603..44f86889 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4182=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4182=08-09 Stats: 10 lines in 3 files changed: 3 ins; 4 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/4182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182 PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v9]
On Mon, 31 May 2021 14:10:50 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.io`, >> `java.math`, and `java.text` packages to make use of the switch expressions? >> >> Kind regards, >> Patrick > > Patrick Concannon 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 10 additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Added missing brace > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Removed trailing whitespace > - 8267670: Remove redundant code from switch > - 8267670: Updated code to use yield > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Update java.io, java.math, and java.text to use switch expressions Marked as reviewed by vtewari (Committer). - PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v9]
On Mon, 31 May 2021 14:10:50 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.io`, >> `java.math`, and `java.text` packages to make use of the switch expressions? >> >> Kind regards, >> Patrick > > Patrick Concannon 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 10 additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Added missing brace > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Removed trailing whitespace > - 8267670: Remove redundant code from switch > - 8267670: Updated code to use yield > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Update java.io, java.math, and java.text to use switch expressions Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v9]
On Mon, 31 May 2021 14:07:54 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.io`, >> `java.math`, and `java.text` packages to make use of the switch expressions? >> >> Kind regards, >> Patrick > > Patrick Concannon 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 10 additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Added missing brace > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Removed trailing whitespace > - 8267670: Remove redundant code from switch > - 8267670: Updated code to use yield > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Update java.io, java.math, and java.text to use switch expressions Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v9]
> Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon 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 10 additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into JDK-8267670 - Merge remote-tracking branch 'origin/master' into JDK-8267670 - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Added missing brace - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Removed trailing whitespace - 8267670: Remove redundant code from switch - 8267670: Updated code to use yield - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Update java.io, java.math, and java.text to use switch expressions - Changes: - all: https://git.openjdk.java.net/jdk/pull/4182/files - new: https://git.openjdk.java.net/jdk/pull/4182/files/933e59e9..44886603 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4182=08 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4182=07-08 Stats: 6066 lines in 412 files changed: 3749 ins; 1268 del; 1049 mod Patch: https://git.openjdk.java.net/jdk/pull/4182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182 PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v8]
On Thu, 27 May 2021 15:33:36 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.io`, >> `java.math`, and `java.text` packages to make use of the switch expressions? >> >> Kind regards, >> Patrick > > Patrick Concannon 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 ten additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Added missing brace > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Removed trailing whitespace > - 8267670: Remove redundant code from switch > - 8267670: Updated code to use yield > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Update java.io, java.math, and java.text to use switch expressions Looks good Patrick - Marked as reviewed by lancea (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v8]
> Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon 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 ten additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into JDK-8267670 - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Added missing brace - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Removed trailing whitespace - 8267670: Remove redundant code from switch - 8267670: Updated code to use yield - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Update java.io, java.math, and java.text to use switch expressions - Changes: - all: https://git.openjdk.java.net/jdk/pull/4182/files - new: https://git.openjdk.java.net/jdk/pull/4182/files/57184fbf..933e59e9 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4182=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4182=06-07 Stats: 29913 lines in 434 files changed: 3355 ins; 25561 del; 997 mod Patch: https://git.openjdk.java.net/jdk/pull/4182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182 PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v7]
> Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon 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 eight additional commits since the last revision: - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Added missing brace - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Removed trailing whitespace - 8267670: Remove redundant code from switch - 8267670: Updated code to use yield - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Update java.io, java.math, and java.text to use switch expressions - Changes: - all: https://git.openjdk.java.net/jdk/pull/4182/files - new: https://git.openjdk.java.net/jdk/pull/4182/files/a0dfbeba..57184fbf Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4182=06 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4182=05-06 Stats: 29 lines in 1 file changed: 17 ins; 5 del; 7 mod Patch: https://git.openjdk.java.net/jdk/pull/4182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182 PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v6]
> Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon has updated the pull request incrementally with one additional commit since the last revision: 8267670: Added missing brace - Changes: - all: https://git.openjdk.java.net/jdk/pull/4182/files - new: https://git.openjdk.java.net/jdk/pull/4182/files/ad7aeacd..a0dfbeba Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4182=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4182=04-05 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/4182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182 PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v5]
On Wed, 26 May 2021 09:39:44 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.io`, >> `java.math`, and `java.text` packages to make use of the switch expressions? >> >> Kind regards, >> Patrick > > Patrick Concannon 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 remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Removed trailing whitespace > - 8267670: Remove redundant code from switch > - 8267670: Updated code to use yield > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Update java.io, java.math, and java.text to use switch expressions Marked as reviewed by dfuchs (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v5]
> Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon 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 remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Removed trailing whitespace - 8267670: Remove redundant code from switch - 8267670: Updated code to use yield - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Update java.io, java.math, and java.text to use switch expressions - Changes: - all: https://git.openjdk.java.net/jdk/pull/4182/files - new: https://git.openjdk.java.net/jdk/pull/4182/files/d4a6cc44..ad7aeacd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4182=04 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4182=03-04 Stats: 1031 lines in 36 files changed: 717 ins; 101 del; 213 mod Patch: https://git.openjdk.java.net/jdk/pull/4182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182 PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v4]
On Wed, 26 May 2021 09:05:34 GMT, Patrick Concannon
wrote:
>> Hi,
>>
>> Could someone please review my code for updating the code in the `java.io`,
>> `java.math`, and `java.text` packages to make use of the switch expressions?
>>
>> Kind regards,
>> Patrick
>
> Patrick Concannon has updated the pull request incrementally with one
> additional commit since the last revision:
>
> 8267670: Removed trailing whitespace
> _Mailing list message from [Brian Goetz](mailto:brian.go...@oracle.com) on
> [core-libs-dev](mailto:core-libs-...@mail.openjdk.java.net):_
>
> In the last hunk, you convert
>
> case Collator.IDENTICAL: toAddTo.append('='); break;
> case Collator.TERTIARY: toAddTo.append(','); break;
> case Collator.SECONDARY: toAddTo.append(';'); break;
> case Collator.PRIMARY: toAddTo.append('<'); break;
> case RESET: toAddTo.append('&'); break;
> case UNSET: toAddTo.append('?'); break;
>
> to
>
> case Collator.IDENTICAL -> toAddTo.append('=');
> case Collator.TERTIARY -> toAddTo.append(',');
> case Collator.SECONDARY -> toAddTo.append(';');
> case Collator.PRIMARY -> toAddTo.append('<');
> case RESET -> toAddTo.append('&');
> case UNSET -> toAddTo.append('?');
>
> But, you can go further, pulling the toAddTo.append() call out of the switch.
> This was one of the benefits we anticipated with expression switches; that it
> would expose more opportunities to push the conditional logic farther down
> towards the leaves. I suspect there are other opportunities for this in this
> patch too.
Hi Brian,
Thanks for your suggestion. I've incorporated it into the PR, and you can find
it in commit e503ed2
-
PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v4]
> Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon has updated the pull request incrementally with one additional commit since the last revision: 8267670: Removed trailing whitespace - Changes: - all: https://git.openjdk.java.net/jdk/pull/4182/files - new: https://git.openjdk.java.net/jdk/pull/4182/files/e503ed26..d4a6cc44 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4182=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4182=02-03 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/4182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182 PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v2]
On Tue, 25 May 2021 15:18:46 GMT, Chris Hegarty wrote:
>> Patrick Concannon 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 three additional
>> commits since the last revision:
>>
>> - 8267670: Updated code to use yield
>> - Merge remote-tracking branch 'origin/master' into JDK-8267670
>> - 8267670: Update java.io, java.math, and java.text to use switch
>> expressions
>
> src/java.base/share/classes/java/io/ObjectStreamClass.java line 2172:
>
>> 2170: switch (typeCodes[i]) {
>> 2171: case 'L', '[' -> vals[offsets[i]] =
>> unsafe.getReference(obj, readKeys[i]);
>> 2172: default -> throw new InternalError();
>
> suggest:
>
> vals[offsets[i]] = switch (typeCodes[i]) {
> case 'L', '[' -> unsafe.getReference(obj, readKeys[i]);
> default -> throw new InternalError();
Comment addressed in e503ed2
> src/java.base/share/classes/java/io/StreamTokenizer.java line 787:
>
>> 785: case TT_WORD-> ret = sval;
>> 786: case TT_NUMBER -> ret = "n=" + nval;
>> 787: case TT_NOTHING -> ret = "NOTHING";
>
> This is not correct, the assignments to ret in these case arms is redundant.
Comment addressed in e503ed2
-
PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v3]
On Tue, 25 May 2021 21:43:36 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.io`, >> `java.math`, and `java.text` packages to make use of the switch expressions? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull request incrementally with one > additional commit since the last revision: > > 8267670: Remove redundant code from switch Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v3]
On Tue, 25 May 2021 21:43:36 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.io`, >> `java.math`, and `java.text` packages to make use of the switch expressions? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull request incrementally with one > additional commit since the last revision: > > 8267670: Remove redundant code from switch Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v3]
On Tue, 25 May 2021 18:54:48 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.io`, >> `java.math`, and `java.text` packages to make use of the switch expressions? >> >> Kind regards, >> Patrick > > Patrick Concannon has updated the pull request incrementally with one > additional commit since the last revision: > > 8267670: Remove redundant code from switch Marked as reviewed by chegar (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v3]
> Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon has updated the pull request incrementally with one additional commit since the last revision: 8267670: Remove redundant code from switch - Changes: - all: https://git.openjdk.java.net/jdk/pull/4182/files - new: https://git.openjdk.java.net/jdk/pull/4182/files/adc8af41..e503ed26 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4182=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4182=01-02 Stats: 24 lines in 3 files changed: 4 ins; 3 del; 17 mod Patch: https://git.openjdk.java.net/jdk/pull/4182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182 PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v2]
On Tue, 25 May 2021 14:57:22 GMT, Patrick Concannon wrote: >> Hi, >> >> Could someone please review my code for updating the code in the `java.io`, >> `java.math`, and `java.text` packages to make use of the switch expressions? >> >> Kind regards, >> Patrick > > Patrick Concannon 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 three additional > commits since the last revision: > > - 8267670: Updated code to use yield > - Merge remote-tracking branch 'origin/master' into JDK-8267670 > - 8267670: Update java.io, java.math, and java.text to use switch expressions Changes in java.math look fine. - Marked as reviewed by darcy (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions
In the last hunk, you convert
case Collator.IDENTICAL: toAddTo.append('='); break;
case Collator.TERTIARY: toAddTo.append(','); break;
case Collator.SECONDARY: toAddTo.append(';'); break;
case Collator.PRIMARY: toAddTo.append('<'); break;
case RESET: toAddTo.append('&'); break;
case UNSET: toAddTo.append('?'); break;
to
case Collator.IDENTICAL -> toAddTo.append('=');
case Collator.TERTIARY -> toAddTo.append(',');
case Collator.SECONDARY -> toAddTo.append(';');
case Collator.PRIMARY -> toAddTo.append('<');
case RESET -> toAddTo.append('&');
case UNSET -> toAddTo.append('?');
But, you can go further, pulling the toAddTo.append() call out of the switch.
This was one of the benefits we anticipated with expression switches; that it
would expose more opportunities to push the conditional logic farther down
towards the leaves. I suspect there are other opportunities for this in this
patch too.
> On May 25, 2021, at 7:57 AM, Patrick Concannon
> wrote:
>
> Hi,
>
> Could someone please review my code for updating the code in the `java.io`,
> `java.math`, and `java.text` packages to make use of the switch expressions?
>
> Kind regards,
> Patrick
>
> -
>
> Commit messages:
> - 8267670: Update java.io, java.math, and java.text to use switch expressions
>
> Changes: https://git.openjdk.java.net/jdk/pull/4182/files
> Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4182=00
> Issue: https://bugs.openjdk.java.net/browse/JDK-8267670
> Stats: 328 lines in 11 files changed: 1 ins; 187 del; 140 mod
> Patch: https://git.openjdk.java.net/jdk/pull/4182.diff
> Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182
>
> PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v2]
On Tue, 25 May 2021 14:53:44 GMT, Patrick Concannon
wrote:
>> src/java.base/share/classes/java/io/StreamTokenizer.java line 795:
>>
>>> 793: * case statements
>>> 794: */
>>> 795: if (ttype < 256 && ((ctype[ttype] & CT_QUOTE) != 0)) {
>>
>> Maybe (since its easier to grok the yield rather than the assignment of ret
>> in branches):
>>
>> String ret = switch (ttype) {
>> case TT_EOF -> "EOF";
>> case TT_EOL -> "EOL";
>> case TT_WORD-> sval;
>> case TT_NUMBER -> "n=" + nval;
>> case TT_NOTHING -> "NOTHING";
>> default -> {
>> /*
>> * ttype is the first character of either a quoted string or
>> * is an ordinary character. ttype can definitely not be less
>> * than 0, since those are reserved values used in the
>> previous
>> * case statements
>> */
>> if (ttype < 256 && ((ctype[ttype] & CT_QUOTE) != 0)) {
>> yield sval;
>> }
>> char s[] = new char[3];
>> s[0] = s[2] = ''';
>> s[1] = (char) ttype;
>> yield new String(s);
>> }
>> };
>> return "Token[" + ret + "], line " + LINENO;
>
> Code updated as suggested. See adc8af4
The snippet above both uses yield in the default case, and also removes the
assignments from the other arms. adc8af4 overlooks the redundant assignments in
the non-default cases.
-
PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v2]
On Tue, 25 May 2021 14:57:22 GMT, Patrick Concannon
wrote:
>> Hi,
>>
>> Could someone please review my code for updating the code in the `java.io`,
>> `java.math`, and `java.text` packages to make use of the switch expressions?
>>
>> Kind regards,
>> Patrick
>
> Patrick Concannon 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 three additional
> commits since the last revision:
>
> - 8267670: Updated code to use yield
> - Merge remote-tracking branch 'origin/master' into JDK-8267670
> - 8267670: Update java.io, java.math, and java.text to use switch expressions
src/java.base/share/classes/java/io/ObjectStreamClass.java line 2172:
> 2170: switch (typeCodes[i]) {
> 2171: case 'L', '[' -> vals[offsets[i]] =
> unsafe.getReference(obj, readKeys[i]);
> 2172: default -> throw new InternalError();
suggest:
vals[offsets[i]] = switch (typeCodes[i]) {
case 'L', '[' -> unsafe.getReference(obj, readKeys[i]);
default -> throw new InternalError();
src/java.base/share/classes/java/io/StreamTokenizer.java line 787:
> 785: case TT_WORD-> ret = sval;
> 786: case TT_NUMBER -> ret = "n=" + nval;
> 787: case TT_NOTHING -> ret = "NOTHING";
This is not correct, the assignments to ret in these case arms is redundant.
-
PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v2]
On Tue, 25 May 2021 12:43:00 GMT, Naoto Sato wrote: >> Patrick Concannon 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 three additional >> commits since the last revision: >> >> - 8267670: Updated code to use yield >> - Merge remote-tracking branch 'origin/master' into JDK-8267670 >> - 8267670: Update java.io, java.math, and java.text to use switch >> expressions > > src/java.base/share/classes/java/text/BreakIterator.java line 569: > >> 567: case SENTENCE_INDEX -> >> breakIteratorProvider.getSentenceInstance(locale); >> 568: default -> null; >> 569: }; > > No need to use the local variable `iterator` here. Simply return with the > switch expression. Hi Naoto, thanks for spotting this. Code updated as suggested. See adc8af4 > src/java.base/share/classes/java/text/NumberFormat.java line 982: > >> 980: case COMPACTSTYLE -> >> provider.getCompactNumberInstance(locale, formatStyle); >> 981: default-> null; >> 982: }; > > Same as `BreakIterator`. No need for `numberFormat`. Code updated as suggested. See adc8af4 - PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v2]
On Tue, 25 May 2021 12:31:38 GMT, Chris Hegarty wrote:
>> Patrick Concannon 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 three additional
>> commits since the last revision:
>>
>> - 8267670: Updated code to use yield
>> - Merge remote-tracking branch 'origin/master' into JDK-8267670
>> - 8267670: Update java.io, java.math, and java.text to use switch
>> expressions
>
> src/java.base/share/classes/java/io/ObjectInputStream.java line 1877:
>
>> 1875: descriptor.checkInitialized();
>> 1876: }
>> 1877: default -> throw new
>> StreamCorruptedException(
>
> What would you think of assigning descriptor with the value returned from
> evaluating the switch?
>
> Either:
>
> 1)
>
> ObjectStreamClass descriptor = switch (tc) {
> case TC_NULL-> (ObjectStreamClass) readNull();
> case TC_PROXYCLASSDESC -> readProxyDesc(unshared);
> case TC_CLASSDESC -> readNonProxyDesc(unshared);
> case TC_REFERENCE -> readAndCheckHandle(unshared);
> default -> throw new StreamCorruptedException(String.format("invalid
> type code: %02X", tc));
> };
> return descriptor;
> }
>
> , where the body of TC_REFERENCE is enclosed in readAndCheckHandle, OR
>
> 2) Simply
>
> case TC_REFERENCE -> {
> var d = (ObjectStreamClass)readHandle(unshared);
> // Should only reference initialized class descriptors
> d.checkInitialized();
> yield d; }
Code updated as suggested. See adc8af4
> src/java.base/share/classes/java/io/StreamTokenizer.java line 795:
>
>> 793: * case statements
>> 794: */
>> 795: if (ttype < 256 && ((ctype[ttype] & CT_QUOTE) != 0)) {
>
> Maybe (since its easier to grok the yield rather than the assignment of ret
> in branches):
>
> String ret = switch (ttype) {
> case TT_EOF -> "EOF";
> case TT_EOL -> "EOL";
> case TT_WORD-> sval;
> case TT_NUMBER -> "n=" + nval;
> case TT_NOTHING -> "NOTHING";
> default -> {
> /*
> * ttype is the first character of either a quoted string or
> * is an ordinary character. ttype can definitely not be less
> * than 0, since those are reserved values used in the
> previous
> * case statements
> */
> if (ttype < 256 && ((ctype[ttype] & CT_QUOTE) != 0)) {
> yield sval;
> }
> char s[] = new char[3];
> s[0] = s[2] = ''';
> s[1] = (char) ttype;
> yield new String(s);
> }
> };
> return "Token[" + ret + "], line " + LINENO;
Code updated as suggested. See adc8af4
-
PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v2]
On Tue, 25 May 2021 13:16:00 GMT, Daniel Fuchs wrote:
>> Patrick Concannon 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 three additional
>> commits since the last revision:
>>
>> - 8267670: Updated code to use yield
>> - Merge remote-tracking branch 'origin/master' into JDK-8267670
>> - 8267670: Update java.io, java.math, and java.text to use switch
>> expressions
>
> src/java.base/share/classes/java/io/ObjectStreamField.java line 123:
>
>> 121: case 'D' -> type = Double.TYPE;
>> 122: case 'L', '[' -> type = Object.class;
>> 123: default -> throw new
>> IllegalArgumentException("illegal signature");
>
> Why not assign type here?
>
>
> type = switch(signature.charAt(0)) {
> case 'Z' -> Boolean.TYPE;
>
Thanks for your suggestion. I've done that now. See adc8af4
-
PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions [v2]
> Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick Patrick Concannon 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 three additional commits since the last revision: - 8267670: Updated code to use yield - Merge remote-tracking branch 'origin/master' into JDK-8267670 - 8267670: Update java.io, java.math, and java.text to use switch expressions - Changes: - all: https://git.openjdk.java.net/jdk/pull/4182/files - new: https://git.openjdk.java.net/jdk/pull/4182/files/b98e47db..adc8af41 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=4182=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=4182=00-01 Stats: 143 lines in 16 files changed: 28 ins; 56 del; 59 mod Patch: https://git.openjdk.java.net/jdk/pull/4182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182 PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions
On Tue, 25 May 2021 09:37:58 GMT, Patrick Concannon
wrote:
> Hi,
>
> Could someone please review my code for updating the code in the `java.io`,
> `java.math`, and `java.text` packages to make use of the switch expressions?
>
> Kind regards,
> Patrick
src/java.base/share/classes/java/io/ObjectStreamField.java line 123:
> 121: case 'D' -> type = Double.TYPE;
> 122: case 'L', '[' -> type = Object.class;
> 123: default -> throw new IllegalArgumentException("illegal
> signature");
Why not assign type here?
type = switch(signature.charAt(0)) {
case 'Z' -> Boolean.TYPE;
-
PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions
On Tue, 25 May 2021 09:37:58 GMT, Patrick Concannon
wrote:
> Hi,
>
> Could someone please review my code for updating the code in the `java.io`,
> `java.math`, and `java.text` packages to make use of the switch expressions?
>
> Kind regards,
> Patrick
src/java.base/share/classes/java/io/StreamTokenizer.java line 795:
> 793: * case statements
> 794: */
> 795: if (ttype < 256 && ((ctype[ttype] & CT_QUOTE) != 0)) {
Maybe (since its easier to grok the yield rather than the assignment of ret in
branches):
String ret = switch (ttype) {
case TT_EOF -> "EOF";
case TT_EOL -> "EOL";
case TT_WORD-> sval;
case TT_NUMBER -> "n=" + nval;
case TT_NOTHING -> "NOTHING";
default -> {
/*
* ttype is the first character of either a quoted string or
* is an ordinary character. ttype can definitely not be less
* than 0, since those are reserved values used in the previous
* case statements
*/
if (ttype < 256 && ((ctype[ttype] & CT_QUOTE) != 0)) {
yield sval;
}
char s[] = new char[3];
s[0] = s[2] = ''';
s[1] = (char) ttype;
yield new String(s);
}
};
return "Token[" + ret + "], line " + LINENO;
-
PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions
On Tue, 25 May 2021 09:37:58 GMT, Patrick Concannon wrote: > Hi, > > Could someone please review my code for updating the code in the `java.io`, > `java.math`, and `java.text` packages to make use of the switch expressions? > > Kind regards, > Patrick src/java.base/share/classes/java/text/BreakIterator.java line 569: > 567: case SENTENCE_INDEX -> > breakIteratorProvider.getSentenceInstance(locale); > 568: default -> null; > 569: }; No need to use the local variable `iterator` here. Simply return with the switch expression. src/java.base/share/classes/java/text/NumberFormat.java line 982: > 980: case COMPACTSTYLE -> > provider.getCompactNumberInstance(locale, formatStyle); > 981: default-> null; > 982: }; Same as `BreakIterator`. No need for `numberFormat`. - PR: https://git.openjdk.java.net/jdk/pull/4182
Re: RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions
On Tue, 25 May 2021 09:37:58 GMT, Patrick Concannon
wrote:
> Hi,
>
> Could someone please review my code for updating the code in the `java.io`,
> `java.math`, and `java.text` packages to make use of the switch expressions?
>
> Kind regards,
> Patrick
src/java.base/share/classes/java/io/ObjectInputStream.java line 1877:
> 1875: descriptor.checkInitialized();
> 1876: }
> 1877: default -> throw new
> StreamCorruptedException(
What would you think of assigning descriptor with the value returned from
evaluating the switch?
Either:
1)
ObjectStreamClass descriptor = switch (tc) {
case TC_NULL-> (ObjectStreamClass) readNull();
case TC_PROXYCLASSDESC -> readProxyDesc(unshared);
case TC_CLASSDESC -> readNonProxyDesc(unshared);
case TC_REFERENCE -> readAndCheckHandle(unshared);
default -> throw new StreamCorruptedException(String.format("invalid
type code: %02X", tc));
};
return descriptor;
}
, where the body of TC_REFERENCE is enclosed in readAndCheckHandle, OR
2) Simply
case TC_REFERENCE -> {
var d = (ObjectStreamClass)readHandle(unshared);
// Should only reference initialized class descriptors
d.checkInitialized();
yield d; }
-
PR: https://git.openjdk.java.net/jdk/pull/4182
RFR: 8267670: Update java.io, java.math, and java.text to use switch expressions
Hi, Could someone please review my code for updating the code in the `java.io`, `java.math`, and `java.text` packages to make use of the switch expressions? Kind regards, Patrick - Commit messages: - 8267670: Update java.io, java.math, and java.text to use switch expressions Changes: https://git.openjdk.java.net/jdk/pull/4182/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4182=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8267670 Stats: 328 lines in 11 files changed: 1 ins; 187 del; 140 mod Patch: https://git.openjdk.java.net/jdk/pull/4182.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/4182/head:pull/4182 PR: https://git.openjdk.java.net/jdk/pull/4182
