Merge pull request #12053 from dotty-staging/fix-12049

odersky · web-flow · commit 104f4371b7fa · 2021-04-15T13:13:36.000+02:00
Explain match type reduction failures in error messages
diff --git a/compiler/src/dotty/tools/dotc/core/MatchTypeTrace.scala b/compiler/src/dotty/tools/dotc/core/MatchTypeTrace.scala
@@ -0,0 +1,103 @@
+package dotty.tools
+package dotc
+package core
+
+import Types._, Contexts._, Symbols._, Decorators._
+import util.Property
+
+/** A utility module to produce match type reduction traces in error messages.
+ */
+object MatchTypeTrace:
+
+  private enum TraceEntry:
+    case TryReduce(scrut: Type)
+    case NoMatches(scrut: Type, cases: List[Type])
+    case Stuck(scrut: Type, stuckCase: Type, otherCases: List[Type])
+  import TraceEntry._
+
+  private class MatchTrace:
+    var entries: List[TraceEntry] = Nil
+
+  private val MatchTrace = new Property.Key[MatchTrace]
+
+  /** Execute `op` and if it involves a failed match type reduction
+   *  return the trace of that reduction. Otherwise return the empty string.
+   */
+  def record(op: Context ?=> Any)(using Context): String =
+    val trace = new MatchTrace
+    inContext(ctx.fresh.setProperty(MatchTrace, trace)) {
+      op
+      if trace.entries.isEmpty then ""
+      else
+        i"""
+           |
+           |Note: a match type could not be fully reduced:
+           |
+           |${trace.entries.reverse.map(explainEntry)}%\n%"""
+    }
+
+  /** Are we running an operation that records a match type trace? */
+  def isRecording(using Context): Boolean =
+    ctx.property(MatchTrace).isDefined
+
+  private def matchTypeFail(entry: TraceEntry)(using Context) =
+    ctx.property(MatchTrace) match
+      case Some(trace) =>
+        trace.entries match
+          case (e: TryReduce) :: es => trace.entries = entry :: trace.entries
+          case _ =>
+      case _ =>
+
+  /** Record a failure that scrutinee `scrut` does not match any case in `cases`.
+   *  Only the first failure is recorded.
+   */
+  def noMatches(scrut: Type, cases: List[Type])(using Context) =
+    matchTypeFail(NoMatches(scrut, cases))
+
+  /** Record a failure that scrutinee `scrut` does not match `stuckCase` but is
+   *  not disjoint from it either, which means that the remaining cases `otherCases`
+   *  cannot be visited. Only the first failure is recorded.
+   */
+  def stuck(scrut: Type, stuckCase: Type, otherCases: List[Type])(using Context) =
+    matchTypeFail(Stuck(scrut, stuckCase, otherCases))
+
+  /** Record in the trace that we are trying to reduce `scrut` when performing `op`
+   *  If `op` succeeds the entry is removed after exit. If `op` fails, it stays.
+   */
+  def recurseWith(scrut: Type)(op: => Type)(using Context): Type =
+    ctx.property(MatchTrace) match
+      case Some(trace) =>
+        val prev = trace.entries
+        trace.entries = TryReduce(scrut) :: prev
+        val res = op
+        if res.exists then trace.entries = prev
+        res
+      case _ =>
+        op
+
+  private def caseText(tp: Type)(using Context): String = tp match
+    case tp: HKTypeLambda => caseText(tp.resultType)
+    case defn.MatchCase(pat, body) => i"case $pat => $body"
+    case _ => i"case $tp"
+
+  private def casesText(cases: List[Type])(using Context) =
+    i"${cases.map(caseText)}%\n    %"
+
+  private def explainEntry(entry: TraceEntry)(using Context): String = entry match
+    case TryReduce(scrut: Type) =>
+      i"  trying to reduce  $scrut"
+    case NoMatches(scrut, cases) =>
+      i"""  failed since selector  $scrut
+         |  matches none of the cases
+         |
+         |    ${casesText(cases)}"""
+    case Stuck(scrut, stuckCase, otherCases) =>
+      i"""  failed since selector  $scrut
+         |  does not match  ${caseText(stuckCase)}
+         |  and cannot be shown to be disjoint from it either.
+         |  Therefore, reduction cannot advance to the remaining case${if otherCases.length == 1 then "" else "s"}
+         |
+         |    ${casesText(otherCases)}"""
+
+end MatchTypeTrace
+
diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala
@@ -2798,10 +2798,20 @@ class TrackingTypeComparer(initctx: Context) extends TypeComparer(initctx) {
         Some(NoType)
     }
 
