Skip to content

More improvements to error messages #23721

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 11 additions & 13 deletions compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1079,16 +1079,15 @@ class CheckCaptures extends Recheck, SymTransformer:
| Externally visible type: $expected""",
tree.srcPos)

def addenda(expected: Type) = new Addenda:
override def toAdd(using Context) =
def result = if tree.isInstanceOf[ValDef] then"" else " result"
i"""
|
|Note that the expected type $expected
|is the previously inferred$result type of $sym
|which is also the type seen in separately compiled sources.
|The new inferred type $tp
|must conform to this type.""" :: Nil
def addenda(expected: Type) = Addenda:
def result = if tree.isInstanceOf[ValDef] then"" else " result"
i"""
|
|Note that the expected type $expected
|is the previously inferred$result type of $sym
|which is also the type seen in separately compiled sources.
|The new inferred type $tp
|must conform to this type."""

tree.tpt match
case tpt: InferredTypeTree if !canUseInferred =>
Expand Down Expand Up @@ -1276,6 +1275,7 @@ class CheckCaptures extends Recheck, SymTransformer:
else new Addenda:
override def toAdd(using Context) = notes.map: note =>
i"""
|
|Note that ${note.description}."""

/** Addendas for error messages that show where we have under-approximated by
Expand All @@ -1291,13 +1291,11 @@ class CheckCaptures extends Recheck, SymTransformer:
(parent, " deep")
case _ =>
(original, "")*/
add ++ new Addenda:
override def toAdd(using Context): List[String] =
add ++ Addenda:
i"""
|
|Note that a capability $ref in a capture set appearing in contravariant position
|was mapped to $mapped which is not a capability. Therefore, it was under-approximated to the empty set."""
:: Nil
case _ =>
foldOver(add, t)

Expand Down
28 changes: 6 additions & 22 deletions compiler/src/dotty/tools/dotc/reporting/Message.scala
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,6 @@ object Message:
*/
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]

/** Register that `tp` was opened during printing. */
def openLambda(tp: LambdaType): Unit =
openedLambdas += tp

val seen = new collection.mutable.HashMap[SeenKey, List[Recorded]].withDefaultValue(Nil)

var nonSensical = false
Expand Down Expand Up @@ -109,16 +102,17 @@ object Message:
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.
* long as their corresponding binders have the same parameter name lists.
* This avoids spurious distinctions between parameters of mapped lambdas at
* the risk that sometimes we cannot distinguish parameters of distinct functions
* that have the same parameter names. 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)
(cur.paramName eq existing.paramName)
&& cur.binder.paramNames == existing.binder.paramNames
case _ =>
false

Expand Down Expand Up @@ -266,20 +260,10 @@ object Message:
case _ =>
super.funMiddleText(isContextual, isPure, refs)

override def toTextMethodAsFunction(info: Type, isPure: Boolean, refs: GeneralCaptureSet | Null): Text =
info match
case info: LambdaType =>
seen.openLambda(info)
case _ =>
super.toTextMethodAsFunction(info, isPure, refs)

override def toText(tp: Type): Text =
if !tp.exists || tp.isErroneous then seen.nonSensical = true
tp match
case tp: TypeRef if useSourceModule(tp.symbol) => Str("object ") ~ super.toText(tp)
case tp: LambdaType =>
seen.openLambda(tp)
super.toText(tp)
case _ => super.toText(tp)

override def toText(sym: Symbol): Text =
Expand Down
10 changes: 5 additions & 5 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import ast.Trees
import config.{Feature, MigrationVersion, ScalaVersion}
import transform.patmat.Space
import transform.patmat.SpaceEngine
import typer.ErrorReporting.{err, matchReductionAddendum, substitutableTypeSymbolsInScope}
import typer.ErrorReporting.{err, matchReductionAddendum, substitutableTypeSymbolsInScope, Addenda, NothingToAdd}
import typer.ProtoTypes.{ViewProto, SelectionProto, FunProto}
import typer.Implicits.*
import typer.Inferencing
Expand Down Expand Up @@ -297,7 +297,7 @@ extends NotFoundMsg(MissingIdentID) {
}
}

class TypeMismatch(val found: Type, expected: Type, val inTree: Option[untpd.Tree], addenda: => String*)(using Context)
class TypeMismatch(val found: Type, expected: Type, val inTree: Option[untpd.Tree], addenda: Addenda = NothingToAdd)(using Context)
extends TypeMismatchMsg(found, expected)(TypeMismatchID):

def msg(using Context) =
Expand Down Expand Up @@ -349,12 +349,12 @@ class TypeMismatch(val found: Type, expected: Type, val inTree: Option[untpd.Tre
|Required: $expectedStr${reported.notes}"""
end msg

