Fix #3561: Correctly handle switches followed by other tests in matches

[odersky]

No description provided.

[smarter]

Trying to test this I noticed that something seems very broken with @switch, we get the Could not emit switch for @switch annotated match warning in all these cases (both before and after this PR):

import scala.annotation.switch

object Test {
  def foo1(x: Int) = (x: @switch) match {
    case 1 =>
      println("a")
  }
  def foo2(x: Int) = (x: @switch) match {
    case 1 =>
      println("b")
    case 2 =>
      println("c")
  }
  def foo3(x: Int) = (x: @switch) match {
    case 1 | 2 =>
      println("d")
  }
}

However, this one works somehow:

import scala.annotation.switch

object Test {
  def foo4(x: Int) = (x: @switch) match {
    case 1 =>
      println("e")
    case 2 | 3 =>
      println("f")
  }
}

[nicolasstucki]

@smarter, when there are 1 or 2 (maybe 3 with default) branches in the pattern match the pattern is always compiled with if/else even if it could potentially do a switch. That is why the first ones fail the @switch tests. We should probably emit a better warning message or just disregard it in those cases.

[smarter]

scalac supports emitting switch when cases contain conditions, it translates something like:

  def foo(x: Int, cond: Boolean) = x match {
    case 1 if cond =>
      println("1")
    case 2 =>
      println("2")
    case 3 =>
      println("3")
    case _ =>
      println("4")
  }

to

      x1 match {
        case 1 => if (cond)
          scala.Predef.println("1")
        else
          default5()
        case 2 => scala.Predef.println("2")
        case 3 => scala.Predef.println("3")
        case _ => default5(){
          scala.Predef.println("4")
        }
      }

Is this an intended limitation of Dotty @switch, or something that still needs to be implemented?

[nicolasstucki]

This was supposed to go in the local optimizations. I made an implementation of this a while ago but I do not know if it went into Dotty or just stayed in the linker.

[smarter]

I think it would make sense to have it by default. Could you open an issue so we keep track of it?

[smarter]

Could you open an issue so we keep track of it?

Turns out there's already one: #1313

[nicolasstucki]

#2840 is fixed, it should be safe to remove the ascription

[nicolasstucki]

This PR needs a rebase