Skip to content

Commit 223b9c6

Browse files
oderskyodersky
committed
Disambiguate and explain override error messages
1 parent 6056ae2 commit 223b9c6

File tree

9 files changed

+116
-66
lines changed

9 files changed

+116
-66
lines changed

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

+3
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ object Formatting {
7676
given [X: Show]: Show[Seq[X]] with
7777
def show(x: Seq[X]) = CtxShow(x.map(toStr))
7878

79+
given [X: Show]: Show[Context => X] = new Show:
80+
def show(x: Context => X) = CtxShow(c ?=> x(c))
81+
7982
given Show[Seq[Nothing]] with
8083
def show(x: Seq[Nothing]) = CtxShow(x)
8184

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

+76-61
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,17 @@ object Message:
4141
i"\n$what can be rewritten automatically under -rewrite $optionStr."
4242
else ""
4343

44+
enum Disambiguation:
45+
case All
46+
case AllExcept(strs: List[String])
47+
case None
48+
49+
def recordOK(str: String): Boolean = this match
50+
case All => true
51+
case AllExcept(strs) => !strs.contains(str)
52+
case None => false
53+
end Disambiguation
54+
4455
private type Recorded = Symbol | ParamRef | SkolemType | CaptureRef
4556

4657
private case class SeenKey(str: String, isType: Boolean)
@@ -49,7 +60,7 @@ object Message:
4960
* adds superscripts for disambiguations, and can explain recorded symbols
5061
* in ` where` clause
5162
*/
52-
private class Seen(disambiguate: Boolean):
63+
private class Seen(disambiguate: Disambiguation):
5364

5465
/** The set of lambdas that were opened at some point during printing. */
5566
private val openedLambdas = new collection.mutable.HashSet[LambdaType]
@@ -63,12 +74,12 @@ object Message:
6374
var nonSensical = false
6475

6576
/** If false, stop all recordings */
66-
private var recordOK = disambiguate
77+
private var disambi = disambiguate
6778

6879
/** Clear all entries and stop further entries to be added */
6980
def disable() =
7081
seen.clear()
71-
recordOK = false
82+
disambi = Disambiguation.None
7283

7384
/** Record an entry `entry` with given String representation `str` and a
7485
* type/term namespace identified by `isType`.
@@ -77,59 +88,61 @@ object Message:
7788
* and following recordings get consecutive superscripts starting with 2.
7889
* @return The possibly superscripted version of `str`.
7990
*/
80-
def record(str: String, isType: Boolean, entry: Recorded)(using Context): String = if !recordOK then str else
81-
//println(s"recording $str, $isType, $entry")
82-
83-
/** If `e1` is an alias of another class of the same name, return the other
84-
* class symbol instead. This normalization avoids recording e.g. scala.List
85-
* and scala.collection.immutable.List as two different types
86-
*/
87-
def followAlias(e1: Recorded): Recorded = e1 match {
88-
case e1: Symbol if e1.isAliasType =>
89-
val underlying = e1.typeRef.underlyingClassRef(refinementOK = false).typeSymbol
90-
if (underlying.name == e1.name) underlying else e1.namedType.dealias.typeSymbol
91-
case _ => e1
92-
}
93-
val key = SeenKey(str, isType)
94-
val existing = seen(key)
95-
lazy val dealiased = followAlias(entry)
96-
97-
/** All lambda parameters with the same name are given the same superscript as
98-
* long as their corresponding binder has been printed.
99-
* See tests/neg/lambda-rename.scala for test cases.
100-
*/
101-
def sameSuperscript(cur: Recorded, existing: Recorded) =
102-
(cur eq existing) ||
103-
(cur, existing).match
104-
case (cur: ParamRef, existing: ParamRef) =>
105-
(cur.paramName eq existing.paramName) &&
106-
openedLambdas.contains(cur.binder) &&
107-
openedLambdas.contains(existing.binder)
108-
case _ =>
109-
false
110-
111-
// The length of alts corresponds to the number of superscripts we need to print.
112-
var alts = existing.dropWhile(alt => !sameSuperscript(dealiased, followAlias(alt)))
113-
if alts.isEmpty then
114-
alts = entry :: existing
115-
seen(key) = alts
116-
117-
val suffix = alts.length match {
118-
case 1 => ""
119-
case n => n.toString.toCharArray.map {
120-
case '0' => '⁰'
121-
case '1' => '¹'
122-
case '2' => '²'
123-
case '3' => '³'
124-
case '4' => '⁴'
125-
case '5' => '⁵'
126-
case '6' => '⁶'
127-
case '7' => '⁷'
128-
case '8' => '⁸'
129-
case '9' => '⁹'
130-
}.mkString
131-
}
132-
str + suffix
91+
def record(str: String, isType: Boolean, entry: Recorded)(using Context): String =
92+
if disambi.recordOK(str) then
93+
//println(s"recording $str, $isType, $entry")
94+
95+
/** If `e1` is an alias of another class of the same name, return the other
96+
* class symbol instead. This normalization avoids recording e.g. scala.List
97+
* and scala.collection.immutable.List as two different types
98+
*/
99+
def followAlias(e1: Recorded): Recorded = e1 match {
100+
case e1: Symbol if e1.isAliasType =>
101+
val underlying = e1.typeRef.underlyingClassRef(refinementOK = false).typeSymbol
102+
if (underlying.name == e1.name) underlying else e1.namedType.dealias.typeSymbol
103+
case _ => e1
104+
}
105+
val key = SeenKey(str, isType)
106+
val existing = seen(key)
107+
lazy val dealiased = followAlias(entry)
108+
109+
/** All lambda parameters with the same name are given the same superscript as
110+
* long as their corresponding binder has been printed.
111+
* See tests/neg/lambda-rename.scala for test cases.
112+
*/
113+
def sameSuperscript(cur: Recorded, existing: Recorded) =
114+
(cur eq existing) ||
115+
(cur, existing).match
116+
case (cur: ParamRef, existing: ParamRef) =>
117+
(cur.paramName eq existing.paramName) &&
118+
openedLambdas.contains(cur.binder) &&
119+
openedLambdas.contains(existing.binder)
120+
case _ =>
121+
false
122+
123+
// The length of alts corresponds to the number of superscripts we need to print.
124+
var alts = existing.dropWhile(alt => !sameSuperscript(dealiased, followAlias(alt)))
125+
if alts.isEmpty then
126+
alts = entry :: existing
127+
seen(key) = alts
128+
129+
val suffix = alts.length match {
130+
case 1 => ""
131+
case n => n.toString.toCharArray.map {
132+
case '0' => '⁰'
133+
case '1' => '¹'
134+
case '2' => '²'
135+
case '3' => '³'
136+
case '4' => '⁴'
137+
case '5' => '⁵'
138+
case '6' => '⁶'
139+
case '7' => '⁷'
140+
case '8' => '⁸'
141+
case '9' => '⁹'
142+
}.mkString
143+
}
144+
str + suffix
145+
else str
133146
end record
134147

135148
/** Create explanation for single `Recorded` type or symbol */
@@ -394,13 +407,15 @@ abstract class Message(val errorId: ErrorMessageID)(using Context) { self =>
394407
*/
395408
def isNonSensical: Boolean = { message; myIsNonSensical }
396409

397-
private var disambiguate: Boolean = true
410+
private var disambiguate: Disambiguation = Disambiguation.All
398411

399-
def withoutDisambiguation(): this.type =
400-
disambiguate = false
412+
def withDisambiguation(disambi: Disambiguation): this.type =
413+
disambiguate = disambi
401414
this
402415

403-
private def inMessageContext(disambiguate: Boolean)(op: Context ?=> String): String =
416+
def withoutDisambiguation(): this.type = withDisambiguation(Disambiguation.None)
417+
418+
private def inMessageContext(disambiguate: Disambiguation)(op: Context ?=> String): String =
404419
if ctx eq NoContext then op
405420
else
406421
val msgContext = ctx.printer match
@@ -419,7 +434,7 @@ abstract class Message(val errorId: ErrorMessageID)(using Context) { self =>
419434

420435
/** The explanation to report. <nonsensical> tags are filtered out */
421436
@threadUnsafe lazy val explanation: String =
422-
inMessageContext(disambiguate = false)(explain)
437+
inMessageContext(disambiguate = Disambiguation.None)(explain)
423438

424439
/** The implicit `Context` in messages is a large thing that we don't want
425440
* persisted. This method gets around that by duplicating the message,

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ import scala.jdk.CollectionConverters.*
3737
import dotty.tools.dotc.util.SourceFile
3838
import dotty.tools.dotc.config.SourceVersion
3939
import DidYouMean.*
40+
import dotty.tools.dotc.reporting.Message.Disambiguation
4041

4142
/** Messages
4243
* ========
@@ -1172,6 +1173,7 @@ class OverrideError(
11721173
member: Symbol, other: Symbol,
11731174
memberTp: Type, otherTp: Type)(using Context)
11741175
extends DeclarationMsg(OverrideErrorID), NoDisambiguation:
1176+
withDisambiguation(Disambiguation.AllExcept(List(member.name.toString)))
11751177
def msg(using Context) =
11761178
val isConcreteOverAbstract =
11771179
(other.owner isSubClass member.owner) && other.is(Deferred) && !member.is(Deferred)
@@ -1181,8 +1183,8 @@ extends DeclarationMsg(OverrideErrorID), NoDisambiguation:
11811183
|(Note that ${err.infoStringWithLocation(other, base)} is abstract,
11821184
|and is therefore overridden by concrete ${err.infoStringWithLocation(member, base)})"""
11831185
else ""
1184-
i"""error overriding ${err.infoStringWithLocation(other, base)};
1185-
| ${err.infoString(member, base, showLocation = member.owner != base.typeSymbol)} $core$addendum"""
1186+
i"""error overriding ${(ctx: Context) => err(using ctx).infoStringWithLocation(other, base)};
1187+
| ${(ctx: Context) => err(using ctx).infoString(member, base, showLocation = member.owner != base.typeSymbol)} $core$addendum"""
11861188
override def canExplain =
11871189
memberTp.exists && otherTp.exists
11881190
def explain(using Context) =

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

+2
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,6 @@
3939
| error overriding method tail in class LazyList of type -> lazylists.LazyList[Nothing];
4040
| method tail of type -> lazylists.LazyList[Nothing]^ has incompatible type
4141
|
42+
| where: ^ refers to a fresh root capability in the result type of method tail
43+
|
4244
| longer explanation available when compiling with `-explain`

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

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

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

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
class LazyListIterable[+A]
2+
3+
private sealed trait State[+A]:
4+
def head: A
5+
def tail: LazyListIterable[A]^
6+
7+
private object State:
8+
object Empty extends State[Nothing]:
9+
def head: Nothing = throw new NoSuchElementException("head of empty lazy list")
10+
def tail: LazyListIterable[Nothing] = throw new UnsupportedOperationException("tail of empty lazy list")
11+
12+
final class Cons[A](val head: A, tl: LazyListIterable[A]^) extends State[A]:
13+
def tail: LazyListIterable[A]^ = tl // error
14+

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

+4-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
17 | def use(@consume x: F): File^ = x // error @consume override
1919
| ^
2020
| error overriding method use in trait Foo of type (x: File^): box File^;
21-
| method use of type (x: File^): File^ has incompatible type
21+
| method use of type (x: File^): File^² has incompatible type
22+
|
23+
| where: ^ refers to the universal root capability
24+
| ^² refers to a root capability associated with the result type of (x: File^): File^²
2225
|
2326
| longer explanation available when compiling with `-explain`

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

+4-1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,9 @@
2929
9 | val foo: IO^ -> IO^ = x => x // error // error
3030
| ^
3131
| error overriding value foo in trait Foo of type IO^ -> box IO^;
32-
| value foo of type IO^ -> IO^ has incompatible type
32+
| value foo of type IO^ -> IO^² has incompatible type
33+
|
34+
| where: ^ refers to the universal root capability
35+
| ^² refers to a fresh root capability in the type of value foo
3336
|
3437
| longer explanation available when compiling with `-explain`

‎tests/neg/abstract-givens.check

+4-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,10 @@
1010
-- [E164] Declaration Error: tests/neg/abstract-givens.scala:9:8 -------------------------------------------------------
1111
9 | given z[T](using T): Seq[T] = List(summon[T]) // error
1212
| ^
13-
| error overriding given instance z in trait T of type [T](using x$1: T): List[T];
13+
| error overriding given instance z in trait T² of type [T](using x$1: T): List[T];
1414
| given instance z of type [T](using x$1: T): Seq[T] has incompatible type
1515
|
16+
| where: T is a type variable
17+
| T² is a trait in the empty package
18+
|
1619
| longer explanation available when compiling with `-explain`

0 commit comments

Comments
 (0)