override def msgPostscript(using Context) =
override def msgPostscript(using Context): String =
def importSuggestions =
if expected.isTopType || found.isBottomType then ""
else ctx.typer.importSuggestionAddendum(ViewProto(found.widen, expected))
super.msgPostscript
++ addenda.dropWhile(_.isEmpty).headOption.getOrElse(importSuggestions)

addenda.toAdd.mkString ++ super.msgPostscript ++ importSuggestions

override def explain(using Context) =
val treeStr = inTree.map(x => s"\nTree:\n\n${x.show}\n").getOrElse("")
Expand Down
21 changes: 13 additions & 8 deletions compiler/src/dotty/tools/dotc/typer/ErrorReporting.scala
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,16 @@ object ErrorReporting {

/** A mixin trait that can produce added elements for an error message */
trait Addenda:
self =>
def toAdd(using Context): List[String] = Nil
def ++ (follow: Addenda) = new Addenda:
override def toAdd(using Context) = self.toAdd ++ follow.toAdd
def toAdd(using Context): List[String]
def ++(follow: Addenda) = new Addenda:
def toAdd(using Context) = Addenda.this.toAdd ++ follow.toAdd

object Addenda:
def apply(msg: Context ?=> String): Addenda = new Addenda:
def toAdd(using Context) = msg :: Nil

object NothingToAdd extends Addenda
object NothingToAdd extends Addenda:
def toAdd(using Context): List[String] = Nil

class Errors(using Context) {

Expand Down Expand Up @@ -195,10 +199,11 @@ object ErrorReporting {

def missingElse = tree match
case If(_, _, elsep @ Literal(Constant(()))) if elsep.span.isSynthetic =>
"\nMaybe you are missing an else part for the conditional?"
case _ => ""
Addenda("\nMaybe you are missing an else part for the conditional?")
case _ =>
NothingToAdd

errorTree(tree, TypeMismatch(treeTp, expectedTp, Some(tree), (addenda.toAdd :+ missingElse)*))
errorTree(tree, TypeMismatch(treeTp, expectedTp, Some(tree), addenda ++ missingElse))
}

/** A subtype log explaining why `found` does not conform to `expected` */
Expand Down
2 changes: 2 additions & 0 deletions compiler/src/dotty/tools/dotc/typer/Implicits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,8 @@ object Implicits:
if (argument.isEmpty) i"match type ${clarify(expectedType)}"
else i"convert from ${argument.tpe} to ${clarify(expectedType)}"
}

def toAdd(using Context) = Nil
}

