Emit efficient code for switch over strings #11937
Conversation
| I'm not sure how to make tests that check the backend code generation, in order to assert that we really do end up seeing a |
| Bytecode tests usually go in https://github.com/lampepfl/dotty/blob/master/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala which makes use of https://github.com/lampepfl/dotty/blob/master/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTest.scala which defines a verifySwitch method you could try to reuse. |
For JS you won't be able to assert that, because in fact Scala.js 1.x has no way to represent switches on Strings in its IR. So they are compiled as if/else chains at the moment. There is an issue upstream to address that (scala-js/scala-js#3843), but it need not concern you here for now. |
| * Int/String values to use as keys, and a code block. The exception is the "default" case | ||
| * clause which doesn't list any key (there is exactly one of these per match). | ||
| */ | ||
| private def genMatch(tree: Match): BType = tree match { |
There was a problem hiding this comment.
Note that the bytecode this generates ends up having a number of bogus goto,nop,..,nop,athrow sequences in it. Scala 2 doesn't have these because it does some extra dead-code elimination to clean up junk like this (the extra instructions are produced by ClassWriter). I hope to get around to that at some point, but likely not in this PR.
harpocrates force-pushed the alec/string-switch branch from c78020b to 02b0cb4 Compare 
dwijnand left a comment
dwijnand left a comment There was a problem hiding this comment.
This is a long one to review, so this is just a first comment from what I looked at.
| Hey @harpocrates do you need any more help with this?, a rebase should fix the CI failure |
harpocrates force-pushed the alec/string-switch branch from 449eeaa to 3b4ed4b Compare 
The pattern matcher will now emit `Match` with `String` scrutinee as well as the existing `Int` scrutinee. The JVM backend handles this case by emitting bytecode that switches on the String's `hashCode` (this matches what Java does). The SJS already handles `String` matches. The approach is similar to scala/scala#8451 (see scala/bug#11740 too), except that instead of doing a transformation on the AST, we just emit the right bytecode straight away. This is desirable since it means that Scala.js (and any other backend) can choose their own optimised strategy for compiling a match on strings. Fixes scala#11923
harpocrates force-pushed the alec/string-switch branch from 3b4ed4b to 3d8fa54 Compare | @bishabosha I don't think there is anything else I'm supposed to do - this is just waiting on a review. I just rebased (and flattened the uninteresting history). I did not realize anything was waiting on that - thank you for the ping! |
| I think it would be nice to include a few sentences about this in the release notes |
6 participants
The pattern matcher will now emit
MatchwithStringscrutinee aswell as the existing
Intscrutinee. The JVM backend handles this caseby emitting bytecode that switches on the String's
hashCode(thismatches what Java does). The SJS already handles
Stringmatches.The approach is similar to scala/scala#8451 (see scala/bug#11740 too),
except that instead of doing a transformation on the AST, we just emit the
right bytecode straight away. This is desirable since it means that
Scala.js (and any other backend) can choose their own optimised strategy
for compiling a match on strings.
Fixes #11923