False unreachable warning

[odersky]

Given:

object Test {
  sealed trait SearchResult
  case class Success() extends SearchResult
  val Fail = new SearchResult {}
  val x: SearchResult = ???

  x match {
    case x: Success => x
    case x => x
  }
}

we get:

-- [E030] Match case Unreachable Warning: unreachable.scala --------------------
9 |        case x => x
  |               ^^^^
  |               unreachable code

one warning found

The warning should not be issued, since the line is reachable.

[smarter]

I think I suggested somewhere that we simply disallow anonymous classes that directly extend sealed traits, since they make it impossible to satisfy exhaustivity checks without adding default cases and can easily be rewritten to something like new OtherResult {} after defining class OtherResult extends SearchResult, what do you think?

[liufengyun]

Seems I missed the notification. Anonymous class is a known issue. Here is the PR note we have before:

1. Anonymous class

In dotty, an anonymous class can inherit a sealed trait. However, 
if we add anonymous class to the trait's children annotations, there 
would be a pickling exception.

Guillaume suggests it might be good to forbid creating anonymous 
classes from sealed base.

I think @smarter 's idea makes sense. WDYT @odersky ?

[odersky]

I think we need anonymous classes in sealed traits for a good treatment of enums. /cc @retronym who suggested this to me.

One way to deal with this currently is to not generate a @children annotation when anonymous classes are encountered.

[DarkDimius]

In case we want to support anonymous-classes, we can also lazily auto-generate a super-class Other for them that extends the sealed class. The Other will get @children annotation, anonymous subclasses won't.

[smarter]

I think we need anonymous classes in sealed traits for a good treatment of enums. /cc @retronym who suggested this to me.

Looking at https://github.com/lampepfl/dotty/pull/1918/files I don't see how you could get nice exhaustiveness messages from something like val Red: Color = $new(0, "Red") without special-casing the exhaustiveness checker for enums. Couldn't you just use case object instead, with the twist that instead of generating a companion val val Red: Red$ = new Red$() it would be val Red: Color = new Red$() ?

[odersky]

Yes, I think we need to specialize the exhaustiveness checker for enums. We cannot replace case object Red extends Color by val Red: Color = new ... because the widened types are different (Color, respectively the module class of Red). - Martin

…

On Fri, Feb 3, 2017 at 11:25 AM, Guillaume Martres ***@***.*** > wrote: I think we need anonymous classes in sealed traits for a good treatment of enums. /cc @retronym <https://github.com/retronym> who suggested this to me. Looking at https://github.com/lampepfl/dotty/pull/1918/files I don't see how you could get nice exhaustiveness messages from something like val Red: Color = $new(0, "Red") without special-casing the exhaustiveness checker for enums. Couldn't you just use case object instead, with the twist that instead of generating a companion val val Red: Red$ = new Red$() it would be val Red: Color = new Red$() ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1917 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAwlViFMwvjA2PKGFx4fGXCGBCgS1l7yks5rYnP2gaJpZM4LyNC-> . {"api_version":"1.0","publisher":{"api_key":" 05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity": {"external_key":"github/lampepfl/dotty","title":" lampepfl/dotty","subtitle":"GitHub repository","main_image_url":" https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88- 11e6-95fc-7290892c7bb5.png","avatar_image_url":"https:// cloud.githubusercontent.com/assets/143418/15842166/ 7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/lampepfl/dotty"}}," ***@***.*** in #1917: \u003e I think we need anonymous classes in sealed traits for a good treatment of enums. /cc @retronym who suggested this to me.\r\n\r\nLooking at https://github.com/lampepfl/dotty/pull/1918/files I don't see how you could get nice exhaustiveness messages from something like `val Red: Color = $new(0, \"Red\")` without special-casing the exhaustiveness checker for enums. Couldn't you just use `case object` instead, with the twist that instead of generating a companion val `val Red: Red$ = new Red$()` it would be `val Red: Color = new Red$()` ?"}],"action":{"name":"View Issue","url":" #1917 (comment)"}}}

-- Prof. Martin Odersky LAMP/IC, EPFL

[liufengyun]

If the purpose to support exhaustivity check of the new enum-style ADTs, then there is an easy solution. We only need to pass the list of children from parser to the exhaustivity checker (some trick in type checking required to transform the children list from syntactic to symbols).

For the enum Color, the children list are symbols to the val defs. During checking, the exhaustivity checker just decompose Color into points (as it's done for Java enum), then it can produce friendly messages.

[DarkDimius]

We only need to pass the list of children from parser

I'd prefer if it was done in a separate Mini-Phase, that would be in block with TailRec. This will ensure it finishes before patmat starts.

[liufengyun]

I'd prefer if it was done in a separate Mini-Phase

If I understand correctly, you mean to gather the children list in a mini-phase after typer?

I assume the enum-style ADTs will get desugared before type checking. If we don't keep the children list from the parser, we lose some information in the syntax and would need a lot of resugaring efforts to restore the children information which is available in the syntax.

[DarkDimius]

I assume the enum-style ADTs will get desugared before type checking

Subclassing relationship will definitely not be lost, and that's the only thing, I think, you need here.

[liufengyun]

For the enum Color, there is only one anonymous class, but many instances: Red, Blue, White. Here the anonymous class doesn't matter for the check, what is important are the instances. Collecting information about the instances after typer requires some effort, but they are available in syntax already -- if we can keep the children list in syntax, it seems to me the implementation will be easier and clean.

https://github.com/lampepfl/dotty/pull/1918/files#diff-07e5eb314978ffa9168b31a3c887de9dR15

[liufengyun]

Fixed in #2197.

[SethTisue]

the opposite of a false unreachable warning is the failure to emit an non-exhaustive-match warning. the discussants above may know this already, but for the record, note that Scala 2.12 correctly issues a warning in this case:

scala 2.12.7> object O { sealed trait T; case object T1 extends T; def g(): Unit = new T { } ; def f(t: T): Unit = t match { case T1 => () } }
<console>:11: warning: match may not be exhaustive.
It would fail on the following input: $anon()

but Dotty still does not. should I open a new ticket on it?

[smarter]

@SethTisue Good timing: #5396 :)