class NoMatchingImplicits(val expectedType: Type, val argument: Tree, constraint: Constraint = OrderingConstraint.empty)
Expand Down
2 changes: 2 additions & 0 deletions tests/neg-custom-args/captures/box-adapt-cases.check
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
| ^^^^^^^^^^^^^^^^
| Found: (cap: Cap^{io}) ->{io} Int
| Required: Cap^{io} -> Int
|
| Note that capability io is not included in capture set {}.
|
| longer explanation available when compiling with `-explain`
Expand All @@ -11,6 +12,7 @@
| ^^^^^^^^^^^^^^^^
| Found: (cap: Cap^'s1) ->{io, fs} Int
| Required: Cap^{io, fs} ->{io} Int
|
| Note that capability fs is not included in capture set {io}.
|
| longer explanation available when compiling with `-explain`
6 changes: 4 additions & 2 deletions tests/neg-custom-args/captures/byname.check
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,17 @@
|Found: () ?->{cap1} Int ->{cap1} Int
|Required: () ?=> Int ->{cap2} Int
|
|where: ?=> refers to a fresh root capability created in method test when checking argument to parameter ff of method h
|
|Note that capability cap1 is not included in capture set {cap2}.
|
|where: ?=> refers to a fresh root capability created in method test when checking argument to parameter ff of method h
|
| longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/byname.scala:19:5 ----------------------------------------
19 | h(g()) // error
| ^^^
| Found: () ?->{cap2} I^'s1
| Required: () ?->{cap1} I
|
| Note that capability cap2 is not included in capture set {cap1}.
|
| longer explanation available when compiling with `-explain`
Expand All @@ -28,6 +29,7 @@
| ^^^^^^^^^
| Found: () ->{cap2} I^'s2
| Required: () ->{cap1} I
|
| Note that capability cap2 is not included in capture set {cap1}.
|
| longer explanation available when compiling with `-explain`
4 changes: 2 additions & 2 deletions tests/neg-custom-args/captures/capt-depfun.check
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
| Found: Str^{} ->{ac, y, z} Str^{y, z}
| Required: Str^{y, z} => Str^{y, z}
|
| where: => refers to a fresh root capability in the type of value dc
|
| Note that capability y is not included in capture set {}.
|
| where: => refers to a fresh root capability in the type of value dc
|
| longer explanation available when compiling with `-explain`
25 changes: 17 additions & 8 deletions tests/neg-custom-args/captures/capt1.check
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| Found: () ->{x} C
| Required: () -> C
|
| Note that capability x is not included in capture set {}.
|
| longer explanation available when compiling with `-explain`
Expand All @@ -11,6 +12,7 @@
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| Found: () ->{x} C^'s1
| Required: Matchable
|
| Note that capability x is not included in capture set {}.
|
| longer explanation available when compiling with `-explain`
Expand All @@ -19,6 +21,7 @@
| ^
| Found: (y: Int) ->{x} Int
| Required: Matchable
|
| Note that capability x is not included in capture set {}.
16 | f
|
Expand All @@ -28,6 +31,7 @@
| ^
| Found: A^{x}
| Required: A
|
| Note that capability x is not included in capture set {}.
23 | def m() = if x == null then y else y
24 | F(22)
Expand All @@ -38,6 +42,7 @@
| ^
| Found: A^{x}
| Required: A
|
| Note that capability x is not included in capture set {}.
28 | def m() = if x == null then y else y
|
Expand All @@ -55,11 +60,13 @@
|Found: () ->'s2 C^
|Required: () -> C^²
|
|where: ^ refers to a root capability associated with the result type of (): C^
| ^² refers to a fresh root capability created in value z2 when checking argument to parameter a of method h
|Note that capability cap is not included in capture set {cap²}
|because cap is not visible from cap² in value z2.
|
|Note that capability <cap of (): C^> is not included in capture set {cap}
|because <cap of (): C^> is not visible from cap in value z2.
|where: ^ refers to a root capability associated with the result type of (): C^
| ^² refers to a fresh root capability created in value z2 when checking argument to parameter a of method h
| cap is a root capability associated with the result type of (): C^
| cap² is a fresh root capability created in value z2 when checking argument to parameter a of method h
|
| longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/capt1.scala:37:5 -----------------------------------------
Expand All @@ -68,11 +75,13 @@
|Found: () ->'s3 C^
|Required: () -> C^²
|
|where: ^ refers to a root capability associated with the result type of (): C^
| ^² refers to a fresh root capability created in value z2 when checking argument to parameter b of method h
|Note that capability cap is not included in capture set {cap²}
|because cap is not visible from cap² in value z2.
|
|Note that capability <cap of (): C^> is not included in capture set {cap}
|because <cap of (): C^> is not visible from cap in value z2.
|where: ^ refers to a root capability associated with the result type of (): C^
| ^² refers to a fresh root capability created in value z2 when checking argument to parameter b of method h
| cap is a root capability associated with the result type of (): C^
| cap² is a fresh root capability created in value z2 when checking argument to parameter b of method h
|
| longer explanation available when compiling with `-explain`
-- Error: tests/neg-custom-args/captures/capt1.scala:38:13 -------------------------------------------------------------
Expand Down
50 changes: 32 additions & 18 deletions tests/neg-custom-args/captures/cc-existential-conformance.check
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,31 @@
| Found: (x : A -> (x²: A) -> B^)
| Required: A -> A -> B^²
|
| where: ^ refers to a root capability associated with the result type of (x²: A): B^
| ^² refers to a fresh root capability in the type of value y
| x is a value in method test
| x² is a reference to a value parameter
| Note that capability cap is not included in capture set {cap²}
| because cap is not visible from cap² in value y.
|
| Note that capability <cap of (x: A): B^> is not included in capture set {cap}
| because <cap of (x: A): B^> is not visible from cap in value y.
| where: ^ refers to a root capability associated with the result type of (x²: A): B^
| ^² refers to a fresh root capability in the type of value y
| cap is a root capability associated with the result type of (x²: A): B^
| cap² is a fresh root capability in the type of value y
| x is a value in method test
| x² is a reference to a value parameter
|
| longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/cc-existential-conformance.scala:9:29 --------------------
9 | val z: A -> (x: A) -> B^ = y // error
| ^
| Found: A -> A -> B^{y*}
| Required: A -> (x: A) -> B^
| Found: A -> A -> B^{y*}
| Required: A -> (x: A) -> B^
|
| where: ^ refers to a root capability associated with the result type of (x: A): B^
| Note that capability y* is not included in capture set {cap}.
|
| Note that capability y* is not included in capture set {<cap of (x: A): B^>}.
| Note that the existential capture root in B^²
| cannot subsume the capability y* since that capability is not a `Sharable` capability..
|
| where: ^ refers to a root capability associated with the result type of (x: A): B^
| ^² refers to the universal root capability
| cap is a root capability associated with the result type of (x: A): B^
|
| longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/cc-existential-conformance.scala:13:19 -------------------
Expand All @@ -30,13 +37,15 @@
| Found: (x : (x²: A) -> B^)
| Required: A -> B^²
|
| where: ^ refers to a root capability associated with the result type of (x²: A): B^
| ^² refers to a fresh root capability in the type of value y
| x is a value in method test2
| x² is a reference to a value parameter
| Note that capability cap is not included in capture set {cap²}
| because cap is not visible from cap² in value y.
|
| Note that capability <cap of (x: A): B^> is not included in capture set {cap}
| because <cap of (x: A): B^> is not visible from cap in value y.
| where: ^ refers to a root capability associated with the result type of (x²: A): B^
| ^² refers to a fresh root capability in the type of value y
| cap is a root capability associated with the result type of (x²: A): B^
| cap² is a fresh root capability in the type of value y
| x is a value in method test2
| x² is a reference to a value parameter
|
| longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/cc-existential-conformance.scala:14:24 -------------------
Expand All @@ -45,8 +54,13 @@
| Found: A -> B^{y*}
| Required: (x: A) -> B^
|
| where: ^ refers to a root capability associated with the result type of (x: A): B^
| Note that capability y* is not included in capture set {cap}.
|
| Note that the existential capture root in B^²
| cannot subsume the capability y* since that capability is not a `Sharable` capability..
|
| Note that capability y* is not included in capture set {<cap of (x: A): B^>}.
| where: ^ refers to a root capability associated with the result type of (x: A): B^
| ^² refers to the universal root capability
| cap is a root capability associated with the result type of (x: A): B^
|
| longer explanation available when compiling with `-explain`
Loading
Loading