On Tue, 4 May 2021 20:48:34 GMT, Maurizio Cimadamore <mcimadam...@openjdk.org> wrote:
>> Jan Lahoda has updated the pull request incrementally with three additional >> commits since the last revision: >> >> - Reflecting recent spec changes. >> - Reflecting review comments. >> - Reflecting review comments on SwitchBootstraps. > > Good work. There's a lot to take in here. I think overall, it holds up well - > I like how case labels have been extended to accommodate for patterns. In the > front-end, I think there are some questions over the split between Attr and > Flow - maybe it is unavoidable, but I'm not sure why some control-flow > analysis happens in Attr instead of Flow and I left some questions to make > sure I understand what's happening. > > In the backend it's mostly good work - but overall the structure of the code, > both in Lower and in TransPattern is getting very difficult to follow, given > there are many different kind of switches each with its own little twist > attached to it. It would be nice, I think (maybe in a separate cleanup?) if > the code paths serving the different switch kinds could be made more > separate, so that, when reading the code we can worry only about one possible > code shape. That would make the code easier to document as well. @mcimadamore, @forax, thanks a lot for comments! I tried to apply the requested changes in recent commits (https://github.com/openjdk/jdk/pull/3863/commits/3fc2502a33cec20f39fe571eb25538ee3b459a9b https://github.com/openjdk/jdk/pull/3863/commits/aeddb85e65bf77ed62dc7fa1ad00c29646d02c99 ). I've also tried to update the implementation for the latest spec changes here: https://github.com/openjdk/jdk/pull/3863/commits/54ba974e1aac00bbde1c3bd2627f01caaaeda09b The spec changes are: total patterns are nullable; pattern matching ("new") statement switches must be complete (as switch expressions). Any further feedback is welcome! ------------- PR: https://git.openjdk.java.net/jdk/pull/3863
