RFR: 8342486: Implement JEP 505: Structured Concurrency (Fifth Preview) [v11]
Alan Bateman
alanb at openjdk.org
Wed May 7 09:31:24 UTC 2025
On Wed, 7 May 2025 07:14:58 GMT, Shaojin Wen <swen at openjdk.org> wrote:
>> Alan Bateman has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 20 commits:
>>
>> - Merge branch 'master' into JDK-8342486
>> - Merge branch 'master' into JDK-8342486
>> - Merge branch 'master' into JDK-8342486
>> - Sync up from loom repo
>> - Merge branch 'master' into JDK-8342486
>> - Add JEP number, update copyright headers
>> - Merge branch 'master' into JDK-8342486
>> - Sync up from loom repo
>> - Merge branch 'master' into JDK-8342486
>> - Sync up from loom repo
>> - ... and 10 more: https://git.openjdk.org/jdk/compare/0eb680ca...5fec717f
>
> src/java.base/share/classes/java/util/concurrent/Joiners.java line 84:
>
>> 82: @SuppressWarnings("unchecked")
>> 83: var s = (Subtask<T>) subtask;
>> 84: subtasks.add(s);
>
> Suggestion:
>
> subtasks.add((Subtask<T>) subtask);
>
> The local variable s is only used once and can be simplified
No, it has to be a local to avoid broadening the use of SW.
> src/java.base/share/classes/java/util/concurrent/Joiners.java line 155:
>
>> 153: case FAILED -> throw subtask.exception();
>> 154: default -> throw new InternalError();
>> 155: };
>
> Suggestion:
>
> return switch (subtask.state()) {
> case SUCCESS -> subtask.get();
> case FAILED -> throw subtask.exception();
> default -> throw new InternalError();
> };
>
> The switch of the stateToInt method is aligned, so it should also be aligned here to keep the style consistent
I'll look to re-align these in the loom repo for the next refresh.
> src/java.base/share/classes/java/util/concurrent/StructuredTaskScope.java line 820:
>
>> 818: @PreviewFeature(feature = PreviewFeature.Feature.STRUCTURED_CONCURRENCY)
>> 819: final class TimeoutException extends RuntimeException {
>> 820: @java.io.Serial
>
> Use import without full path
I think `@java.io.Serial` is okay here, you'll see the same in many other areas.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r2077217919
PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r2077215677
PR Review Comment: https://git.openjdk.org/jdk/pull/21934#discussion_r2077222469
More information about the core-libs-dev
mailing list