Add the StringSwitch and EnumSwitch constructs

[mkouba]

This pull request introduces the StringSwitch and EnumSwitch constructs that can be used to generate bytecode similar to the Java switch.

API usage examples:

// switch(someString) {
//     case "boom":
//     case "foo":
//          return "fooo";
//     case "bar":
//          return "barr"
//      default:
//          return null;
// }
StringSwitch s = method.stringSwitch(someStringResultHandle);
s.caseOf(List.of("boom", "foo"), bc -> {
  bc.returnValue(bc.load("fooo"));
});
s.caseOf("bar", bc -> {
  bc.returnValue(bc.load("barr"));
});
s.defaultCase(bc -> bc.returnNull());

// switch(status) {
//     case ON:
//     case OFF:
//          return status.toString();
//     case UNKNOWN:
//          return "?";
//      default:
//          return null;
// }
EnumSwitch<Status> s = method.enumSwitch(method.getMethodParam(0), Status.class);
s.caseOf(List.of(Status.ON, Status.OFF), bc -> {
   bc.returnValue(Gizmo.toString(bc, method.getMethodParam(0)));
});
s.caseOf(Status.UNKNOWN, bc -> {
   bc.returnValue(bc.load("?"));
});
s.defaultCase(bc -> bc.returnNull());

The StringSwitch is implemented with the lookupswitch instruction while the EnumSwitch is implemented with the tableswitch instruction. In general, the resulting bytecode is very similar to the bytecode generated by javac.

We could also introduce a switch for int but enums and strings are probably more common.

[Ladicek]

I didn't review properly (yet), but I've got 2 questions about the API:

1. What about collapsing all the case values for a single "branch" to one method call? I know that

s.caseOf("boom");
s.caseOf("foo", bc -> {
  bc.returnValue(bc.load("fooo"));
});

looks closer to the corresponding Java source code, but wouldn't

s.caseOf(Set.of("boom", "foo"), bc -> {
  bc.returnValue(bc.load("fooo"));
});

actually be a better API?

2. How about we ditch fall through? At least by default. Falling through is almost never what you want, and having to explicitly add the break statement is a chore. In my opinion, s.caseOf(...) should not fall through, and if we want to support it, we should have an extra method that opts into the fall through behavior (e.g. s.caseWithFallThrough(...) or so).

[mkouba]

I didn't review properly (yet), but I've got 2 questions about the API:

1. What about collapsing all the `case` values for a single "branch" to one method call? I know that

s.caseOf("boom");
s.caseOf("foo", bc -> {
  bc.returnValue(bc.load("fooo"));
});

looks closer to the corresponding Java source code, but wouldn't

s.caseOf(Set.of("boom", "foo"), bc -> {
  bc.returnValue(bc.load("fooo"));
});

actually be a better API?

+1

But I wonder if we need to keep the ordering or not..? I mean the result should be the same but the evaluation order not.

2. How about we ditch fall through? At least by default. Falling through is almost never what you want, and having to explicitly add the `break` statement is a chore. In my opinion, `s.caseOf(...)` should _not_ fall through, and if we want to support it, we should have an extra method that _opts into_ the fall through behavior (e.g. `s.caseWithFallThrough(...)` or so).

I'm not so sure it's reasonable to "break" by default. I mean it's not "expected".

[Ladicek]

I think the case order doesn't matter (I mean the order of constants in the Java source code / Gizmo API calls), the generated bytecode is always ordered numerically (at least for tableswitch, and lookupswitch has some ordering in there too IIRC). But I wrote Set without really thinking about it -- it could just as well be List.

Fall through is a contentious topic, because Java has it and we should be close to Java, but my Strong Opinion here is that Java should have never copied this from C(++) and Go did the Right Thing (https://golangbot.com/switch/#fallthrough). I guess I don't mind too much though.

[mkouba]

and lookupswitch has some ordering in there too IIRC

Hm, you're right that the lookupswitch keys must be sorted in increasing numerical order and we use the hashcode as the key. So in theory it should not really matter.

[dmlloyd]

I'm not so sure it's reasonable to "break" by default. I mean it's not "expected".

Java 17's new switch syntax works in this way FWIW.

[mkouba]

I'm not so sure it's reasonable to "break" by default. I mean it's not "expected".

Java 17's new switch syntax works in this way FWIW.

Hm, you mean the case L -> syntax. Fair enough.

We could just provide two variants of the method. One for the "switch labeled statement group" (case L:) and one for the "switch rule block" (case L ->).

We could even add the fallThrough() method to the Switch to control the default behavior. Because otherwise we would have to check if different kinds of case blocks are used etc. This way we could keep one method for case blocks.

// equivalent of case L ->
StringSwitch s1 = method.stringSwitch(val);
s1.caseOf("foo", bc -> bc.assign(ret, bc.load("fooo")));
s1.caseOf("bar", bc -> bc.assign(ret, bc.load("barr")));

// equivalent of case L:
StringSwitch s2 = method.stringSwitch(val);
s.fallThrough();
s2.caseOf("foo", bc -> bc.assign(ret, bc.load("fooo")));
s2.caseOf("bar", bc -> bc.assign(ret, bc.load("barr")));

@Ladicek @dmlloyd WDYT?

[mkouba]

I've added the Switch.fallThrough() method.

[Ladicek]

Sorry it took me a while, I had to browse through this in my IDE and look at the generated bytecode to be able to understand what's going on. LGTM.

[Ladicek]

(I also originally wanted to say that perhaps the fallThrough() method should be withFallThrough() and return a new Switch instance, because the current setup allows changing the "fall through" flag in between cases, which is pretty terrible, but I see that wouldn't be super-easy to do, so... I don't mind.)

[mkouba]

Sorry it took me a while, I had to browse through this in my IDE and look at the generated bytecode to be able to understand what's going on. LGTM.

No problem.

(I also originally wanted to say that perhaps the fallThrough() method should be withFallThrough() and return a new Switch instance, because the current setup allows changing the "fall through" flag in between cases, which is pretty terrible, but I see that wouldn't be super-easy to do, so... I don't mind.)

Well, the Switch construct is mutable and it's not intended to be reused. Also you can't mix the syntax (L-> and L:) in Java so I think that it's fine.

[mkouba]

FYI I need to send a follow-up PR to fix the problem when using result handles from the enclosing scope.