Fix #3144: emit warnings for unchecked type patterns

[liufengyun]

Fix #3144: emit warnings for unchecked type patterns

[smarter]

I think you meant to write "are not checked"

[allanrenucci]

I think there are two separate issues described in #3144:

• Missing unchecked warning for type pattern that are eliminated by erasure. This seems to be fixed in this PR. Do we want to support the @unchecked annotation like scalac?

class Test {
  def f(x: Any): Int = x match {
    case xs: List[Int] @unchecked => xs.head
  }
}

• Unreachable case for unrelated type when one the type is a sealed trait. Maybe I am missing something but the case seems unreachable to me (scalac doesn't emit a warning either).

sealed trait Foo
class Bar
  
class Test {
  def shouldError(foo: Foo) = foo match {
    case _: Bar => 1
  }
}

[liufengyun]

@allanrenucci The second is not an issue, as there could be a class like the follows:

class Ruu extends Bar with Foo

[allanrenucci]

@liufengyun But Foo is sealed, so we know there is no such class Ruu, right?

[liufengyun]

Ah I see, currently the algorithm just assumes the first case is always reachable to save some computation. We could also check if the first branch is reachable or not, but I'm not sure if it's worth it.

BTW, we've checks for hopeless matches, which are located in IsInstanceOfEvaluator.

[allanrenucci]

Ah I see, currently the algorithm just assumes the first case is always reachable to save some computation.

If Foo is a class dotc gives me an error for the first case though:

-- Error: tests/allan/Test.scala:6:9 -------------------------------------------
6 |    case _: Foo => 1
  |         ^
  |      this case is unreachable since class Bar and class Foo are unrelated

one error found

[liufengyun]

@allanrenucci That check is from Erasure, not from the exhaustivity check. The Erasure handles it more generally by checking if isInstanceOf is sensical.

[allanrenucci]

You might want to add a test case for

case xs: List[Int @unchecked] =>

[allanrenucci]

You should extract this case into another function. This case is unreachable here

[smarter]

This is a very specific kind, kinds are supposed to apply to more than one message, I guess here it could be "Pattern Match Exhaustivity" (kinds should also be an enum, but that's a separate issue).

[smarter]

The explanation could mention possible fixes: either replace the type arguments by _ or use @unchecked.

[smarter]

Instead of passing an implicit around, you could have an outer function which defines warn and an inner recursive function which uses it.

[liufengyun]

@smarter thanks a lot, I've addressed other comments. For this one, doing the refactoring would require to pass an additional Position to erase, which will make the API a little ugly.