Skip to content

Spurious "Unreachable case" warning in a match on an ADT #14807

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
cb372 opened this issue Mar 29, 2022 · 7 comments · Fixed by #15109
Closed

Spurious "Unreachable case" warning in a match on an ADT #14807

cb372 opened this issue Mar 29, 2022 · 7 comments · Fixed by #15109

Comments

@cb372
Copy link
Contributor

cb372 commented Mar 29, 2022

Compiler version

3.1.1

Minimized code

enum Foo:
  case One(value: String)
  case Two(value: Long, month: java.time.Month)

object Issue:

  def doSomething(foo: Foo): String = foo match {
    case Foo.One(x)    => s"1 $x"
    case Foo.Two(x, y) => s"2 $x $y"
  }

Output

[warn] ./patmatbug.scala:9:10: Unreachable case
[warn]     case Foo.Two(x, y) => s"2 $x $y"
[warn]          ^^^^^^^^^^^^^

Expectation

Compiles without warnings, like Scala 2.13.

Notes

Surprisingly, if I switch the lines around to make case Foo.Two(x, y) the very first case in the match, the warning goes away.

@cb372 cb372 added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 29, 2022
@KacperFKorban KacperFKorban added area:pattern-matching and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Mar 29, 2022
@KacperFKorban KacperFKorban changed the title Spurious "Unreachable case" warning when case references a Java enum Spurious "Unreachable case" warning in a match on an ADT Mar 29, 2022
@anatoliykmetyuk
Copy link
Contributor

Minimized to:

case class Two(month: java.time.Month)

def doSomething(foo: Any) = foo match
  case Two(x) =>
  case _ =>

Intermediate cause:

https://github.com/lampepfl/dotty/blob/4a96ce7374cdb258c22facd18925583a5e09adaa/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala#L629

The pattern matcher tries to see if the space of all values covered by Month is empty. If it is, then the case is declared unreachable. One of the rules of such an inference for Java enums is to see if the list of their cases is empty. If it is, the space of the entire enum is assumed empty.

In this case, on the line above, the decomposition algorithm thinks that the Month enum is empty since tp.classSymbol.children is an empty list – although it shouldn't be.

The question is why is it empty?

@odersky
Copy link
Contributor

odersky commented Apr 3, 2022

The children in the original enum case are not empty, though. I see: List(class One, class Two).

@odersky
Copy link
Contributor

odersky commented Apr 3, 2022

Surprisingly, if I switch the lines around to make case Foo.Two(x, y) the very first case in the match, the warning goes away.

I could not reproduce that either. For me, the warning appears no matter what the case ordering is.

@dwijnand
Copy link
Member

dwijnand commented Apr 3, 2022

Yeah, I changed the way warnings are emitted so those reorderings don't affect the behaviour, but I'm guessing that fix isn't released or wasn't the version Chris was running.

I'll look into why Month has no children, which I agree with @anatoliykmetyuk seems like is the root cause. On the side, I think the behaviour of considering the case unreachable if it contains an uninhabited type seems reasonable to me: like a sealed trait with no children should cause the outer case to be unreachable.

@odersky
Copy link
Contributor

odersky commented Apr 3, 2022

Month has no children since it is a java class. We record children only for sealed classes. For other classes we cannot possibly do that, since children might be in separate compilation units that we have not seen.

@anatoliykmetyuk
Copy link
Contributor

Month has no children since it is a java class. We record children only for sealed classes. For other classes we cannot possibly do that, since children might be in separate compilation units that we have not seen.

This means the following code is wrong by definition:

https://github.com/lampepfl/dotty/blob/4a96ce7374cdb258c22facd18925583a5e09adaa/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala#L628-L629

It will always have empty children.

@odersky
Copy link
Contributor

odersky commented Apr 3, 2022

Ah, I guess Java enums should have children. Not sure whether that's the case in general.

@dwijnand dwijnand linked a pull request May 5, 2022 that will close this issue
@Kordyjan Kordyjan added this to the 3.2.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.

7 participants