On Thu, 30 Sep 2021 19:11:31 GMT, Jonathan Gibbons <[email protected]> wrote:
>> For the sake of this review, using `switch` would be better: fewer changes,
>> less cognitive load. Later we can think of ways of improving that code. It
>> would be nice to structure it such that it would fail at compile time, if we
>> forgot a case.
>
> It's hard to know how to make it fail at compile time. There are essentially
> 3 groups of trees ... inline tags (and other trees that use `DCEndPosTree`),
> block tags (the big block of cases round about line 177), and "other". We
> could arguably move block tags into the `default` case and use a type test,
> which would improve the future-safety. We could arguably catch missing cases
> at runtime with suitable use of `IllegalStateException`.
>
> As for tests, we *do* have tests for `ProvidesTree` and `UsesTree`, but the
> new `RangeChecker` didn't catch the missing case labels because the basic
> checks in `RangeChecker` were satisfied by the code in the default case in
> the switch statement.
I agree that it might prove hard, if at all possible. That said, we should
investigate this later, not in this PR. One potential direction for
investigation would be "switching on sealed interfaces". A class or an
interface can be a part of multiple sealed-interface hierarchies:
sealed interface Tag permits Block, Inline {}
sealed interface Block extends Tag permits Block1, Bimodal {}
non-sealed interface Block1 extends Block {}
sealed interface Inline extends Tag permits Inline1, Bimodal {}
non-sealed interface Inline1 extends Inline {}
non-sealed interface Bimodal extends Block, Inline {}
After pattern matching for `switch` (currently in preview via JEP 406) has been
standardized, the exhaustiveness of a switch could help us catch bugs at
compile time. Possibly.
-------------
PR: https://git.openjdk.java.net/jdk/pull/5510
