From 03681c50c19cc3d24a65bda177f8178eacc2939e Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Tue, 26 Mar 2024 15:10:11 +0100 Subject: [PATCH 1/4] Add note about type mismatch in automatically inserted apply argument Co-Authored-By: Jan-Pieter van den Heuvel <1197006+jan-pieter@users.noreply.github.com> Co-Authored-By: Lucas Nouguier [Cherry-picked 2c70f3dbab44c492beef3dec770aba0c76e7beb6] --- .../dotc/reporting/ExploringReporter.scala | 5 ++- .../dotty/tools/dotc/reporting/Reporter.scala | 3 ++ .../tools/dotc/reporting/StoreReporter.scala | 5 ++- .../dotty/tools/dotc/reporting/messages.scala | 2 +- .../dotty/tools/dotc/typer/Applications.scala | 32 ++++++++++++++++++- tests/neg/19680.check | 24 ++++++++++++++ tests/neg/19680.scala | 9 ++++++ tests/neg/19680b.check | 25 +++++++++++++++ tests/neg/19680b.scala | 2 ++ 9 files changed, 103 insertions(+), 4 deletions(-) create mode 100644 tests/neg/19680.check create mode 100644 tests/neg/19680.scala create mode 100644 tests/neg/19680b.check create mode 100644 tests/neg/19680b.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/ExploringReporter.scala b/compiler/src/dotty/tools/dotc/reporting/ExploringReporter.scala index f469c03764c0..99720b8e4d29 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ExploringReporter.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ExploringReporter.scala @@ -18,6 +18,9 @@ class ExploringReporter extends StoreReporter(null, fromTyperState = false): override def removeBufferedMessages(using Context): List[Diagnostic] = try infos.toList finally reset() + override def mapBufferedMessages(f: Diagnostic => Diagnostic)(using Context): Unit = + infos.mapInPlace(f) + def reset(): Unit = infos.clear() -end ExploringReporter \ No newline at end of file +end ExploringReporter diff --git a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala index 9ab1a0029277..eb9756028c27 100644 --- a/compiler/src/dotty/tools/dotc/reporting/Reporter.scala +++ b/compiler/src/dotty/tools/dotc/reporting/Reporter.scala @@ -270,6 +270,9 @@ abstract class Reporter extends interfaces.ReporterResult { /** If this reporter buffers messages, remove and return all buffered messages. */ def removeBufferedMessages(using Context): List[Diagnostic] = Nil + /** If this reporter buffers messages, apply `f` to all buffered messages. */ + def mapBufferedMessages(f: Diagnostic => Diagnostic)(using Context): Unit = () + /** Issue all messages in this reporter to next outer one, or make sure they are written. */ def flush()(using Context): Unit = val msgs = removeBufferedMessages diff --git a/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala b/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala index aef5f2c5863b..9395788d4cc7 100644 --- a/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala +++ b/compiler/src/dotty/tools/dotc/reporting/StoreReporter.scala @@ -21,7 +21,7 @@ class StoreReporter(outer: Reporter | Null = Reporter.NoReporter, fromTyperState protected var infos: mutable.ListBuffer[Diagnostic] | Null = null - def doReport(dia: Diagnostic)(using Context): Unit = { + override def doReport(dia: Diagnostic)(using Context): Unit = { typr.println(s">>>> StoredError: ${dia.message}") // !!! DEBUG if (infos == null) infos = new mutable.ListBuffer infos.uncheckedNN += dia @@ -37,6 +37,9 @@ class StoreReporter(outer: Reporter | Null = Reporter.NoReporter, fromTyperState if (infos != null) try infos.uncheckedNN.toList finally infos = null else Nil + override def mapBufferedMessages(f: Diagnostic => Diagnostic)(using Context): Unit = + if infos != null then infos.uncheckedNN.mapInPlace(f) + override def pendingMessages(using Context): List[Diagnostic] = if (infos != null) infos.uncheckedNN.toList else Nil diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index c8e6fe21409e..f891cec1df67 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -287,7 +287,7 @@ extends NotFoundMsg(MissingIdentID) { } } -class TypeMismatch(val found: Type, expected: Type, inTree: Option[untpd.Tree], addenda: => String*)(using Context) +class TypeMismatch(val found: Type, expected: Type, val inTree: Option[untpd.Tree], addenda: => String*)(using Context) extends TypeMismatchMsg(found, expected)(TypeMismatchID): def msg(using Context) = diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index 75c43b8e8fb6..ce0ec4dd8913 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1080,7 +1080,37 @@ trait Applications extends Compatibility { simpleApply(fun1, proto) } { (failedVal, failedState) => - def fail = { failedState.commit(); failedVal } + def fail = + insertedApplyNote() + failedState.commit() + failedVal + + /** If the applied function is an automatically inserted `apply` + * method and one of its arguments has a type mismatch , append + * a note to the error message that explains where the required + * type comes from. See #19680 and associated test case. + */ + def insertedApplyNote() = + if fun1.symbol.name == nme.apply && fun1.span.isSynthetic then + fun1 match + case Select(qualifier, _) => + failedState.reporter.mapBufferedMessages: + case dia: Diagnostic.Error => + dia.msg match + case msg: TypeMismatch => + msg.inTree match + case Some(arg) if tree.args.exists(_.span == arg.span) => + val Select(qualifier, _) = fun1: @unchecked + val noteText = + i"""The required type comes from a parameter of the automatically + |inserted `apply` method of `${qualifier.tpe}`, + |which is the type of `${qualifier.show}`.""".stripMargin + Diagnostic.Error(msg.appendExplanation("\n\n" + noteText), dia.pos) + case _ => dia + case msg => dia + case dia => dia + case _ => () + // Try once with original prototype and once (if different) with tupled one. // The reason we need to try both is that the decision whether to use tupled // or not was already taken but might have to be revised when an implicit diff --git a/tests/neg/19680.check b/tests/neg/19680.check new file mode 100644 index 000000000000..8372d5129960 --- /dev/null +++ b/tests/neg/19680.check @@ -0,0 +1,24 @@ +-- [E007] Type Mismatch Error: tests/neg/19680.scala:9:67 -------------------------------------------------------------- +9 |def renderWidget(using Config): Unit = renderWebsite("/tmp")(Config()) // error: found Config, required Int + | ^^^^^^^^ + | Found: Config + | Required: Int + |--------------------------------------------------------------------------------------------------------------------- + | Explanation (enabled by `-explain`) + |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + | + | Tree: new Config() + | I tried to show that + | Config + | conforms to + | Int + | but none of the attempts shown below succeeded: + | + | ==> Config <: Int = false + | + | The tests were made under the empty constraint + | + | The required type comes from a parameter of the automatically + | inserted `apply` method of `scala.collection.StringOps`, + | which is the type of `augmentString(renderWebsite("/tmp")(x$1))`. + --------------------------------------------------------------------------------------------------------------------- diff --git a/tests/neg/19680.scala b/tests/neg/19680.scala new file mode 100644 index 000000000000..57fdd851dc54 --- /dev/null +++ b/tests/neg/19680.scala @@ -0,0 +1,9 @@ +//> using options -explain + +// Tests that the error message indicates that the required type `Int` comes +// from the automatically inserted `apply` method of `String`. This note is +// inserted by `insertedApplyNote` in `Applications`. + +class Config() +def renderWebsite(path: String)(using config: Config): String = ??? +def renderWidget(using Config): Unit = renderWebsite("/tmp")(Config()) // error: found Config, required Int diff --git a/tests/neg/19680b.check b/tests/neg/19680b.check new file mode 100644 index 000000000000..14f2a30c5caa --- /dev/null +++ b/tests/neg/19680b.check @@ -0,0 +1,25 @@ +-- [E007] Type Mismatch Error: tests/neg/19680b.scala:2:21 ------------------------------------------------------------- +2 |def Test = List(1,2)("hello") // error: found String, required Int + | ^^^^^^^ + | Found: ("hello" : String) + | Required: Int + |--------------------------------------------------------------------------------------------------------------------- + | Explanation (enabled by `-explain`) + |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + | + | Tree: "hello" + | I tried to show that + | ("hello" : String) + | conforms to + | Int + | but none of the attempts shown below succeeded: + | + | ==> ("hello" : String) <: Int + | ==> String <: Int = false + | + | The tests were made under the empty constraint + | + | The required type comes from a parameter of the automatically + | inserted `apply` method of `List[Int]`, + | which is the type of `List.apply[Int]([1,2 : Int]*)`. + --------------------------------------------------------------------------------------------------------------------- diff --git a/tests/neg/19680b.scala b/tests/neg/19680b.scala new file mode 100644 index 000000000000..a089d23e6a32 --- /dev/null +++ b/tests/neg/19680b.scala @@ -0,0 +1,2 @@ +//> using options -explain +def Test = List(1,2)("hello") // error: found String, required Int From 561b46e220f978c730f662666523926ba8bb357d Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Sun, 28 Apr 2024 18:10:23 +0200 Subject: [PATCH 2/4] Remove left-over line [Cherry-picked dd527fc120aa6e7819b40869f7537a5c88aa657c] --- compiler/src/dotty/tools/dotc/typer/Applications.scala | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index ce0ec4dd8913..dbd2644d9a5e 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1100,7 +1100,6 @@ trait Applications extends Compatibility { case msg: TypeMismatch => msg.inTree match case Some(arg) if tree.args.exists(_.span == arg.span) => - val Select(qualifier, _) = fun1: @unchecked val noteText = i"""The required type comes from a parameter of the automatically |inserted `apply` method of `${qualifier.tpe}`, From 8fa10ee9af53e9a1859210b5f36cea86c5418bed Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Mon, 29 Apr 2024 11:21:00 +0200 Subject: [PATCH 3/4] Move maybeAddInsertedApplyNote to outer scope, add explicit types [Cherry-picked 1f06af15885e7fabb5a6dc801a7c946622b4cab8] --- .../dotty/tools/dotc/typer/Applications.scala | 55 ++++++++++--------- 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index dbd2644d9a5e..e7b3485ea2fd 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1020,6 +1020,34 @@ trait Applications extends Compatibility { } } + /** If the applied function is an automatically inserted `apply` + * method and one of its arguments has a type mismatch , append + * a note to the error message that explains where the required + * type comes from. See #19680 and associated test case. + */ + def maybeAddInsertedApplyNote(failedState: TyperState, fun1: Tree)(using Context): Unit = + if fun1.symbol.name == nme.apply && fun1.span.isSynthetic then + fun1 match + case Select(qualifier, _) => + def mapMessage(dia: Diagnostic): Diagnostic = + dia match + case dia: Diagnostic.Error => + dia.msg match + case msg: TypeMismatch => + msg.inTree match + case Some(arg) if tree.args.exists(_.span == arg.span) => + val noteText = + i"""The required type comes from a parameter of the automatically + |inserted `apply` method of `${qualifier.tpe}`, + |which is the type of `${qualifier.show}`.""".stripMargin + Diagnostic.Error(msg.appendExplanation("\n\n" + noteText), dia.pos) + case _ => dia + case msg => dia + case dia => dia + failedState.reporter.mapBufferedMessages(mapMessage) + case _ => () + else () + fun1.tpe match { case err: ErrorType => cpy.Apply(tree)(fun1, proto.typedArgs()).withType(err) case TryDynamicCallType => @@ -1081,35 +1109,10 @@ trait Applications extends Compatibility { } { (failedVal, failedState) => def fail = - insertedApplyNote() + maybeAddInsertedApplyNote(failedState, fun1) failedState.commit() failedVal - /** If the applied function is an automatically inserted `apply` - * method and one of its arguments has a type mismatch , append - * a note to the error message that explains where the required - * type comes from. See #19680 and associated test case. - */ - def insertedApplyNote() = - if fun1.symbol.name == nme.apply && fun1.span.isSynthetic then - fun1 match - case Select(qualifier, _) => - failedState.reporter.mapBufferedMessages: - case dia: Diagnostic.Error => - dia.msg match - case msg: TypeMismatch => - msg.inTree match - case Some(arg) if tree.args.exists(_.span == arg.span) => - val noteText = - i"""The required type comes from a parameter of the automatically - |inserted `apply` method of `${qualifier.tpe}`, - |which is the type of `${qualifier.show}`.""".stripMargin - Diagnostic.Error(msg.appendExplanation("\n\n" + noteText), dia.pos) - case _ => dia - case msg => dia - case dia => dia - case _ => () - // Try once with original prototype and once (if different) with tupled one. // The reason we need to try both is that the decision whether to use tupled // or not was already taken but might have to be revised when an implicit From 49a97d7cc3667c253efa2ea2b3c679ee9abc48e1 Mon Sep 17 00:00:00 2001 From: Matt Bovel Date: Tue, 30 Apr 2024 19:14:17 +0200 Subject: [PATCH 4/4] Remove message last line [Cherry-picked 748596a894aca7c8ecca47499ac738b448280054] --- compiler/src/dotty/tools/dotc/typer/Applications.scala | 3 +-- tests/neg/19680.check | 3 +-- tests/neg/19680b.check | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/typer/Applications.scala b/compiler/src/dotty/tools/dotc/typer/Applications.scala index e7b3485ea2fd..2f0186f0fe4b 100644 --- a/compiler/src/dotty/tools/dotc/typer/Applications.scala +++ b/compiler/src/dotty/tools/dotc/typer/Applications.scala @@ -1038,8 +1038,7 @@ trait Applications extends Compatibility { case Some(arg) if tree.args.exists(_.span == arg.span) => val noteText = i"""The required type comes from a parameter of the automatically - |inserted `apply` method of `${qualifier.tpe}`, - |which is the type of `${qualifier.show}`.""".stripMargin + |inserted `apply` method of `${qualifier.tpe}`.""".stripMargin Diagnostic.Error(msg.appendExplanation("\n\n" + noteText), dia.pos) case _ => dia case msg => dia diff --git a/tests/neg/19680.check b/tests/neg/19680.check index 8372d5129960..5bdaaad99c2a 100644 --- a/tests/neg/19680.check +++ b/tests/neg/19680.check @@ -19,6 +19,5 @@ | The tests were made under the empty constraint | | The required type comes from a parameter of the automatically - | inserted `apply` method of `scala.collection.StringOps`, - | which is the type of `augmentString(renderWebsite("/tmp")(x$1))`. + | inserted `apply` method of `scala.collection.StringOps`. --------------------------------------------------------------------------------------------------------------------- diff --git a/tests/neg/19680b.check b/tests/neg/19680b.check index 14f2a30c5caa..06ff26ee3289 100644 --- a/tests/neg/19680b.check +++ b/tests/neg/19680b.check @@ -20,6 +20,5 @@ | The tests were made under the empty constraint | | The required type comes from a parameter of the automatically - | inserted `apply` method of `List[Int]`, - | which is the type of `List.apply[Int]([1,2 : Int]*)`. + | inserted `apply` method of `List[Int]`. ---------------------------------------------------------------------------------------------------------------------