Skip to content

Incorrect inexhaustivity warning when leafs are defined in an unrelated sealed trait #15029

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

Closed
kitlangton opened this issue Apr 25, 2022 · 7 comments · Fixed by #16621
Closed

Comments

@kitlangton
Copy link

kitlangton commented Apr 25, 2022

Compiler version

3.1.1

Minimized code

Here is a Scastie.

The exhaustivity check fails when instances of Schema are defined in a trait.

sealed trait Schema[A]

object Schema extends RecordInstances

sealed trait RecordInstances:
  case class Field[A]() extends Schema[A]
  case object Thing extends Schema[Int]

import Schema._

// Match not exhaustive error! (with fatal warnings :P)
def handle[A](schema: Schema[A]) =
  schema match
    case Field() => println("field")
    case Thing   => println("thing")

Expectation

While it is a bit odd to define these instances in a trait, I would still expect this to correctly check exhaustivity. Scala 2 works as expected here.

@kitlangton kitlangton added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 25, 2022
@kitlangton kitlangton changed the title Unexpective "Non-Exhaustive Match" Error Unexpected "Non-Exhaustive Match" Error Apr 25, 2022
@kitlangton
Copy link
Author

kitlangton commented Apr 25, 2022

On second thought. Maybe this is more sound. I don't know why the code I'm working on was originally organized like this—I think I can merely remove these intermediate traits. Feel free to close this if you agree! But I'll leave it open in case this is interesting. It causes another similar compiler error here (https://scastie.scala-lang.org/2ME3I1HIQuyUAe67T2jziw)

@som-snytt
Copy link
Contributor

For example,

@main def test() = println {
  val r = new RecordInstances {}
  val s = r.Thing
  handle(s)
}

@smarter
Copy link
Member

smarter commented Apr 26, 2022

There is an exhaustiveness issue here since RecordInstances isn't sealed (but the error message wording is a bit confusing: it says RecordInstances.Field() when it should say something like _: RecordInstances#Field). However, if RecordInstances is sealed the error doesn't go away when it should (but it's a pretty weird thing to do indeed).

@smarter smarter changed the title Unexpected "Non-Exhaustive Match" Error exhaustivity issue involving leafs in unrelated trait Apr 26, 2022
@smarter smarter added area:pattern-matching and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Apr 26, 2022
@kitlangton
Copy link
Author

kitlangton commented Apr 26, 2022

@smarter @som-snytt Apologies. It actually still fails: https://scastie.scala-lang.org/WwDjucNPS4ekVwn2977OKA
Forgot to seal both in my example (updating my issue link now)

Oh. You already realized that 😆 Misunderstood your comment whoops :)

@som-snytt
Copy link
Contributor

Also Scala 2 warns. It would fail on the following inputs: Field(), Thing, _

@smarter
Copy link
Member

smarter commented Apr 26, 2022

It causes another similar compiler error here (scastie.scala-lang.org/2ME3I1HIQuyUAe67T2jziw)

That looks like an unrelated bug (missing substitution of RecordInstances.this into Schema), could you open a separate issue for it?

@kitlangton
Copy link
Author

Also Scala 2 warns. It would fail on the following inputs: Field(), Thing, _

Oh yeah... Don't know why that didn't trigger fatal warnings for me in Scala 2. The solution for me was to replace these traits with // RECORD INSTANCES comment headers instead 😛. I think someone just wanted the organizational niceties of traits to organize a very large sealed trait file.

@smarter smarter changed the title exhaustivity issue involving leafs in unrelated trait Incorrect inexhaustivity warning when leafs are defined in an unrelated sealed trait Apr 26, 2022
@dwijnand dwijnand self-assigned this Jan 5, 2023
@dwijnand dwijnand linked a pull request Jan 5, 2023 that will close this issue
@SethTisue SethTisue self-assigned this Jan 5, 2023
@Kordyjan Kordyjan added this to the 3.3.0 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants