Disambiguate and explain override error messages

Author: odersky

compiler/src/dotty/tools/dotc/printing/Formatting.scala

@@ -13,6 +13,17 @@ import Highlighting.*
 
 object Formatting {
 
+  /** Essentially, a function Context => T, which can be created with `delay` */
+  abstract class Delay[T]:
+    def apply(c: Context): T
+
+  /** Delay a Context => T computation so that it is generated from the embedded
+   *  context of a string formatter instead of the enclosing context. This is needed
+   *  to make disambiguation work for such embedded computatons.
+   */
+  def delay[T](fn: Context ?=> T): Delay[T] = new Delay[T]:
+    def apply(c: Context) = fn(using c)
+
   object ShownDef:
     /** Represents a value that has been "shown" and can be consumed by StringFormatter.
      *  Not just a string because it may be a Seq that StringFormatter will intersperse with the trailing separator.
@@ -76,6 +87,9 @@ object Formatting {
       given [X: Show]: Show[Seq[X]] with
         def show(x: Seq[X]) = CtxShow(x.map(toStr))
 
+      given [X: Show]: Show[Delay[X]] = new Show:
+        def show(x: Delay[X]) = CtxShow(c ?=> x(c))
+
       given Show[Seq[Nothing]] with
         def show(x: Seq[Nothing]) = CtxShow(x)
 

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

@@ -41,6 +41,17 @@ object Message:
       i"\n$what can be rewritten automatically under -rewrite $optionStr."
     else ""
 
+  enum Disambiguation:
+    case All
+    case AllExcept(strs: List[String])
+    case None
+
+    def recordOK(str: String): Boolean = this match
+      case All => true
+      case AllExcept(strs) => !strs.contains(str)
+      case None => false
+  end Disambiguation
+
   private type Recorded = Symbol | ParamRef | SkolemType | CaptureRef
 
   private case class SeenKey(str: String, isType: Boolean)
@@ -49,7 +60,7 @@ object Message:
    *  adds superscripts for disambiguations, and can explain recorded symbols
    *  in ` where` clause
    */
-  private class Seen(disambiguate: Boolean):
+  private class Seen(disambiguate: Disambiguation):
 
     /** The set of lambdas that were opened at some point during printing. */
     private val openedLambdas = new collection.mutable.HashSet[LambdaType]
@@ -63,12 +74,12 @@ object Message:
     var nonSensical = false
 
     /** If false, stop all recordings */
-    private var recordOK = disambiguate
+    private var disambi = disambiguate
 
     /** Clear all entries and stop further entries to be added */
     def disable() =
       seen.clear()
-      recordOK = false
+      disambi = Disambiguation.None
 
     /** Record an entry `entry` with given String representation `str` and a
      *  type/term namespace identified by `isType`.
@@ -77,59 +88,61 @@ object Message:
      *  and following recordings get consecutive superscripts starting with 2.
      *  @return  The possibly superscripted version of `str`.
      */
-    def record(str: String, isType: Boolean, entry: Recorded)(using Context): String = if !recordOK then str else
-      //println(s"recording $str, $isType, $entry")
-
-      /** If `e1` is an alias of another class of the same name, return the other
-       *  class symbol instead. This normalization avoids recording e.g. scala.List
-       *  and scala.collection.immutable.List as two different types
-       */
-      def followAlias(e1: Recorded): Recorded = e1 match {
-        case e1: Symbol if e1.isAliasType =>
-          val underlying = e1.typeRef.underlyingClassRef(refinementOK = false).typeSymbol
-          if (underlying.name == e1.name) underlying else e1.namedType.dealias.typeSymbol
-        case _ => e1
-      }
-      val key = SeenKey(str, isType)
-      val existing = seen(key)
-      lazy val dealiased = followAlias(entry)
-
-      /** All lambda parameters with the same name are given the same superscript as
-       *  long as their corresponding binder has been printed.
-       *  See tests/neg/lambda-rename.scala for test cases.
-       */
-      def sameSuperscript(cur: Recorded, existing: Recorded) =
-        (cur eq existing) ||
-        (cur, existing).match
-          case (cur: ParamRef, existing: ParamRef) =>
-            (cur.paramName eq existing.paramName) &&
-            openedLambdas.contains(cur.binder) &&
-            openedLambdas.contains(existing.binder)
-          case _ =>
-            false
-
-      // The length of alts corresponds to the number of superscripts we need to print.
-      var alts = existing.dropWhile(alt => !sameSuperscript(dealiased, followAlias(alt)))
-      if alts.isEmpty then
-        alts = entry :: existing
-        seen(key) = alts
-
-      val suffix = alts.length match {
-        case 1 => ""
-        case n => n.toString.toCharArray.map {
-          case '0' => '⁰'
-          case '1' => '¹'
-          case '2' => '²'
-          case '3' => '³'
-          case '4' => '⁴'
-          case '5' => '⁵'
-          case '6' => '⁶'
-          case '7' => '⁷'
-          case '8' => '⁸'
-          case '9' => '⁹'
-        }.mkString
-      }
-      str + suffix
+    def record(str: String, isType: Boolean, entry: Recorded)(using Context): String =
+      if disambi.recordOK(str) then
+        //println(s"recording $str, $isType, $entry")
+
+        /** If `e1` is an alias of another class of the same name, return the other
+         *  class symbol instead. This normalization avoids recording e.g. scala.List
+         *  and scala.collection.immutable.List as two different types
+         */
+        def followAlias(e1: Recorded): Recorded = e1 match {
+          case e1: Symbol if e1.isAliasType =>
+            val underlying = e1.typeRef.underlyingClassRef(refinementOK = false).typeSymbol
+            if (underlying.name == e1.name) underlying else e1.namedType.dealias.typeSymbol
+          case _ => e1
+        }
+        val key = SeenKey(str, isType)
+        val existing = seen(key)
+        lazy val dealiased = followAlias(entry)
+
+        /** All lambda parameters with the same name are given the same superscript as
+         *  long as their corresponding binder has been printed.
+         *  See tests/neg/lambda-rename.scala for test cases.
+         */
+        def sameSuperscript(cur: Recorded, existing: Recorded) =
+          (cur eq existing) ||
+          (cur, existing).match
+            case (cur: ParamRef, existing: ParamRef) =>
+              (cur.paramName eq existing.paramName) &&
+              openedLambdas.contains(cur.binder) &&
+              openedLambdas.contains(existing.binder)
+            case _ =>
+              false
+
+        // The length of alts corresponds to the number of superscripts we need to print.
+        var alts = existing.dropWhile(alt => !sameSuperscript(dealiased, followAlias(alt)))
+        if alts.isEmpty then
+          alts = entry :: existing
+          seen(key) = alts
+
+        val suffix = alts.length match {
+          case 1 => ""
+          case n => n.toString.toCharArray.map {
+            case '0' => '⁰'
+            case '1' => '¹'
+            case '2' => '²'
+            case '3' => '³'
+            case '4' => '⁴'
+            case '5' => '⁵'
+            case '6' => '⁶'
+            case '7' => '⁷'
+            case '8' => '⁸'
+            case '9' => '⁹'
+          }.mkString
+        }
+        str + suffix
+      else str
     end record
 
     /** Create explanation for single `Recorded` type or symbol */
@@ -394,13 +407,15 @@ abstract class Message(val errorId: ErrorMessageID)(using Context) { self =>
    */
   def isNonSensical: Boolean = { message; myIsNonSensical }
 
-  private var disambiguate: Boolean = true
+  private var disambiguate: Disambiguation = Disambiguation.All
 
-  def withoutDisambiguation(): this.type =
-    disambiguate = false
+  def withDisambiguation(disambi: Disambiguation): this.type =
+    disambiguate = disambi
     this
 
-  private def inMessageContext(disambiguate: Boolean)(op: Context ?=> String): String =
+  def withoutDisambiguation(): this.type = withDisambiguation(Disambiguation.None)
+
+  private def inMessageContext(disambiguate: Disambiguation)(op: Context ?=> String): String =
     if ctx eq NoContext then op
     else
       val msgContext = ctx.printer match
@@ -419,7 +434,7 @@ abstract class Message(val errorId: ErrorMessageID)(using Context) { self =>
 
   /** The explanation to report. <nonsensical> tags are filtered out */
   @threadUnsafe lazy val explanation: String =
-    inMessageContext(disambiguate = false)(explain)
+    inMessageContext(disambiguate = Disambiguation.None)(explain)
 
   /** The implicit `Context` in messages is a large thing that we don't want
     * persisted. This method gets around that by duplicating the message,

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

@@ -23,7 +23,7 @@ import typer.Implicits.*
 import typer.Inferencing
 import scala.util.control.NonFatal
 import StdNames.nme
-import printing.Formatting.hl
+import Formatting.{hl, delay}
 import ast.Trees.*
 import ast.untpd
 import ast.tpd
@@ -37,6 +37,7 @@ import scala.jdk.CollectionConverters.*
 import dotty.tools.dotc.util.SourceFile
 import dotty.tools.dotc.config.SourceVersion
 import DidYouMean.*
+import Message.Disambiguation
 
 /**  Messages
   *  ========
@@ -1172,6 +1173,7 @@ class OverrideError(
     member: Symbol, other: Symbol,
     memberTp: Type, otherTp: Type)(using Context)
 extends DeclarationMsg(OverrideErrorID), NoDisambiguation:
+  withDisambiguation(Disambiguation.AllExcept(List(member.name.toString)))
   def msg(using Context) =
     val isConcreteOverAbstract =
       (other.owner isSubClass member.owner) && other.is(Deferred) && !member.is(Deferred)
@@ -1181,8 +1183,8 @@ extends DeclarationMsg(OverrideErrorID), NoDisambiguation:
             |(Note that ${err.infoStringWithLocation(other, base)} is abstract,
             |and is therefore overridden by concrete ${err.infoStringWithLocation(member, base)})"""
         else ""
-    i"""error overriding ${err.infoStringWithLocation(other, base)};
-        |  ${err.infoString(member, base, showLocation = member.owner != base.typeSymbol)} $core$addendum"""
+    i"""error overriding ${delay(err.infoStringWithLocation(other, base))};
+        |  ${delay(err.infoString(member, base, showLocation = member.owner != base.typeSymbol))} $core$addendum"""
   override def canExplain =
     memberTp.exists && otherTp.exists
   def explain(using Context) =

tests/neg-custom-args/captures/lazylist.check

@@ -39,4 +39,6 @@
    |      error overriding method tail in class LazyList of type -> lazylists.LazyList[Nothing];
    |        method tail of type -> lazylists.LazyList[Nothing]^ has incompatible type
    |
+   |      where:    ^ refers to a fresh root capability in the result type of method tail
+   |
    | longer explanation available when compiling with `-explain`

tests/neg-custom-args/captures/sep-lazyListState.check

@@ -0,0 +1,5 @@
+-- Error: tests/neg-custom-args/captures/sep-lazyListState.scala:13:14 -------------------------------------------------
+13 |    def tail: LazyListIterable[A]^ = tl // error
+   |              ^^^^^^^^^^^^^^^^^^^^
+   |    Separation failure: method tail's result type LazyListIterable[A]^ hides non-local this of class class Cons.
+   |    The access must be in a @consume method to allow this.

tests/neg-custom-args/captures/sep-lazyListState.scala

@@ -0,0 +1,14 @@
+class LazyListIterable[+A]
+
+private sealed trait State[+A]:
+  def head: A
+  def tail: LazyListIterable[A]^
+
+private object State:
+  object Empty extends State[Nothing]:
+    def head: Nothing = throw new NoSuchElementException("head of empty lazy list")
+    def tail: LazyListIterable[Nothing] = throw new UnsupportedOperationException("tail of empty lazy list")
+
+  final class Cons[A](val head: A, tl: LazyListIterable[A]^) extends State[A]:
+    def tail: LazyListIterable[A]^ = tl // error
+

tests/neg-custom-args/captures/unsound-reach-4.check

@@ -18,6 +18,9 @@
 17 |  def use(@consume x: F): File^ = x // error @consume override
    |      ^
    |      error overriding method use in trait Foo of type (x: File^): box File^;
-   |        method use of type (x: File^): File^ has incompatible type
+   |        method use of type (x: File^): File^² has incompatible type
+   |
+   |      where:    ^  refers to the universal root capability
+   |                ^² refers to a root capability associated with the result type of (x: File^): File^²
    |
    | longer explanation available when compiling with `-explain`

tests/neg-custom-args/captures/widen-reach.check

@@ -29,6 +29,9 @@
 9 |  val foo: IO^ -> IO^ = x => x // error // error
   |      ^
   |      error overriding value foo in trait Foo of type IO^ -> box IO^;
-  |        value foo of type IO^ -> IO^ has incompatible type
+  |        value foo of type IO^ -> IO^² has incompatible type
+  |
+  |      where:    ^  refers to the universal root capability
+  |                ^² refers to a fresh root capability in the type of value foo
   |
   | longer explanation available when compiling with `-explain`

tests/neg/abstract-givens.check

@@ -10,7 +10,10 @@
 -- [E164] Declaration Error: tests/neg/abstract-givens.scala:9:8 -------------------------------------------------------
 9 |  given z[T](using T): Seq[T] = List(summon[T]) // error
   |        ^
-  |        error overriding given instance z in trait T of type [T](using x$1: T): List[T];
+  |        error overriding given instance z in trait T² of type [T](using x$1: T): List[T];
   |          given instance z of type [T](using x$1: T): Seq[T] has incompatible type
   |
+  |        where:    T  is a type variable
+  |                  T² is a trait in the empty package
+  |
   | longer explanation available when compiling with `-explain`