Merge pull request #11643 from dotty-staging/fix-11637

Merge -explain-types behavior into -explain behavior

Author: odersky

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

@@ -103,7 +103,9 @@ class ScalaSettings extends Settings.SettingGroup with CommonScalaSettings {
   val semanticdbTarget: Setting[String] = PathSetting("-semanticdb-target", "Specify an alternative output directory for SemanticDB files.", "")
 
   val deprecation: Setting[Boolean] = BooleanSetting("-deprecation", "Emit warning and location for usages of deprecated APIs.", aliases = List("--deprecation"))
-  val explainTypes: Setting[Boolean] = BooleanSetting("-explain-types", "Explain type errors in more detail.", aliases = List("--explain-types"))
+  val explainTypes: Setting[Boolean] = BooleanSetting("-explain-types", "Explain type errors in more detail (deprecated, use -explain instead).", aliases = List("--explain-types"))
+    // this setting is necessary for cross compilation, since it is mentioned in sbt-tpolecat, for instance
+    // it is otherwise subsumed by -explain, and should be dropped as soon as we can.
   val explain: Setting[Boolean] = BooleanSetting("-explain", "Explain errors in more detail.", aliases = List("--explain"))
   val feature: Setting[Boolean] = BooleanSetting("-feature", "Emit warning and location for usages of features that should be imported explicitly.", aliases = List("--feature"))
   val release: Setting[String] = ChoiceSetting("-release", "release", "Compile code with classes specific to the given version of the Java platform available on the classpath and emit bytecode for this version.", supportedReleaseVersions, "", aliases = List("--release"))

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

@@ -168,4 +168,7 @@ abstract class Constraint extends Showable {
 
   /** Check that constraint only refers to TypeParamRefs bound by itself */
   def checkClosed()(using Context): Unit
+
+  /** A string describing the constraint's contents without a header or trailer */
+  def contentsToString(using Context): String
 }

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

@@ -648,7 +648,7 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
 
 // ---------- toText -----------------------------------------------------
 
-  override def toText(printer: Printer): Text = {
+  private def contentsToText(printer: Printer): Text =
     //Printer.debugPrintUnique = true
     def entryText(tp: Type) = tp match {
       case tp: TypeBounds =>
@@ -657,20 +657,19 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
         " := " ~ tp.toText(printer)
     }
     val indent = 3
-    val header: Text = "Constraint("
-    val uninstVarsText = " uninstVars = " ~
-      Text(uninstVars map (_.toText(printer)), ", ") ~ ";"
+    val uninstVarsText = " uninstantiated variables: " ~
+      Text(uninstVars.map(_.toText(printer)), ", ")
     val constrainedText =
-      " constrained types = " ~ Text(domainLambdas map (_.toText(printer)), ", ")
+      " constrained types: " ~ Text(domainLambdas map (_.toText(printer)), ", ")
     val boundsText =
-      " bounds = " ~ {
+      " bounds: " ~ {
         val assocs =
           for (param <- domainParams)
           yield (" " * indent) ~ param.toText(printer) ~ entryText(entry(param))
         Text(assocs, "\n")
       }
     val orderingText =
-      " ordering = " ~ {
+      " ordering: " ~ {
         val deps =
           for {
             param <- domainParams
@@ -683,8 +682,13 @@ class OrderingConstraint(private val boundsMap: ParamBounds,
         Text(deps, "\n")
       }
     //Printer.debugPrintUnique = false
-    Text.lines(List(header, uninstVarsText, constrainedText, boundsText, orderingText, ")"))
-  }
+    Text.lines(List(uninstVarsText, constrainedText, boundsText, orderingText))
+
+  override def toText(printer: Printer): Text =
+    Text.lines(List("Constraint(", contentsToText(printer), ")"))
+
+  def contentsToString(using Context): String =
+    contentsToText(ctx.printer).show
 
   override def toString: String = {
     def entryText(tp: Type): String = tp match {

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

@@ -206,7 +206,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
    *  code would have two extra parameters for each of the many calls that go from
    *  one sub-part of isSubType to another.
    */
-  protected def recur(tp1: Type, tp2: Type): Boolean = trace(s"isSubType ${traceInfo(tp1, tp2)} ${approx.show}", subtyping) {
+  protected def recur(tp1: Type, tp2: Type): Boolean = trace(s"isSubType ${traceInfo(tp1, tp2)}${approx.show}", subtyping) {
 
     def monitoredIsSubType = {
       if (pendingSubTypes == null) {
@@ -2276,13 +2276,6 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
       NoType
   }
 
-  /** Show type, handling type types better than the default */
-  private def showType(tp: Type)(using Context) = tp match {
-    case ClassInfo(_, cls, _, _, _) => cls.showLocated
-    case bounds: TypeBounds => "type bounds" + bounds.show
-    case _ => tp.show
-  }
-
   /** A comparison function to pick a winner in case of a merge conflict */
   private def isAsGood(tp1: Type, tp2: Type): Boolean = tp1 match {
     case tp1: ClassInfo =>
@@ -2542,10 +2535,10 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
     finally myInstance = saved
 
   /** The trace of comparison operations when performing `op` */
-  def explained[T](op: ExplainingTypeComparer => T)(using Context): String =
+  def explained[T](op: ExplainingTypeComparer => T, header: String = "Subtype trace:")(using Context): String =
     val cmp = explainingTypeComparer
     inSubComparer(cmp)(op)
-    cmp.lastTrace()
+    cmp.lastTrace(header)
 
   def tracked[T](op: TrackingTypeComparer => T)(using Context): T =
     inSubComparer(trackingTypeComparer)(op)
@@ -2561,10 +2554,13 @@ object TypeComparer {
     var tpe: Type = NoType
   }
 
-  private[core] def show(res: Any)(using Context): String = res match {
-    case res: printing.Showable if !ctx.settings.YexplainLowlevel.value => res.show
-    case _ => String.valueOf(res)
-  }
+  private[core] def show(res: Any)(using Context): String =
+    if ctx.settings.YexplainLowlevel.value then String.valueOf(res)
+    else res match
+      case ClassInfo(_, cls, _, _, _) => cls.showLocated
+      case bounds: TypeBounds => i"type bounds [$bounds]"
+      case res: printing.Showable => res.show
+      case _ => String.valueOf(res)
 
   /** The approximation state indicates how the pair of types currently compared
    *  relates to the types compared originally.
@@ -2591,8 +2587,8 @@ object TypeComparer {
       def addLow: Repr = approx | LoApprox
       def addHigh: Repr = approx | HiApprox
       def show: String =
-        val lo = if low then "LoApprox" else ""
-        val hi = if high then "HiApprox" else ""
+        val lo = if low then " (left is approximated)" else ""
+        val hi = if high then " (right is approximated)" else ""
         lo ++ hi
   end ApproxState
   type ApproxState = ApproxState.Repr
@@ -2694,8 +2690,8 @@ object TypeComparer {
   def constrainPatternType(pat: Type, scrut: Type)(using Context): Boolean =
     comparing(_.constrainPatternType(pat, scrut))
 
-  def explained[T](op: ExplainingTypeComparer => T)(using Context): String =
-    comparing(_.explained(op))
+  def explained[T](op: ExplainingTypeComparer => T, header: String = "Subtype trace:")(using Context): String =
+    comparing(_.explained(op, header))
 
   def tracked[T](op: TrackingTypeComparer => T)(using Context): T =
     comparing(_.tracked(op))
@@ -2844,17 +2840,20 @@ class ExplainingTypeComparer(initctx: Context) extends TypeComparer(initctx) {
       res
     }
 
+  private def frozenNotice: String =
+    if frozenConstraint then " in frozen constraint" else ""
+
   override def isSubType(tp1: Type, tp2: Type, approx: ApproxState): Boolean =
     def moreInfo =
       if Config.verboseExplainSubtype || ctx.settings.verbose.value
       then s" ${tp1.getClass} ${tp2.getClass}"
       else ""
-    traceIndented(s"${show(tp1)} <:< ${show(tp2)}$moreInfo ${approx.show} ${if (frozenConstraint) " frozen" else ""}") {
+    traceIndented(s"${show(tp1)}  <:  ${show(tp2)}$moreInfo${approx.show}$frozenNotice") {
       super.isSubType(tp1, tp2, approx)
     }
 
   override def recur(tp1: Type, tp2: Type): Boolean =
-    traceIndented(s"${show(tp1)} <:< ${show(tp2)} recur ${if (frozenConstraint) " frozen" else ""}") {
+    traceIndented(s"${show(tp1)}  <:  ${show(tp2)} (recurring)$frozenNotice") {
       super.recur(tp1, tp2)
     }
 
@@ -2878,5 +2877,5 @@ class ExplainingTypeComparer(initctx: Context) extends TypeComparer(initctx) {
       super.addConstraint(param, bound, fromBelow)
     }
 
-  def lastTrace(): String = "Subtype trace:" + { try b.toString finally b.clear() }
+  def lastTrace(header: String): String = header + { try b.toString finally b.clear() }
 }

compiler/src/dotty/tools/dotc/reporting/ConsoleReporter.scala

@@ -35,7 +35,7 @@ class ConsoleReporter(
 
     if (didPrint && shouldExplain(dia))
       printMessage(explanation(dia.msg))
-    else if (didPrint && dia.msg.explanation.nonEmpty)
+    else if (didPrint && dia.msg.canExplain)
       printMessage("\nlonger explanation available when compiling with `-explain`")
   }
 

compiler/src/dotty/tools/dotc/reporting/Diagnostic.scala

@@ -12,7 +12,9 @@ import java.util.Optional
 object Diagnostic:
 
   def shouldExplain(dia: Diagnostic)(using Context): Boolean =
-    dia.msg.explanation.nonEmpty && ctx.settings.explain.value
+    ctx.settings.explain.value && dia.msg.canExplain
+    || ctx.settings.explainTypes.value && dia.msg.isInstanceOf[TypeMismatchMsg]
+        // keep old explain-types behavior for backwards compatibility and cross-compilation
 
   // `Diagnostics to be consumed by `Reporter` ---------------------- //
   class Error(

compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

@@ -170,7 +170,9 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
     TraitMayNotDefineNativeMethodID,
     JavaEnumParentArgsID,
     AlreadyDefinedID,
-    CaseClassInInlinedCodeID
+    CaseClassInInlinedCodeID,
+    OverrideTypeMismatchErrorID,
+    OverrideErrorID
 
   def errorNumber = ordinal - 2
 }

compiler/src/dotty/tools/dotc/reporting/Message.scala

@@ -58,6 +58,13 @@ abstract class Message(val errorId: ErrorMessageID) { self =>
     */
   protected def explain: 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
+   *  `explain` method.
+   */
+  def canExplain: Boolean = explain.nonEmpty
+
   private var myMsg: String | Null = null
   private var myIsNonSensical: Boolean = false
 
@@ -95,22 +102,25 @@ abstract class Message(val errorId: ErrorMessageID) { self =>
     * that was captured in the original message.
     */
   def persist: Message = new Message(errorId) {
-    val kind    = self.kind
-    val msg     = self.msg
-    val explain = self.explain
+    val kind       = self.kind
+    val msg        = self.msg
+    val explain    = self.explain
+    override val canExplain = self.canExplain
   }
 
   def append(suffix: => String): Message = mapMsg(_ ++ suffix)
 
   def mapMsg(f: String => String): Message = new Message(errorId):
-    val kind    = self.kind
-    def msg     = f(self.msg)
-    def explain = self.explain
+    val kind       = self.kind
+    def msg        = f(self.msg)
+    def explain    = self.explain
+    override def canExplain = self.canExplain
 
   def appendExplanation(suffix: => String): Message = new Message(errorId):
-    val kind    = self.kind
-    def msg     = self.msg
-    def explain = self.explain ++ suffix
+    val kind       = self.kind
+    def msg        = self.msg
+    def explain    = self.explain ++ suffix
+    override def canExplain = true
 
   override def toString = msg
 }

compiler/src/dotty/tools/dotc/reporting/messages.scala

@@ -45,8 +45,10 @@ import transform.SymUtils._
   abstract class TypeMsg(errorId: ErrorMessageID) extends Message(errorId):
     def kind = "Type"
 
-  abstract class TypeMismatchMsg(errorId: ErrorMessageID) extends Message(errorId):
+  abstract class TypeMismatchMsg(found: Type, expected: Type)(errorId: ErrorMessageID)(using Context) extends Message(errorId):
     def kind = "Type Mismatch"
+    def explain = err.whyNoMatchStr(found, expected)
+    override def canExplain = true
 
   abstract class NamingMsg(errorId: ErrorMessageID) extends Message(errorId):
     def kind = "Naming"
@@ -237,7 +239,7 @@ import transform.SymUtils._
   }
 
   class TypeMismatch(found: Type, expected: Type, addenda: => String*)(using Context)
-    extends TypeMismatchMsg(TypeMismatchID):
+    extends TypeMismatchMsg(found, expected)(TypeMismatchID):
 
     // replace constrained TypeParamRefs and their typevars by their bounds where possible
     // the idea is that if the bounds are also not-subtypes of each other to report
@@ -275,9 +277,7 @@ import transform.SymUtils._
       val (foundStr, expectedStr) = Formatting.typeDiff(found2, expected2)(using printCtx)
       s"""|Found:    $foundStr
           |Required: $expectedStr""".stripMargin
-        + whereSuffix + err.whyNoMatchStr(found, expected) + postScript
-
-    def explain = ""
+        + whereSuffix + postScript
   end TypeMismatch
 
   class NotAMember(site: Type, val name: Name, selected: String, addendum: => String = "")(using Context)
@@ -1075,6 +1075,14 @@ import transform.SymUtils._
            |"""
   }
 
+  class OverrideError(override val msg: String) extends DeclarationMsg(OverrideErrorID):
+    def explain = ""
+
+  class OverrideTypeMismatchError(override val msg: String, memberTp: Type, otherTp: Type)(using Context)
+  extends DeclarationMsg(OverrideTypeMismatchErrorID):
+    def explain = err.whyNoMatchStr(memberTp, otherTp)
+    override def canExplain = true
+
   class ForwardReferenceExtendsOverDefinition(value: Symbol, definition: Symbol)(using Context)
   extends ReferenceMsg(ForwardReferenceExtendsOverDefinitionID) {
     def msg = em"${definition.name} is a forward reference extending over the definition of ${value.name}"
@@ -1325,7 +1333,7 @@ import transform.SymUtils._
         if (isNullary) "\nNullary methods may not be called with parenthesis"
         else ""
 
-      "You have specified more parameter lists as defined in the method definition(s)." + addendum
+      "You have specified more parameter lists than defined in the method definition(s)." + addendum
     }
 
   }
@@ -1415,36 +1423,22 @@ import transform.SymUtils._
   }
 
   class DoesNotConformToBound(tpe: Type, which: String, bound: Type)(using Context)
-    extends TypeMismatchMsg(DoesNotConformToBoundID) {
-    def msg = em"Type argument ${tpe} does not conform to $which bound $bound${err.whyNoMatchStr(tpe, bound)}"
-    def explain = ""
+    extends TypeMismatchMsg(tpe, bound)(DoesNotConformToBoundID) {
+    def msg = em"Type argument ${tpe} does not conform to $which bound $bound"
   }
 
   class DoesNotConformToSelfType(category: String, selfType: Type, cls: Symbol,
-                                      otherSelf: Type, relation: String, other: Symbol)(
+                                 otherSelf: Type, relation: String, other: Symbol)(
     implicit ctx: Context)
-    extends TypeMismatchMsg(DoesNotConformToSelfTypeID) {
+    extends TypeMismatchMsg(selfType, otherSelf)(DoesNotConformToSelfTypeID) {
     def msg = em"""$category: self type $selfType of $cls does not conform to self type $otherSelf
                   |of $relation $other"""
-    def explain =
-      em"""You mixed in $other which requires self type $otherSelf, but $cls has self type
-          |$selfType and does not inherit from $otherSelf.
-          |
-          |Note: Self types are indicated with the notation
-          |  ${s"class "}$other ${hl("{ this: ")}$otherSelf${hl(" => ")}
-        """
   }
 
   class DoesNotConformToSelfTypeCantBeInstantiated(tp: Type, selfType: Type)(
     implicit ctx: Context)
-    extends TypeMismatchMsg(DoesNotConformToSelfTypeCantBeInstantiatedID) {
+    extends TypeMismatchMsg(tp, selfType)(DoesNotConformToSelfTypeCantBeInstantiatedID) {
     def msg = em"""$tp does not conform to its self type $selfType; cannot be instantiated"""
-    def explain =
-      em"""To create an instance of $tp it needs to inherit $selfType in some way.
-          |
-          |Note: Self types are indicated with the notation
-          |  ${s"class "}$tp ${hl("{ this: ")}$selfType${hl(" => ")}
-          |"""
   }
 
   class AbstractMemberMayNotHaveModifier(sym: Symbol, flag: FlagSet)(
@@ -2208,7 +2202,7 @@ import transform.SymUtils._
   }
 
   class NoMatchingOverload(val alternatives: List[SingleDenotation], pt: Type)(using Context)
-    extends TypeMismatchMsg(NoMatchingOverloadID) {
+    extends TypeMsg(NoMatchingOverloadID) {
     def msg =
       em"""None of the ${err.overloadedAltsStr(alternatives)}
           |match ${err.expectedTypeStr(pt)}"""

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

@@ -117,14 +117,25 @@ object ErrorReporting {
     }
 
     /** A subtype log explaining why `found` does not conform to `expected` */
-    def whyNoMatchStr(found: Type, expected: Type): String = {
-      if (ctx.settings.explainTypes.value)
-        i"""
-           |${ctx.typerState.constraint}
-           |${TypeComparer.explained(_.isSubType(found, expected))}"""
-      else
-        ""
-    }
+    def whyNoMatchStr(found: Type, expected: Type): String =
+      val header =
+        i"""I tried to show that
+          |  $found
+          |conforms to
+          |  $expected
+          |but the comparison trace ended with `false`:
+          """
+      val c = ctx.typerState.constraint
+      val constraintText =
+        if c.domainLambdas.isEmpty then
+          "the empty constraint"
+        else
+          i"""a constraint with:
+             |${c.contentsToString}"""
+      i"""
+        |${TypeComparer.explained(_.isSubType(found, expected), header)}
+        |
+        |The tests were made under $constraintText"""
 
     /** Format `raw` implicitNotFound or implicitAmbiguous argument, replacing
      *  all occurrences of `${X}` where `X` is in `paramNames` with the