RFR: 8350983: JShell LocalExecutionControl only needs stopCheck() on backward branches [v2]

[Archie Cobbs]

On Tue, 22 Apr 2025 11:36:27 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> Archie Cobbs has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Niceify using pattern matching switch with guard per review suggestion.
>
> src/jdk.jshell/share/classes/jdk/jshell/execution/LocalExecutionControl.java line 99:
> 
>> 97:                         ClassTransform.transformingMethodBodies(
>> 98:                             CodeTransform.ofStateful(() -> {
>> 99:                                 HashSet<Label> priorLabels = new HashSet<>();
> 
> Nit: could be `Set<Label> priorLabels = new HashSet<>();`.

Thanks - fixed in 16a39b28197.

> src/jdk.jshell/share/classes/jdk/jshell/execution/LocalExecutionControl.java line 102:
> 
>> 100:                                 return (builder, element) -> {
>> 101:                                     switch (element) {
>> 102:                                     case LabelTarget target -> priorLabels.add(target.label());
> 
> I think we usually indentat for `case`/`default`, right? (The old way of putting `case`/`default` at the same indentation level as was the switch is not really used in new code, or is it?)

In `jdk.compiler` both variants and appear and there does not appear to be a clear winner. In `jdk.jshell` it appears that indenting is the standard approach.

So we should try to be consistent with the local environment and indent. Thanks - fixed in 16a39b28197.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23850#discussion_r2054213354
PR Review Comment: https://git.openjdk.org/jdk/pull/23850#discussion_r2054216357