scala / scala3

The Scala 3 compiler, also known as Dotty.
https://dotty.epfl.ch
Apache License 2.0
5.83k stars 1.05k forks source link

Typing varargs in pattern position #4790

Open allanrenucci opened 6 years ago

allanrenucci commented 6 years ago
import scala.collection.immutable

class Test {
  def foo(as: immutable.Seq[Int]) = {
    val List(_, bs: _*) = as
    val cs: collection.Seq[Int] = bs
  }
}
> dotc -d out tests/allan/Test.scala                                                                                                                                      13:14
-- [E007] Type Mismatch Error: tests/allan/Test.scala:47:34 --------------------
47 |    val cs: collection.Seq[Int] = bs
   |                                  ^^
   |                                  found:    Seq[Any](bs)
   |                                  required: Seq[Int]
   |                                  
one error found

But it does compile with -language:Scala2...

allanrenucci commented 6 years ago

The reason it work with -language:Scala2: https://github.com/lampepfl/dotty/blob/2e5368896239673be3c4aeaf273fc998871ad725/compiler/src/dotty/tools/dotc/typer/Inferencing.scala#L210

Blaisorblade commented 6 years ago

The restriction is basically because we're afraid that some class might implement scala.Seq[Int] & List[Any] (https://github.com/lampepfl/dotty/pull/4013/files#diff-67f55293f2001c7ef140f0932e65023e):

  trait A[+X]
  class B[+X](val x: X) extends A[X]
  class C[+X](x: Any) extends B[Any](x) with A[X]

But it seems too conservative here for two reasons:

  1. even if scala.Seq[Int] & List[Any] were inhabited, we could still type bs as Seq[Int]; only typing it as List[Int] would be potentially dangerous.
  2. here List is sealed and its subclasses are final, and indeed we can handle matching against :: just fine:
class Test {
  def foo(as: immutable.Seq[Int]) = {
    val _ :: bs = as
    val cs: collection.Seq[Int] = bs
  }
}

Double-checking scenario 2 is safe is the harder part, but extending refinementIsInvariant to recognize scenario 2. seems relatively easy, one only needs a bit of care to handle hierarchies with multiple layers:

diff --git a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala
index 66adb9194..13786309d 100644
--- a/compiler/src/dotty/tools/dotc/typer/Inferencing.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Inferencing.scala
@@ -20,6 +20,7 @@ import config.Printers.{typr, constr}
 import annotation.tailrec
 import reporting._
 import collection.mutable
+import transform.SymUtils._
 import config.Config

 import scala.annotation.internal.sharable
@@ -191,8 +192,10 @@ object Inferencing {
    *  TODO: Update so that GADT symbols can be variant, and we special case final class types in patterns
    */
   def constrainPatternType(tp: Type, pt: Type)(implicit ctx: Context): Boolean = {
+    def refinementIsInvariantCls(cls: Symbol): Boolean =
+      cls.is(Final) || cls.is(Case) || cls.is(Sealed) && cls.children.forall(refinementIsInvariantCls) // checking .forall(childCls => childCls.is(Final) || childCls.is(Final)) is a tempting mistake, but I think we need recursion here to handle multiple levels.
     def refinementIsInvariant(tp: Type): Boolean = tp match {
-      case tp: ClassInfo => tp.cls.is(Final) || tp.cls.is(Case)
+      case tp: ClassInfo => refinementIsInvariantCls(tp.cls)
       case tp: TypeProxy => refinementIsInvariant(tp.underlying)
       case tp: AndType => refinementIsInvariant(tp.tp1) && refinementIsInvariant(tp.tp2)
       case tp: OrType => refinementIsInvariant(tp.tp1) && refinementIsInvariant(tp.tp2)

(Tested in this case). EDIT: of course, I should probably update checkCaseClassInheritanceInvariant: the above only checks that the hierarchy is closed, we also need to check that the hierarchy we see doesn't contain inheritance that prevents refinement.

odersky commented 5 years ago

I probably won't be able to work on this in the near future.