Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support Match nodes with non-int-literals in the back-end. #3842

Merged
merged 1 commit into from Nov 13, 2019

Conversation

sjrd
Copy link
Member

@sjrd sjrd commented Nov 11, 2019

See scala/scala#8451 upstream (not yet merged, but fully approved)


Since Scala 2.13.2, the pattern matcher will keep Match nodes that match on Strings and nulls as is, to be desugared later in cleanup. This was implemented upstream in scala/scala#8451

We implement a more general translation that will accept any kind of scalac Literal. If the scrutinee is an integer and all cases are int literals, we emit a js.Match as before. Otherwise, we emit an if..else chain with === comparisons.


Locally tested with:

> set resolvers in Global += "scala-pr-validation" at "https://scala-ci.typesafe.com/artifactory/scala-pr-validation-snapshots/"
> ++2.13.2-bin-9f25ca0-SNAPSHOT
> testSuite/test
> compiler/test
[...] copy the big test case from the PR in HelloWorld.scala and:
> helloworld/run

As noted at scala/scala#8451 (comment), 0.6.x already works out of the box with the change upstream.

It actually raises the question whether we should revert the design of our js.Match to accept any kind of js.Literals and really iron out the linker (notably for IR literals that are not JS literals, such as ClassOf and LongLiteral) instead of changing the compiler back-end. This PR implements the less disruptive change.

Since Scala 2.13.2, the pattern matcher will keep `Match` nodes
that match on `String`s and `null`s as is, to be desugared later
in `cleanup`. This was implemented upstream in
scala/scala#8451

We implement a more general translation that will accept any kind
of scalac `Literal`. If the scrutinee is an integer and all cases
are int literals, we emit a `js.Match` as before. Otherwise, we
emit an `if..else` chain with `===` comparisons.
@gzm0
Copy link
Contributor

gzm0 commented Nov 13, 2019

It actually raises the question whether we should revert the design of our js.Match to accept any kind of js.Literals and really iron out the linker (notably for IR literals that are not JS literals, such as ClassOf and LongLiteral) instead of changing the compiler back-end.

We should keep this as a a consideration, especially as it would simplify the emitted code IIUC.

@gzm0 gzm0 merged commit e936801 into scala-js:master Nov 13, 2019
@sjrd sjrd deleted the string-switch branch November 13, 2019 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants