Skip to content
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

Typing varargs in pattern position #4790

Open
allanrenucci opened this issue Jul 13, 2018 · 3 comments · May be fixed by #16924
Open

Typing varargs in pattern position #4790

allanrenucci opened this issue Jul 13, 2018 · 3 comments · May be fixed by #16924

Comments

@allanrenucci
Copy link
Contributor

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
Copy link
Contributor Author

@Blaisorblade
Copy link
Contributor

Blaisorblade commented Aug 14, 2018

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.

Blaisorblade added a commit to dotty-staging/dotty that referenced this issue Aug 14, 2018
Must also check if those closed hierarchies prevent refinement.
Blaisorblade added a commit to dotty-staging/dotty that referenced this issue Aug 15, 2018
Must also check if those closed hierarchies prevent refinement.
@odersky odersky removed their assignment Feb 15, 2019
@odersky
Copy link
Contributor

odersky commented Feb 15, 2019

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

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.

5 participants