Done

Author: SolalPirelli

compiler/src/dotty/tools/backend/jvm/opt/OptimizerUtils.scala

@@ -107,10 +107,15 @@ object OptimizerUtils:
       }
     }
 
+    private def isSideEffectFreeListCall(mi: MethodInsnNode): Boolean =
+      (mi.owner == "scala/collection/immutable/$colon$colon") &&
+        (mi.name == "next" || mi.name == "head")
+
     def isSideEffectFreeCall(mi: MethodInsnNode): Boolean = {
       isScalaBox(mi) || // not Scala unbox, it may CCE
         isJavaBox(mi) || // not Java unbox, it may NPE
         isSideEffectFreeConstructorCall(mi) ||
+        isSideEffectFreeListCall(mi) ||
         AnalysisUtils.isClassTagApply(mi) ||
         isStdLibModuleAlias(mi)
     }

compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala

@@ -246,7 +246,7 @@ object PatternMatcher {
     }
 
     /** Plan for matching `scrutinee` symbol against `tree` pattern */
-    private def patternPlan(scrutinee: Symbol, tree: Tree, onSuccess: Plan, isDefinitelyCatchAll: Boolean): Plan = {
+    private def patternPlan(scrutinee: Symbol, tree: Tree, onSuccess: Plan, isDefinitelyCatchAll: Boolean, selectorType: Type): Plan = {
 
       extension (tree: Tree) def avoidPatBoundType(): Type =
         tree.tpe.widen match
@@ -289,7 +289,7 @@ object PatternMatcher {
                 ResultPlan(tpd.Throw(tpd.nullLiteral.cast(defn.ThrowableType)))
               else
                 val sym :: syms1 = syms: @unchecked
-                patternPlan(sym, arg, matchArgsPatternPlan(args1, syms1), isDefinitelyCatchAll)
+                patternPlan(sym, arg, matchArgsPatternPlan(args1, syms1), isDefinitelyCatchAll, selectorType)
             case Nil =>
               assert(syms.isEmpty)
               onSuccess
@@ -343,7 +343,7 @@ object PatternMatcher {
             val matchRemaining =
               letAbstract(remaining): remainingResult =>
                 if trailing.isEmpty then
-                  patternPlan(remainingResult, arg, onSuccess, isDefinitelyCatchAll)
+                  patternPlan(remainingResult, arg, onSuccess, isDefinitelyCatchAll, selectorType)
                 else
                   val seq = ref(remainingResult)
                     .select(defn.Seq_dropRight.matchingMember(remainingResult.info))
@@ -355,7 +355,7 @@ object PatternMatcher {
                     val matchTrailing =
                       letAbstract(rest): trailingResult =>
                         matchElemsPlan(trailingResult, trailing, noLengthTest, onSuccess)
-                    patternPlan(seqResult, arg, matchTrailing, isDefinitelyCatchAll)
+                    patternPlan(seqResult, arg, matchTrailing, isDefinitelyCatchAll, selectorType)
             matchElemsPlan(getResult, leading,
               LengthTest(leading.length + trailing.length, exact = false),
               matchRemaining)
@@ -485,10 +485,17 @@ object PatternMatcher {
             else tpt.tpe
           val body = letAbstract(ref(scrutinee).cast(castTp)) { casted =>
             nonNull += casted
-            patternPlan(casted, pat, onSuccess, isDefinitelyCatchAll)
+            patternPlan(casted, pat, onSuccess, isDefinitelyCatchAll, selectorType)
           }
-          if isDefinitelyCatchAll then body
-          else TestPlan(TypeTest(tpt, isTrusted(pat)), scrutinee, tree.span, body)
+          val patIsTrusted = isTrusted(pat)
+          if isDefinitelyCatchAll then
+            // Because we're not inserting a type test, we need to explicitly check for unchecked type conversions
+            // e.g., `(x: List[Int] | List[String]) match { case l: List[Int] => ??? }` should warn,
+            // even if it is a catch-all after erasure.
+            TypeTestsCasts.checkTypePattern(selectorType, castTp, tree.srcPos, patIsTrusted)
+            body
+          else
+            TestPlan(TypeTest(tpt, patIsTrusted), scrutinee, tree.span, body)
         case UnApply(extractor, implicits, args) =>
           val unappPlan = if (scrutinee.info.isBottomType)
             // Generate a throwaway but type-correct plan.
@@ -513,19 +520,19 @@ object PatternMatcher {
           if (scrutinee.info.isNotNull || nonNull(scrutinee)) unappPlan
           else TestPlan(NonNullTest, scrutinee, tree.span, unappPlan)
         case Bind(name, body) =>
-          if (name == nme.WILDCARD) patternPlan(scrutinee, body, onSuccess, isDefinitelyCatchAll)
+          if (name == nme.WILDCARD) patternPlan(scrutinee, body, onSuccess, isDefinitelyCatchAll, selectorType)
           else {
             // The type of `name` may refer to val in `body`, therefore should come after `body`
             val bound = tree.symbol.asTerm
             initializer(bound) = ref(scrutinee)
-            patternPlan(scrutinee, body, LetPlan(bound, onSuccess), isDefinitelyCatchAll)
+            patternPlan(scrutinee, body, LetPlan(bound, onSuccess), isDefinitelyCatchAll, selectorType)
           }
         case Alternative(alts) =>
           altsLabeledAbstract { onf =>
             SeqPlan(
               altsLabeledAbstract { ons =>
                 alts.foldRight(onf) { (alt, next) =>
-                  SeqPlan(patternPlan(scrutinee, alt, ons, isDefinitelyCatchAll), next)
+                  SeqPlan(patternPlan(scrutinee, alt, ons, isDefinitelyCatchAll, selectorType), next)
                 }
               },
               onSuccess
@@ -542,34 +549,35 @@ object PatternMatcher {
         case SeqLiteral(pats, _) =>
           matchElemsPlan(scrutinee, pats, LengthTest(pats.length, exact = true), onSuccess)
         case _ =>
-          TestPlan(EqualTest(tree), scrutinee, tree.span, onSuccess)
+          if isDefinitelyCatchAll then onSuccess
+          else TestPlan(EqualTest(tree), scrutinee, tree.span, onSuccess)
       }
     }
 
-    private def caseDefPlan(scrutinee: Symbol, cdef: CaseDef, isDefinitelyCatchAll: Boolean): Plan =
+    private def caseDefPlan(scrutinee: Symbol, cdef: CaseDef, isDefinitelyCatchAll: Boolean, selectorType: Type): Plan =
       val CaseDef(pat, guard, body) = cdef
       val caseDefBodyPlan: Plan = body match
         case t: SubMatch => subMatchPlan(t)
         case _ => ResultPlan(body)
       val onSuccess: Plan =
         if guard.isEmpty then caseDefBodyPlan
         else TestPlan(GuardTest, guard, guard.span, caseDefBodyPlan)
-      patternPlan(scrutinee, pat, onSuccess, isDefinitelyCatchAll)
+      patternPlan(scrutinee, pat, onSuccess, isDefinitelyCatchAll, selectorType)
     end caseDefPlan
 
     // like matchPlan but without a final matchError ResultPlan at the end of SeqPlans
     // s.t. we fall back to the outer SeqPlan
     private def subMatchPlan(tree: SubMatch): Plan =
       letAbstract(tree.selector) { scrutinee =>
-        tree.cases.map(caseDefPlan(scrutinee, _, false)).reduceRight(SeqPlan(_, _))
+        tree.cases.map(caseDefPlan(scrutinee, _, false, tree.selector.tpe)).reduceRight(SeqPlan(_, _))
       }
 
     private def matchPlan(tree: Match, isExhaustive: Boolean): Plan =
       letAbstract(tree.selector) { scrutinee =>
         val matchError: Plan = ResultPlan(Throw(New(defn.MatchErrorClass.typeRef, ref(scrutinee) :: Nil)))
         var isCatchAll = isExhaustive
         tree.cases.foldRight(matchError) { (cdef, next) =>
-          val casePlan = SeqPlan(caseDefPlan(scrutinee, cdef, isCatchAll), next)
+          val casePlan = SeqPlan(caseDefPlan(scrutinee, cdef, isCatchAll, tree.selector.tpe), next)
           isCatchAll = false
           casePlan
         }

compiler/src/dotty/tools/dotc/transform/TypeTestsCasts.scala

@@ -387,7 +387,7 @@ object TypeTestsCasts {
   def checkBind(tree: Bind)(using Context) =
     checkTypePattern(defn.ThrowableType, tree.body.tpe, tree.srcPos)
 
-  private def checkTypePattern(exprTpe: Type, castTpe: Type, pos: SrcPos, trustTypeApplication: Boolean = false)(using Context) =
+  def checkTypePattern(exprTpe: Type, castTpe: Type, pos: SrcPos, trustTypeApplication: Boolean = false)(using Context) =
     val isUnchecked = exprTpe.widenTermRefExpr.hasAnnotation(defn.UncheckedAnnot)
     if !isUnchecked then
       val whyNot = whyUncheckable(exprTpe, castTpe, pos.span, trustTypeApplication)

compiler/test/dotty/tools/backend/jvm/LocalOptimizationBytecodeTests.scala

@@ -17,6 +17,26 @@ class LocalOptimizationBytecodeTests extends OptimizationBytecodeTest {
       returnType = "Boolean"
     )
 
+  // Ensure writing `case None` is as efficient as `case _` after `case Some`
+  // This matters not only to avoid a null check in itself, but because the function may then be small enough to be inlineable
+  @Test def patternMatchingOption =
+    assertEquivalence(
+      "if x.isInstanceOf[Some[Int]] then x.asInstanceOf[Some[Int]].value else 0",
+      "x match { case Some(v) => v; case None => 0 }",
+      params = List("x: Option[Int]"),
+      returnType = "Int"
+    )
+
+  // Same but for List: `case Nil` should be as efficient as `case _` after `case _ :: _`
+  @Test def patternMatchingList =
+    assertEquivalence(
+      "if x.isInstanceOf[`::`[Int]] then x.asInstanceOf[`::`[Int]].head else 0",
+      "x match { case v :: _ => v; case Nil => 0 }",
+      params = List("x: List[Int]"),
+      returnType = "Int"
+    )
+
+  // Similar as the above, inspired by a compiler function
   @Test def patternMatchingLazyOfT =
     assertEquivalence(
       "if x.isInstanceOf[Lazy[?]] then x.asInstanceOf[Lazy[T]].value else x.asInstanceOf[T]",

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

@@ -15,7 +15,7 @@ import org.junit.Ignore
     // can add, e.g., .and("-some-option")
     val options = defaultOptions
     // can also use `compileDir` (single test as a dir), `compileFilesInDir` (all tests within a dir)
-    val test = compileFile("tests/warn/i5826.min.scala", options)
+    val test = compileFile("tests/pos/tuple-filter.scala", options)
     // or `RunTestWithCoverage` for "run" tests with output, or `WarnTestWithCoverage` for "warn" tests with warnings
     type TestKind = PosTestWithCoverage
     val compilationTest = withCoverage(aggregateTests(test))

tests/patmat/t11620.check

@@ -1,3 +1,9 @@
+-- [E092] Pattern Match Unchecked Warning: tests/patmat/t11620.scala:33:13 -----
+33 |  case B.Aux(a: A2[T]) => a.t1  // 👎 (false-positive): unreachable code
+   |             ^^^^^^^^
+   |the type test for A2[T] cannot be checked at runtime because its type arguments can't be determined from B.Aux[_A, T]
+   |
+   | longer explanation available when compiling with `-explain`
 -- [E029] Pattern Match Exhaustivity Warning: tests/patmat/t11620.scala:36:23 --
 36 |def foo4[T](b: B[T]) = b match {
    |                       ^

tests/pos-deep-subtype/i16899.scala tests/warn/i16899.scalatests/pos-deep-subtype/i16899.scala renamed to tests/warn/i16899.scala

@@ -1,7 +1,5 @@
-//> using options -Werror
-
 sealed trait Unset
 
 def foo(v: Unset|Option[Int]): Unit = v match
   case v: Unset => ()
-  case v: Option[Int] => ()
+  case v: Option[Int] => () // ok

tests/warn/i25100.scala

@@ -13,8 +13,8 @@ def main(): Unit = {
   val i: Foo | Bar.type = Foo("Foo")
 
   i match {
+    case Bar => println("i is a Bar")
     case i: Foo => println("i is a Foo")
-    case Bar => println("i is a Long")
   }
 }
 

tests/warn/i5826.check

@@ -1,15 +1,15 @@
+-- [E092] Pattern Match Unchecked Warning: tests/warn/i5826.scala:8:9 --------------------------------------------------
+8 |    case ls: List[Int] => ls.head       // warn: unchecked
+  |         ^^^^^^^^^^^^^
+  |the type test for List[Int] cannot be checked at runtime because its type arguments can't be determined from List[String]
+  |
+  | longer explanation available when compiling with `-explain`
 -- [E092] Pattern Match Unchecked Warning: tests/warn/i5826.scala:3:9 --------------------------------------------------
 3 |    case ls: List[Int] => ls.head       // warn: unchecked
   |         ^
   |    the type test for List[Int] cannot be checked at runtime because its type arguments can't be determined from A
   |
   | longer explanation available when compiling with `-explain`
--- [E092] Pattern Match Unchecked Warning: tests/warn/i5826.scala:8:9 --------------------------------------------------
-8 |    case ls: List[Int] => ls.head       // warn: unchecked
-  |         ^
-  |the type test for List[Int] cannot be checked at runtime because its type arguments can't be determined from List[String]
-  |
-  | longer explanation available when compiling with `-explain`
 -- [E092] Pattern Match Unchecked Warning: tests/warn/i5826.scala:16:9 -------------------------------------------------
 16 |    case ls: A[X] => 4                 // warn
    |         ^