-    def recur(cases: List[Type]): Type = cases match {
-      case cas :: cases1 => matchCase(cas).getOrElse(recur(cases1))
-      case Nil => NoType
-    }
+    def recur(remaining: List[Type]): Type = remaining match
+      case cas :: remaining1 =>
+        matchCase(cas) match
+          case None =>
+            recur(remaining1)
+          case Some(NoType) =>
+            if remaining1.isEmpty then MatchTypeTrace.noMatches(scrut, cases)
+            else MatchTypeTrace.stuck(scrut, cas, remaining1)
+            NoType
+          case Some(tp) =>
+            tp
+      case Nil =>
+        MatchTypeTrace.noMatches(scrut, cases)
+        NoType
 
     inFrozenConstraint {
       // Empty types break the basic assumption that if a scrutinee and a
diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala
@@ -4025,7 +4025,9 @@ object Types {
         def tryMatchAlias = tycon.info match {
           case MatchAlias(alias) =>
             trace(i"normalize $this", typr, show = true) {
-              alias.applyIfParameterized(args).tryNormalize
+              MatchTypeTrace.recurseWith(this) {
+                alias.applyIfParameterized(args).tryNormalize
+              }
             }
           case _ =>
             NoType
@@ -4537,7 +4539,11 @@ object Types {
         }
 
       record("MatchType.reduce called")
-      if (!Config.cacheMatchReduced || myReduced == null || !isUpToDate) {
+      if !Config.cacheMatchReduced
+          || myReduced == null
+          || !isUpToDate
+          || MatchTypeTrace.isRecording
+      then
         record("MatchType.reduce computed")
         if (myReduced != null) record("MatchType.reduce cache miss")
         myReduced =
@@ -4549,7 +4555,6 @@ object Types {
               finally updateReductionContext(cmp.footprint)
             TypeComparer.tracked(matchCases)
           }
-      }
       myReduced
     }
 
diff --git a/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala b/compiler/src/dotty/tools/dotc/printing/PlainPrinter.scala
@@ -178,13 +178,14 @@ class PlainPrinter(_ctx: Context) extends Printer {
       case MatchType(bound, scrutinee, cases) =>
         changePrec(GlobalPrec) {
           def caseText(tp: Type): Text = tp match {
+            case tp: HKTypeLambda => caseText(tp.resultType)
             case defn.MatchCase(pat, body) => "case " ~ toText(pat) ~ " => " ~ toText(body)
             case _ => "case " ~ toText(tp)
           }
           def casesText = Text(cases.map(caseText), "\n")
-            atPrec(InfixPrec) { toText(scrutinee) } ~
-            keywordStr(" match ") ~ "{" ~ casesText ~ "}" ~
-            (" <: " ~ toText(bound) provided !bound.isAny)
+          atPrec(InfixPrec) { toText(scrutinee) } ~
+          keywordStr(" match ") ~ "{" ~ casesText ~ "}" ~
+          (" <: " ~ toText(bound) provided !bound.isAny)
         }.close
       case tp: PreviousErrorType if ctx.settings.XprintTypes.value =>
         "<error>" // do not print previously reported error message because they may try to print this error type again recuresevely
diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala
@@ -105,15 +105,16 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
 
   override def toTextPrefix(tp: Type): Text = controlled {
     def isOmittable(sym: Symbol) =
-      if (printDebug) false
-      else if (homogenizedView) isEmptyPrefix(sym) // drop <root> and anonymous classes, but not scala, Predef.
+      if printDebug then false
+      else if homogenizedView then isEmptyPrefix(sym) // drop <root> and anonymous classes, but not scala, Predef.
+      else if sym.isPackageObject then isOmittablePrefix(sym.owner)
       else isOmittablePrefix(sym)
     tp match {
       case tp: ThisType if isOmittable(tp.cls) =>
         ""
       case tp @ TermRef(pre, _) =>
         val sym = tp.symbol
-        if (sym.isPackageObject && !homogenizedView) toTextPrefix(pre)
+        if sym.isPackageObject && !homogenizedView && !printDebug then toTextPrefix(pre)
         else if (isOmittable(sym)) ""
         else super.toTextPrefix(tp)
       case _ => super.toTextPrefix(tp)
@@ -240,6 +241,9 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
         toTextParents(tp.parents) ~~ "{...}"
       case JavaArrayType(elemtp) =>
         toText(elemtp) ~ "[]"
+      case tp: LazyRef if !printDebug =>
+        try toText(tp.ref)
+        catch case ex: Throwable => "..."
       case tp: SelectionProto =>
         "?{ " ~ toText(tp.name) ~
            (Str(" ") provided !tp.name.toSimpleName.last.isLetterOrDigit) ~
diff --git a/compiler/src/dotty/tools/dotc/reporting/Message.scala b/compiler/src/dotty/tools/dotc/reporting/Message.scala
@@ -58,6 +58,9 @@ abstract class Message(val errorId: ErrorMessageID) { self =>
     */
   protected def explain: String
 
+  /** A message suffix that can be added for certain subclasses */
+  protected def msgSuffix: String = ""
+
   /** Does this message have an explanation?
    *  This is normally the same as `explain.nonEmpty` but can be overridden
    *  if we need a way to return `true` without actually calling the
@@ -82,7 +85,7 @@ abstract class Message(val errorId: ErrorMessageID) { self =>
   def rawMessage = message
 
   /** The message to report. <nonsensical> tags are filtered out */
-  lazy val message: String = dropNonSensical(msg)
+  lazy val message: String = dropNonSensical(msg + msgSuffix)
 
   /** The explanation to report. <nonsensical> tags are filtered out */
   lazy val explanation: String = dropNonSensical(explain)
diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala
@@ -16,7 +16,7 @@ import printing.Formatting
 import ErrorMessageID._
 import ast.Trees
 import config.{Feature, ScalaVersion}
-import typer.ErrorReporting.err
+import typer.ErrorReporting.{err, matchReductionAddendum}
 import typer.ProtoTypes.ViewProto
 import scala.util.control.NonFatal
 import StdNames.nme
@@ -45,7 +45,11 @@ import transform.SymUtils._
   abstract class TypeMsg(errorId: ErrorMessageID) extends Message(errorId):
     def kind = "Type"
 
-  abstract class TypeMismatchMsg(found: Type, expected: Type)(errorId: ErrorMessageID)(using Context) extends Message(errorId):
+  trait ShowMatchTrace(tps: Type*)(using Context) extends Message:
+    override def msgSuffix: String = matchReductionAddendum(tps*)
+
+  abstract class TypeMismatchMsg(found: Type, expected: Type)(errorId: ErrorMessageID)(using Context)
+  extends Message(errorId), ShowMatchTrace(found, expected):
     def kind = "Type Mismatch"
     def explain = err.whyNoMatchStr(found, expected)
     override def canExplain = true
@@ -281,7 +285,7 @@ import transform.SymUtils._
   end TypeMismatch
 
   class NotAMember(site: Type, val name: Name, selected: String, addendum: => String = "")(using Context)
-  extends NotFoundMsg(NotAMemberID) {
+  extends NotFoundMsg(NotAMemberID), ShowMatchTrace(site) {
     //println(i"site = $site, decls = ${site.decls}, source = ${site.typeSymbol.sourceFile}") //DEBUG
 
     def msg = {
diff --git a/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala b/compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala
@@ -51,6 +51,16 @@ object ErrorReporting {
       case _ =>
         report.error(em"missing arguments for $meth", tree.srcPos)
 
+  def matchReductionAddendum(tps: Type*)(using Context): String =
+    val collectMatchTrace = new TypeAccumulator[String]:
+      def apply(s: String, tp: Type): String =
+        if s.nonEmpty then s
+        else tp match
+          case tp: AppliedType if tp.isMatchAlias => MatchTypeTrace.record(tp.tryNormalize)
+          case tp: MatchType => MatchTypeTrace.record(tp.tryNormalize)
+          case _ => foldOver(s, tp)
+    tps.foldLeft("")(collectMatchTrace)
+
   class Errors(using Context) {
 
     /** An explanatory note to be added to error messages
@@ -253,7 +263,9 @@ class ImplicitSearchError(
       val shortMessage = userDefinedImplicitNotFoundParamMessage
         .orElse(userDefinedImplicitNotFoundTypeMessage)
         .getOrElse(defaultImplicitNotFoundMessage)
-      formatMsg(shortMessage)() ++ hiddenImplicitsAddendum
+      formatMsg(shortMessage)()
+      ++ hiddenImplicitsAddendum
+      ++ ErrorReporting.matchReductionAddendum(pt)
   }
 
   private def formatMsg(shortForm: String)(headline: String = shortForm) = arg match {
diff --git a/compiler/test-resources/repl/i5218 b/compiler/test-resources/repl/i5218
@@ -4,4 +4,4 @@ scala> 0.0 *: tuple
 val res0: (Double, Int, String, Long) = (0.0,1,2,3)
 scala> tuple ++ tuple
 val res1: Int *: String *: Long *:
-  scala.Tuple.Concat[scala.Tuple$package.EmptyTuple.type, tuple.type] = (1,2,3,1,2,3)
+  scala.Tuple.Concat[EmptyTuple.type, tuple.type] = (1,2,3,1,2,3)
diff --git a/language-server/test/dotty/tools/languageserver/CompletionTest.scala b/language-server/test/dotty/tools/languageserver/CompletionTest.scala
@@ -42,8 +42,8 @@ class CompletionTest {
 
   @Test def completionFromSyntheticPackageObject: Unit = {
     code"class Foo { val foo: IArr${m1} }".withSource
-      .completion(m1, Set(("IArray", Field, "scala.IArray"),
-                          ("IArray", Module, "scala.IArray$package.IArray$")))
+      .completion(m1, Set(("IArray", Module, "IArray$"),
+                          ("IArray", Field, "scala.IArray")))
   }
 
   @Test def completionFromJavaDefaults: Unit = {
diff --git a/tests/neg/i12049.check b/tests/neg/i12049.check
diff --git a/tests/neg/i12049.scala b/tests/neg/i12049.scala
diff --git a/tests/neg/i7056.check b/tests/neg/i7056.check
diff --git a/tests/neg/matchtype-seq.check b/tests/neg/matchtype-seq.check

Original file line number	Diff line number	Diff line change
`@@ -42,8 +42,8 @@ class CompletionTest {`
`42`	`42`
`43`	`43`	`@Test def completionFromSyntheticPackageObject: Unit = {`
`44`	`44`	`code"class Foo { val foo: IArr${m1} }".withSource`
`45`		`- .completion(m1, Set(("IArray", Field, "scala.IArray"),`
`46`		`- ("IArray", Module, "scala.IArray$package.IArray$")))`
	`45`	`+ .completion(m1, Set(("IArray", Module, "IArray$"),`
	`46`	`+ ("IArray", Field, "scala.IArray")))`
`47`	`47`	`}`
`48`	`48`
`49`	`49`	`@Test def completionFromJavaDefaults: Unit = {`