Refine return value of unapply/unapplySeq to Some[T] to avoid pattern match warnings

[mschwerhoff]

After updating to Scala 2.13 and Kiama 2.4.0 I got a lot of "match may not be exhaustive" warnings in my code. According to this Scala issue discussion, these are due to unapply (and I assume unapplySeq) extractor methods with return type Option[T] instead of Some[T], which they actually return.

A quick look at Kiama's source reveals extractors such as this one, which have the same mismatch and thus give rise to compiler warnings when used.

It would be great if the return type could be refined, wherever possible, to avoid such spurious pattern match exhaustiveness warnings.

[inkytonik]

Thanks for the report, Malte.

I agree that the extractor you point to should now give a warning, but it doesn't for me at the moment.

Can you clarify what steps you carry out to see these warnings? I fixed all of the warnings that were reported by 2.13.4 before I released Kiama 2.4.0 (and just double-checked with the current state of the repo). E.g., are you compiling your code with options that I don't use in the Kiama build?

[mschwerhoff]

Regarding options: the project in question uses the following options:

scalaVersion := "2.13.4"
scalacOptions := Seq(
  "-encoding", "UTF-8", 
  "-deprecation",
  "-unchecked",
  "-feature",
  "-Wunused",
  "-Ypatmat-exhaust-depth", "40")

The project is unfortunately fairly large and currently hosted in a private repository. For what it's worth, I'll try to provide as much information as possible/reasonable, hopefully that suffices to shed light on the problem. Sorry for the long read to come.

Here is the snippet — with superfluous bits such as obj and sth for debugging purposes — for which I get a compiler warning:

// File: SemanticAnalyser.scala

class SemanticAnalyser(tree: VoilaTree) extends Attribution {
  ...
  lazy val definedEntity: PIdnDef => VoilaEntity =
    attr[PIdnDef, VoilaEntity] ((obj: PIdnDef) => obj match {
      case sth @ tree.parent(p) =>
        p match {
          ...
          case _ => UnknownEntity()
        }
//      case PIdnDef(_) => ???
    })
   ...
}

sth is typed (by IntelliJ) as PIdnDef, as expected. The compiler warning I get is

... match may not be exhaustive.
It would fail on the following input: PIdnDef(_)
    attr[PIdnDef, VoilaEntity] ((obj: PIdnDef) => obj match {
                                                  ^

I've locally modified Kiama's class relation.Relation[T, U] such that its unapplySeq method has return type Some[Vector[U]]. Unfortunately, this did not resolve the warning.

Uncommenting the additional case PIdnDef(_) => ??? gets rid of the warning (as does case _: PIdnDef). This suggests to me that the compiler is not able to figure out that case sth @ tree.parent(p) will match any PIdnDef — but I can't work out, why.

Any help is welcome.

[inkytonik]

Ah, yes. 2.13.4 doesn't seem to deal with exhaustiveness checking for irrefutable extractors properly, even though the release notes imply that it does.

3.0.0-M2 seems to do better, so I don't think the extra cases are needed with that version (at least for the handful of examples I've tried).

Since Kiama builds on 2.11 onwards, for now I have added extra cases to avoid the warnings. E.g., from the Oberon0 example:

    def entityFromDecl(n : IdnDef, i : String) : Oberon0Entity =
        n match {
            case tree.parent(p) =>
                p match {
                    case p : ModuleDecl => Module(i, p)
                    ...
                    case _ =>
                        sys.error(s"entityFromDecl: unexpected IdnDef parent $p")
                }
            case _ =>
                sys.error(s"entityFromDecl: unexpected IdnDef $n")
        }

In the next version I will tighten the return types of Kiama's irrefutable extractors, just in case this makes a difference sometime down the line.

[inkytonik]

Actually, "better" above in reference to 3.0.0-M2 is probably not right. I have no way of knowing what exhaustivity checking it is doing, so the absence of warnings might just mean it's not looking, as was the case before 2.13.4.

[mschwerhoff]

Thank you for the detailed explanation, Tony! I've already added "dead" cases just to suppress the warnings, and I'll wait for Scala 3 to magically resolve the problem :-)

I've also found myself wondering over the years how sound and precise (and complete) Scala's pattern match warnings are. Would be nice if their quality strictly increased over time, but unfortunately, that doesn't seem to be the case.

[inkytonik]

Ok, cool. Sounds like a good plan.

[inkytonik]

Closed by e56ca93f9a26949ee9c9f9c48afdedf8498fee7e