Fix #2578 Part 1: Tighten type checking of pattern bindings

compiler/src/dotty/tools/dotc/ast/Desugar.scala

@@ -32,6 +32,9 @@ object desugar {
    */
   val DerivingCompanion: Property.Key[SourcePosition] = new Property.Key
 
+  /** An attachment for match expressions generated from a PatDef */
+  val PatDefMatch: Property.Key[Unit] = new Property.Key
+
   /** Info of a variable in a pattern: The named tree and its type */
   private type VarInfo = (NameTree, Tree)
 
@@ -956,7 +959,11 @@ object desugar {
       // - `pat` is a tuple of N variables or wildcard patterns like `(x1, x2, ..., xN)`
       val tupleOptimizable = forallResults(rhs, isMatchingTuple)
 
-      def rhsUnchecked = makeAnnotated("scala.unchecked", rhs)
+      def rhsUnchecked = {
+        val rhs1 = makeAnnotated("scala.unchecked", rhs)
+        rhs1.pushAttachment(PatDefMatch, ())
+        rhs1
+      }
       val vars =
         if (tupleOptimizable) // include `_`
           pat match {

compiler/src/dotty/tools/dotc/core/Types.scala

@@ -1528,13 +1528,17 @@ object Types {
      */
     def signature(implicit ctx: Context): Signature = Signature.NotAMethod
 
-    def dropRepeatedAnnot(implicit ctx: Context): Type = this match {
-      case AnnotatedType(parent, annot) if annot.symbol eq defn.RepeatedAnnot => parent
-      case tp @ AnnotatedType(parent, annot) =>
-        tp.derivedAnnotatedType(parent.dropRepeatedAnnot, annot)
-      case tp => tp
+    /** Drop annotation of given `cls` from this type */
+    def dropAnnot(cls: Symbol)(implicit ctx: Context): Type = stripTypeVar match {
+      case self @ AnnotatedType(pre, annot) =>
+        if (annot.symbol eq cls) pre
+        else self.derivedAnnotatedType(pre.dropAnnot(cls), annot)
+      case _ =>
+        this
     }
 
+    def dropRepeatedAnnot(implicit ctx: Context): Type = dropAnnot(defn.RepeatedAnnot)
+
     def annotatedToRepeated(implicit ctx: Context): Type = this match {
       case tp @ ExprType(tp1) => tp.derivedExprType(tp1.annotatedToRepeated)
       case AnnotatedType(tp, annot) if annot matches defn.RepeatedAnnot =>

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -1200,14 +1200,15 @@ object Parsers {
      *                      |  ForExpr
      *                      |  [SimpleExpr `.'] id `=' Expr
      *                      |  SimpleExpr1 ArgumentExprs `=' Expr
-     *                      |  PostfixExpr [Ascription]
-     *                      |  [‘inline’] PostfixExpr `match' `{' CaseClauses `}'
+     *                      |  Expr2
+     *                      |  [‘inline’] Expr2 `match' `{' CaseClauses `}'
      *                      |  `implicit' `match' `{' ImplicitCaseClauses `}'
-     *  Bindings          ::= `(' [Binding {`,' Binding}] `)'
-     *  Binding           ::= (id | `_') [`:' Type]
-     *  Ascription        ::= `:' CompoundType
-     *                      | `:' Annotation {Annotation}
-     *                      | `:' `_' `*'
+     *  Bindings          ::=  `(' [Binding {`,' Binding}] `)'
+     *  Binding           ::=  (id | `_') [`:' Type]
+     *  Expr2             ::=  PostfixExpr [Ascription]
+     *  Ascription        ::=  `:' InfixType
+     *                      |  `:' Annotation {Annotation}
+     *                      |  `:' `_' `*'
      */
     val exprInParens: () => Tree = () => expr(Location.InParens)
 
@@ -1324,15 +1325,16 @@ object Parsers {
              t
          }
       case COLON =>
-        ascription(t, location)
+        in.nextToken()
+        val t1 = ascription(t, location)
+        if (in.token == MATCH) expr1Rest(t1, location) else t1
       case MATCH =>
         matchExpr(t, startOffset(t), Match)
       case _ =>
         t
     }
 
     def ascription(t: Tree, location: Location.Value): Tree = atSpan(startOffset(t)) {
-      in.skipToken()
       in.token match {
         case USCORE =>
           val uscoreStart = in.skipToken()
@@ -1801,7 +1803,10 @@ object Parsers {
      */
     def pattern1(): Tree = {
       val p = pattern2()
-      if (isVarPattern(p) && in.token == COLON) ascription(p, Location.InPattern)
+      if (isVarPattern(p) && in.token == COLON) {
+        in.nextToken()
+        ascription(p, Location.InPattern)
+      }
       else p
     }
 
@@ -2353,14 +2358,32 @@ object Parsers {
         tmplDef(start, mods)
     }
 
-    /** PatDef ::= Pattern2 {`,' Pattern2} [`:' Type] `=' Expr
-     *  VarDef ::= PatDef | id {`,' id} `:' Type `=' `_'
-     *  ValDcl ::= id {`,' id} `:' Type
-     *  VarDcl ::= id {`,' id} `:' Type
+    /** PatDef  ::=  ids [‘:’ Type] ‘=’ Expr
+     *            |  Pattern2 [‘:’ Type | Ascription] ‘=’ Expr
+     *  VarDef  ::=  PatDef | id {`,' id} `:' Type `=' `_'
+     *  ValDcl  ::=  id {`,' id} `:' Type
+     *  VarDcl  ::=  id {`,' id} `:' Type
      */
     def patDefOrDcl(start: Offset, mods: Modifiers): Tree = atSpan(start, nameStart) {
-      val lhs = commaSeparated(pattern2)
-      val tpt = typedOpt()
+      val first = pattern2()
+      var lhs = first match {
+        case id: Ident if in.token == COMMA =>
+          in.nextToken()
+          id :: commaSeparated(() => termIdent())
+        case _ =>
+          first :: Nil
+      }
+      def emptyType = TypeTree().withSpan(Span(in.lastOffset))
+      val tpt =
+        if (in.token == COLON) {
+          in.nextToken()
+          if (in.token == AT && lhs.tail.isEmpty) {
+            lhs = ascription(first, Location.ElseWhere) :: Nil
+            emptyType
+          }
+          else toplevelTyp()
+        }
+        else emptyType
       val rhs =
         if (tpt.isEmpty || in.token == EQUALS) {
           accept(EQUALS)
@@ -2374,9 +2397,9 @@ object Parsers {
       lhs match {
         case (id: BackquotedIdent) :: Nil if id.name.isTermName =>
           finalizeDef(BackquotedValDef(id.name.asTermName, tpt, rhs), mods, start)
-        case Ident(name: TermName) :: Nil => {
+        case Ident(name: TermName) :: Nil =>
           finalizeDef(ValDef(name, tpt, rhs), mods, start)
-        } case _ =>
+        case _ =>
           PatDef(mods, lhs, tpt, rhs)
       }
     }

compiler/src/dotty/tools/dotc/typer/Applications.scala

@@ -1105,7 +1105,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
           if (selType <:< unapplyArgType) {
             unapp.println(i"case 1 $unapplyArgType ${ctx.typerState.constraint}")
             fullyDefinedType(unapplyArgType, "pattern selector", tree.span)
-            selType
+            selType.dropAnnot(defn.UncheckedAnnot) // need to drop @unchecked. Just because the selector is @unchecked, the pattern isn't.
           } else if (isSubTypeOfParent(unapplyArgType, selType)(ctx.addMode(Mode.GADTflexible))) {
             val patternBound = maximizeType(unapplyArgType, tree.span, fromScala2x)
             if (patternBound.nonEmpty) unapplyFn = addBinders(unapplyFn, patternBound)

compiler/src/dotty/tools/dotc/typer/Checking.scala

@@ -16,13 +16,16 @@ import ProtoTypes._
 import Scopes._
 import CheckRealizable._
 import ErrorReporting.errorTree
+import rewrites.Rewrites.patch
+import util.Spans.Span
 
 import util.SourcePosition
 import transform.SymUtils._
 import Decorators._
 import ErrorReporting.{err, errorType}
-import config.Printers.typr
+import config.Printers.{typr, patmatch}
 import NameKinds.DefaultGetterName
+import Applications.unapplyArgs
 
 import collection.mutable
 import SymDenotations.{NoCompleter, NoDenotation}
@@ -594,6 +597,48 @@ trait Checking {
       ctx.error(ex"$cls cannot be instantiated since it${rstatus.msg}", pos)
   }
 
+  /** Check that pattern `pat` is irrefutable for scrutinee tye `pt`.
+   *  This means `pat` is either marked @unchecked or `pt` conforms to the
+   *  pattern's type. If pattern is an UnApply, do the check recursively.
+   */
+  def checkIrrefutable(pat: Tree, pt: Type)(implicit ctx: Context): Boolean = {
+    patmatch.println(i"check irrefutable $pat: ${pat.tpe} against $pt")
+
+    def check(pat: Tree, pt: Type): Boolean = {
+      if (pt <:< pat.tpe)
+        true
+      else {
+        ctx.errorOrMigrationWarning(
+          ex"""pattern's type ${pat.tpe} is more specialized than the right hand side expression's type ${pt.dropAnnot(defn.UncheckedAnnot)}
+              |
+              |If the narrowing is intentional, this can be communicated by writing `: @unchecked` after the full pattern.${err.rewriteNotice}""",
+          pat.sourcePos)
+        false
+      }
+    }
+
+    !ctx.settings.strict.value || // only in -strict mode for now since mitigations work only after this PR
+    pat.tpe.widen.hasAnnotation(defn.UncheckedAnnot) || {
+      pat match {
+        case Bind(_, pat1) =>
+          checkIrrefutable(pat1, pt)
+        case UnApply(fn, _, pats) =>
+          check(pat, pt) && {
+            val argPts = unapplyArgs(fn.tpe.finalResultType, fn, pats, pat.sourcePos)
+            pats.corresponds(argPts)(checkIrrefutable)
+          }
+        case Alternative(pats) =>
+          pats.forall(checkIrrefutable(_, pt))
+        case Typed(arg, tpt) =>
+          check(pat, pt) && checkIrrefutable(arg, pt)
+        case Ident(nme.WILDCARD) =>
+          true
+        case _ =>
+          check(pat, pt)
+      }
+    }
+  }
+
   /** Check that `path` is a legal prefix for an import or export clause */
   def checkLegalImportPath(path: Tree)(implicit ctx: Context): Unit = {
     checkStable(path.tpe, path.sourcePos)

compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala

@@ -157,6 +157,10 @@ object ErrorReporting {
       }
       """\$\{\w*\}""".r.replaceSomeIn(raw, m => translate(m.matched.drop(2).init))
     }
+
+    def rewriteNotice: String =
+      if (ctx.scala2Mode) "\nThis patch can be inserted automatically under -rewrite."
+      else ""
   }
 
   def err(implicit ctx: Context): Errors = new Errors

compiler/src/dotty/tools/dotc/typer/Typer.scala

@@ -1036,7 +1036,15 @@ class Typer extends Namer
         if (tree.isInline) checkInInlineContext("inline match", tree.posd)
         val sel1 = typedExpr(tree.selector)
         val selType = fullyDefinedType(sel1.tpe, "pattern selector", tree.span).widen
-        typedMatchFinish(tree, sel1, selType, tree.cases, pt)
+        val result = typedMatchFinish(tree, sel1, selType, tree.cases, pt)
+        result match {
+          case Match(sel, CaseDef(pat, _, _) :: _)
+          if (tree.selector.removeAttachment(desugar.PatDefMatch).isDefined) =>
+            if (!checkIrrefutable(pat, sel.tpe) && ctx.scala2Mode)
+              patch(Span(pat.span.end), ": @unchecked")
+          case _ =>
+        }
+        result
     }
   }
 
@@ -1817,8 +1825,11 @@ class Typer extends Namer
           }
         case _ => arg1
       }
-      val tpt = TypeTree(AnnotatedType(arg1.tpe.widenIfUnstable, Annotation(annot1)))
-      assignType(cpy.Typed(tree)(arg2, tpt), tpt)
+      val argType =
+        if (arg1.isInstanceOf[Bind]) arg1.tpe.widen // bound symbol is not accessible outside of Bind node
+        else arg1.tpe.widenIfUnstable
+      val annotatedTpt = TypeTree(AnnotatedType(argType, Annotation(annot1)))
+      assignType(cpy.Typed(tree)(arg2, annotatedTpt), annotatedTpt)
     }
   }
 

compiler/test-resources/repl/patdef

@@ -21,6 +21,3 @@ scala> val _ @ List(x) = List(1)
 val x: Int = 1
 scala> val List(_ @ List(x)) = List(List(2))
 val x: Int = 2
-scala> val B @ List(), C: List[Int] = List()
-val B: List[Int] = List()
-val C: List[Int] = List()

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -150,6 +150,7 @@ class CompilationTests extends ParallelTesting {
     aggregateTests(
       compileFilesInDir("tests/neg", defaultOptions),
       compileFilesInDir("tests/neg-tailcall", defaultOptions),
+      compileFilesInDir("tests/neg-strict", defaultOptions.and("-strict")),
       compileFilesInDir("tests/neg-no-kind-polymorphism", defaultOptions and "-Yno-kind-polymorphism"),
       compileFilesInDir("tests/neg-custom-args/deprecation", defaultOptions.and("-Xfatal-warnings", "-deprecation")),
       compileFilesInDir("tests/neg-custom-args/fatal-warnings", defaultOptions.and("-Xfatal-warnings")),
@@ -160,8 +161,6 @@ class CompilationTests extends ParallelTesting {
       compileFile("tests/neg-custom-args/i3246.scala", scala2Mode),
       compileFile("tests/neg-custom-args/overrideClass.scala", scala2Mode),
       compileFile("tests/neg-custom-args/autoTuplingTest.scala", defaultOptions.and("-language:noAutoTupling")),
-      compileFile("tests/neg-custom-args/i1050.scala", defaultOptions.and("-strict")),
-      compileFile("tests/neg-custom-args/nullless.scala", defaultOptions.and("-strict")),
       compileFile("tests/neg-custom-args/nopredef.scala", defaultOptions.and("-Yno-predef")),
       compileFile("tests/neg-custom-args/noimports.scala", defaultOptions.and("-Yno-imports")),
       compileFile("tests/neg-custom-args/noimports2.scala", defaultOptions.and("-Yno-imports")),
@@ -249,7 +248,9 @@ class CompilationTests extends ParallelTesting {
 
     val lib =
       compileList("src", librarySources,
-        defaultOptions.and("-Ycheck-reentrant", "-strict", "-priorityclasspath", defaultOutputDir))(libGroup)
+        defaultOptions.and("-Ycheck-reentrant",
+          //  "-strict",  // TODO: re-enable once we allow : @unchecked in pattern definitions. Right now, lots of narrowing pattern definitions fail.
+          "-priorityclasspath", defaultOutputDir))(libGroup)
 
     val compilerSources = sources(Paths.get("compiler/src"))
     val compilerManagedSources = sources(Properties.dottyCompilerManagedSources)

docs/docs/internals/syntax.md

@@ -196,9 +196,10 @@ Expr1             ::=  ‘if’ ‘(’ Expr ‘)’ {nl}
                     |  ForExpr
                     |  [SimpleExpr ‘.’] id ‘=’ Expr                             Assign(expr, expr)
                     |  SimpleExpr1 ArgumentExprs ‘=’ Expr                       Assign(expr, expr)
-                    |  PostfixExpr [Ascription]
-                    |  [‘inline’] PostfixExpr ‘match’ ‘{’ CaseClauses ‘}’       Match(expr, cases) -- point on match
+                    |  Expr2
+                    |  [‘inline’] Expr2 ‘match’ ‘{’ CaseClauses ‘}’             Match(expr, cases) -- point on match
                     |  ‘implicit’ ‘match’ ‘{’ ImplicitCaseClauses ‘}’
+Expr2             ::=  PostfixExpr [Ascription]
 Ascription        ::=  ‘:’ InfixType                                            Typed(expr, tp)
                     |  ‘:’ Annotation {Annotation}                              Typed(expr, Annotated(EmptyTree, annot)*)
 Catches           ::=  ‘catch’ Expr
@@ -224,7 +225,7 @@ SimpleExpr1       ::=  Literal
 Quoted            ::=  ‘'’ ‘{’ Block ‘}’
                     |  ‘'’ ‘[’ Type ‘]’
 ExprsInParens     ::=  ExprInParens {‘,’ ExprInParens}
-ExprInParens      ::=  PostfixExpr ‘:’ Type
+ExprInParens      ::=  PostfixExpr ‘:’ Type                                     -- normal Expr allows only RefinedType here
                     |  Expr
 ParArgumentExprs  ::=  ‘(’ ExprsInParens ‘)’                                    exprs
                     |  ‘(’ [ExprsInParens ‘,’] PostfixExpr ‘:’ ‘_’ ‘*’ ‘)’          exprs :+ Typed(expr, Ident(wildcardStar))
@@ -358,7 +359,8 @@ Def               ::=  ‘val’ PatDef
                     |  ‘type’ {nl} TypeDcl
                     |  TmplDef
                     |  INT
-PatDef            ::=  Pattern2 {‘,’ Pattern2} [‘:’ Type] ‘=’ Expr              PatDef(_, pats, tpe?, expr)
+PatDef            ::=  ids [‘:’ Type] ‘=’ Expr
+                    |  Pattern2 [‘:’ Type | Ascription] ‘=’ Expr                PatDef(_, pats, tpe?, expr)
 VarDef            ::=  PatDef
                     |  ids ‘:’ Type ‘=’ ‘_’
 DefDef            ::=  DefSig [(‘:’ | ‘<:’) Type] ‘=’ Expr                      DefDef(_, name, tparams, vparamss, tpe, expr)

tests/neg-strict/unchecked-patterns.scala

@@ -0,0 +1,10 @@
+object Test {
+  val (y1: Some[Int] @unchecked) = Some(1): Option[Int]  // OK
+  val y2: Some[Int] @unchecked = Some(1): Option[Int]    // error
+
+  val x :: xs = List(1, 2, 3)        // error
+  val (1, c) = (1, 2)                // error
+  val 1 *: cs = 1 *: ()              // error
+
+  val (_: Int | _: Any) = ??? : Any  // error
+}

tests/neg/multi-patterns.scala

@@ -0,0 +1,4 @@
+object Test {
+  val (a :: as), bs = List(1, 2, 3) // error
+  val B @ List(), C: List[Int] = List() // error
+}

tests/pos-special/fatal-warnings/unchecked-scrutinee.scala

@@ -0,0 +1,5 @@
+object Test {
+  List(1: @unchecked, 2, 3): @unchecked match {
+    case a :: as =>
+  }
+}

tests/pos/i3412.scala

@@ -1,5 +1,3 @@
 class Test {
   val A @ List() = List()
-  val B @ List(), C: List[Int] = List()
-  val D @ List(), E @ List() = List()
 }

tests/run/unchecked-patterns.scala

@@ -0,0 +1,9 @@
+object Test extends App {
+  val x: Int @unchecked = 2
+  val (y1: Some[Int] @unchecked) = Some(1): Option[Int]
+
+  val a :: as: @unchecked = List(1, 2, 3)
+  val lst @ b :: bs: @unchecked = List(1, 2, 3)
+  val (1, c): @unchecked = (1, 2)
+  val 1 *: cs: @unchecked = 1 *: ()        // error
+}