Skip to content

Treat opaque types as abstract during patmat exhaustivity tests #5467

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

Open
abgruszecki opened this issue Nov 19, 2018 · 5 comments
Open

Treat opaque types as abstract during patmat exhaustivity tests #5467

abgruszecki opened this issue Nov 19, 2018 · 5 comments

Comments

@abgruszecki
Copy link
Contributor

Compiling the following code:

object opaq {
  opaque type Pos = Int

  enum E[T] {
    case IntLit(i: Int) extends E[Int]
    case PosLit(p: Pos) extends E[Pos]
    case StrLit(s: String) extends E[String]
  }

  def eval(e: E[Pos]): Pos = e match {
    case E.PosLit(p) => p
  }
}

emits a warning like this:

10: Pattern Match Exhaustivity: E.IntLit(_)

and it should emit a warning like this:

10: Pattern Match Exhaustivity: E.StrLit(_), E.IntLit(_)  

See discussion starting around #5300 (comment).

To sum up the discussion we had about this, the issue happens because opaque aliases are revealed during ElimOpaque phase, which happens before exhaustivity checks. To fix this, their order should be reversed. We should be careful with moving ElimOpaque further (in particular we should not move it right before erasure), since warnings & optimisations currently after ElimOpaque may actually want to see through the opaque alias. It may be easier to instead move exhaustivity checks before ElimOpaque.

@lostintime
Copy link

I'm not sure, but this seems to be the most relevant to issue I got into:

I would expect it to match on newtype, or at least fail at compile time

opaque type Email = String
object Email {
  def apply(email: String): Email = email
}

opaque type Phone = String
object Phone {
  def apply(nr: String): Phone = nr
}

type EmailOrPhone = Email | Phone

object Main {
  def sayIt(u: EmailOrPhone): String = u match {
    case Email => "It's an email"
    case Phone => "It's a phone number"
  }

  def main(args: Array[String]): Unit = {
    println(sayIt(Email("somebody@somewhere")))
  }
}

This compiles but fails with:

...
[info] Done packaging.
[info] Running Main
[error] (run-main-0) scala.MatchError: somebody@somewhere (of class java.lang.String)
[error] scala.MatchError: somebody@somewhere (of class java.lang.String)
[error] 	at Main$.sayIt(Main.scala:18)
[error] 	at Main$.main(Main.scala:22)
[error] 	at Main.main(Main.scala)
[error] 	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
[error] 	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
[error] 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
[error] 	at java.lang.reflect.Method.invoke(Method.java:498)
[error] Nonzero exit code: 1
[error] (Compile / run) Nonzero exit code: 1

What behavior is designed for this case?

@smarter
Copy link
Member

smarter commented Jun 12, 2019

case Email =>

You're matching on the value of the object Email, not the type Email which you'd get by writing case _: Email =>, the compiler is indeed missing some checks here.

@lostintime
Copy link

@smarter ohh, didn't think of that :). Then here is the next one:

opaque type Email = String
object Email {
  def apply(email: String): Email = email
}

opaque type Phone = String
object Phone {
  def apply(nr: String): Phone = nr
}

type EmailOrPhone = Email | Phone

object Main {
  def sayIt(u: EmailOrPhone): String = u match {
    case _: Phone => "It's a phone number"
    case _: Email => "It's an email"
  }

  def main(args: Array[String]): Unit = {
    println(sayIt(Email("somebody@somewhere")))
    println(sayIt(Phone("123123123")))
  }
}

It gives a warning, but I probably have wrong expectations from opaque types:

[info] Compiling 1 Scala source to ./target/scala-0.16/classes ...
[warn] -- [E030] Match case Unreachable Warning: ./src/main/scala/Main.scala:18:9
[warn] 18 |    case _: Email => "It's an email"
[warn]    |         ^^^^^^^^
[warn]    |         Unreachable case
[warn] one warning found
[info] Done compiling.
[info] Packaging ./target/scala-0.16/dotty-simple_0.16-0.1.0.jar ...
[info] Done packaging.
[info] Running Main
It's a phone number
It's a phone number

@smarter
Copy link
Member

smarter commented Jun 12, 2019

Yes, this is normal, case _: Email and case _: Phone are both erased to case _: String, you cannot distinguish between them, this is inevitable for opaque types.

@abgruszecki
Copy link
Contributor Author

For the record, we very likely should always emit a warning whenever we see a typecase pattern for an opaque type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants