From 784497d86e83a3ed436d97695b0c56f4fd7e2965 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= <sjrdoeraene@gmail.com>
Date: Tue, 6 Feb 2024 10:27:44 +0100
Subject: [PATCH] Fix #19607: Allow to instantiate *wildcard* type captures to
 TypeBounds.

When matching in a match type, if we encounter a `TypeBounds`
scrutinee and we have a wildcard capture on the right, we used to
pick the `hi` bound "because anything between between `lo` and
`hi` would work".

It turns out that *nothing* between `lo` and `hi` works when the
type constructor is invariant. Instead, we must be keep the type
bounds, and instantiate the wildcard capture to a wildcard type
argument.

This is fine because a wildcard capture can never be referred to
in the body of the case. However, previously this could never
happen in successful cases, and we therefore used the presence of
a `TypeBounds` in the `instances` as the canonical signal for
"fail as not specific". We now use a separate `noInstances` list
to be that signal.

This change departs from the letter of the spec but not from its
spirit. As evidenced by the wording, the spec always *intended*
for "the pick" to one that would always succeed. We wrongly
assumed `hi` was always working.
---
 .../dotty/tools/dotc/core/TypeComparer.scala  | 41 +++++++++++--------
 tests/pos/i19607.scala                        | 12 ++++++
 2 files changed, 35 insertions(+), 18 deletions(-)
 create mode 100644 tests/pos/i19607.scala

diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala
index b04978357508..43b88734e669 100644
--- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala
+++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala
@@ -3410,29 +3410,38 @@ class TrackingTypeComparer(initctx: Context) extends TypeComparer(initctx) {
       // Actual matching logic
 
       val instances = Array.fill[Type](spec.captureCount)(NoType)
+      val noInstances = mutable.ListBuffer.empty[(TypeName, TypeBounds)]
 
       def rec(pattern: MatchTypeCasePattern, scrut: Type, variance: Int, scrutIsWidenedAbstract: Boolean): Boolean =
         pattern match
-          case MatchTypeCasePattern.Capture(num, isWildcard) =>
+          case MatchTypeCasePattern.Capture(num, /* isWildcard = */ true) =>
+            // instantiate the wildcard in a way that the subtype test always succeeds
+            instances(num) = variance match
+              case 1  => scrut.hiBound // actually important if we are not in a class type constructor
+              case -1 => scrut.loBound
+              case 0  => scrut
+            !instances(num).isError
+
+          case MatchTypeCasePattern.Capture(num, /* isWildcard = */ false) =>
+            def failNotSpecific(bounds: TypeBounds): TypeBounds =
+              noInstances += spec.origMatchCase.paramNames(num) -> bounds
+              bounds
+
             instances(num) = scrut match
               case scrut: TypeBounds =>
-                if isWildcard then
-                  // anything will do, as long as it conforms to the bounds for the subsequent `scrut <:< instantiatedPat` test
-                  scrut.hi
-                else if scrutIsWidenedAbstract then
-                  // always keep the TypeBounds so that we can report the correct NoInstances
-                  scrut
+                if scrutIsWidenedAbstract then
+                  failNotSpecific(scrut)
                 else
                   variance match
                     case 1  => scrut.hi
                     case -1 => scrut.lo
-                    case 0  => scrut
+                    case 0  => failNotSpecific(scrut)
               case _ =>
-                if !isWildcard && scrutIsWidenedAbstract && variance != 0 then
-                  // force a TypeBounds to report the correct NoInstances
+                if scrutIsWidenedAbstract && variance != 0 then
+                  // fail as not specific
                   // the Nothing and Any bounds are used so that they are not displayed; not for themselves in particular
-                  if variance > 0 then TypeBounds(defn.NothingType, scrut)
-                  else TypeBounds(scrut, defn.AnyType)
+                  if variance > 0 then failNotSpecific(TypeBounds(defn.NothingType, scrut))
+                  else failNotSpecific(TypeBounds(scrut, defn.AnyType))
                 else
                   scrut
             !instances(num).isError
@@ -3508,12 +3517,8 @@ class TrackingTypeComparer(initctx: Context) extends TypeComparer(initctx) {
           MatchResult.Stuck
 
       if rec(spec.pattern, scrut, variance = 1, scrutIsWidenedAbstract = false) then
-        if instances.exists(_.isInstanceOf[TypeBounds]) then
-          MatchResult.NoInstance {
-            constrainedCaseLambda.paramNames.zip(instances).collect {
-              case (name, bounds: TypeBounds) => (name, bounds)
-            }
-          }
+        if noInstances.nonEmpty then
+          MatchResult.NoInstance(noInstances.toList)
         else
           val defn.MatchCase(instantiatedPat, reduced) =
             instantiateParamsSpec(instances, constrainedCaseLambda)(constrainedCaseLambda.resultType): @unchecked
diff --git a/tests/pos/i19607.scala b/tests/pos/i19607.scala
new file mode 100644
index 000000000000..c4f65e978701
--- /dev/null
+++ b/tests/pos/i19607.scala
@@ -0,0 +1,12 @@
+trait Foo
+trait Bar[T]
+
+type MatchType[T] = T match
+  case Bar[?] => Nothing
+  case _ => T
+
+object Test:
+  def foo(b: Bar[? >: Foo]): Unit =
+    summon[MatchType[b.type] =:= Nothing]
+    summon[MatchType[Bar[? >: Foo]] =:= Nothing]
+end Test