From be1b0fb0acd753b8722976c5fd35c9d6f71e1194 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Wed, 27 Jun 2018 20:42:14 +0200 Subject: [PATCH 1/9] Refine type of ValOrDefDef.name --- compiler/src/dotty/tools/dotc/ast/Trees.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/src/dotty/tools/dotc/ast/Trees.scala b/compiler/src/dotty/tools/dotc/ast/Trees.scala index 8c23a4a26c42..c2ce0f27a308 100644 --- a/compiler/src/dotty/tools/dotc/ast/Trees.scala +++ b/compiler/src/dotty/tools/dotc/ast/Trees.scala @@ -373,6 +373,7 @@ object Trees { /** A ValDef or DefDef tree */ trait ValOrDefDef[-T >: Untyped] extends MemberDef[T] with WithLazyField[Tree[T]] { + def name: TermName def tpt: Tree[T] def unforcedRhs: LazyTree = unforced def rhs(implicit ctx: Context): Tree[T] = forceIfLazy From 0bd5dbe00f0604fe56da5f10adb95e6e1548c962 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Thu, 5 Jul 2018 19:47:24 +0200 Subject: [PATCH 2/9] Fix unrelated typo --- .../test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala index 50cf7f0860df..9ead8ee63e70 100644 --- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala +++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala @@ -334,7 +334,7 @@ class ErrorMessagesTests extends ErrorMessagesTest { assertEquals(namedImport, prevPrec) } - @Test def methodDoesNotTakePrameters = + @Test def methodDoesNotTakeParameters = checkMessagesAfter(FrontEnd.name) { """ |object Scope { @@ -353,7 +353,7 @@ class ErrorMessagesTests extends ErrorMessagesTest { assertEquals("method foo", msg.methodSymbol.show) } - @Test def methodDoesNotTakeMorePrameters = + @Test def methodDoesNotTakeMoreParameters = checkMessagesAfter(FrontEnd.name) { """ |object Scope{ From feef97e0cf25c6137bd65bfb5f53a62be64ce401 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Thu, 5 Jul 2018 21:36:10 +0200 Subject: [PATCH 3/9] Make error explanation more correct --- .../src/dotty/tools/dotc/reporting/diagnostic/messages.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 72d0284ff51b..578525508855 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1270,7 +1270,8 @@ object messages { |it calls another, you need to specify the calling method's return type. | |Case 2: ${method} is recursive - |If `${method}` calls itself on any path, you need to specify its return type. + |If `${method}` calls itself on any path (even through mutual recursion), you need to specify the return type + |of `${method}` or of a definition it's mutually recursive with. |""".stripMargin } From 68c5d5731f1b0f8923306ce13fc4f9327bb5aac6 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Wed, 27 Jun 2018 13:33:15 +0200 Subject: [PATCH 4/9] CyclicReferenceInvolving: improve explanation for consistency Tentative advice copied from `CyclicReferenceInvolvingImplicit`. --- .../src/dotty/tools/dotc/reporting/diagnostic/messages.scala | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 578525508855..287079e2d2b1 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1291,6 +1291,7 @@ object messages { val explanation = hl"""|$denot is declared as part of a cycle which makes it impossible for the |compiler to decide upon ${denot.name}'s type. + |To avoid this error, try giving `${denot.name}` an explicit type. |""".stripMargin } From 08977bb3ce1676e46a4bbc0e462f04a23f9f1e45 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Wed, 27 Jun 2018 11:24:27 +0200 Subject: [PATCH 5/9] Fix #4709 and #3253: report CyclicError in implicit search better - Record in CyclicReference if it's emitted during implicit search. - Before checking if the error is on an implicit, check if we have an error during implicit search that happens while inferring a return type; that can happen whether the member we're typechecking is implicit or not, and needs the same cure. - Add ErrorMessagesTests as requested This improves the error message, and also puts more effort into diagnosing the cycle: we now use similar logic as for recursive/overloaded members to check we're indeed dealing with a (potentially) recursive implicit. Remaining `CyclicReferenceInvolvingImplicit` are unexpected and might involve bugs, so make the error message and explanation more tentative, following `CyclicReferenceInvolving`. *However*, this does not yet handle mutually recursive methods in a reasonable way. --- .../dotty/tools/dotc/core/TypeErrors.scala | 32 ++++-- .../reporting/diagnostic/ErrorMessageID.java | 1 + .../dotc/reporting/diagnostic/messages.scala | 19 +++- .../dotty/tools/dotc/typer/Implicits.scala | 10 +- .../dotc/reporting/ErrorMessagesTests.scala | 105 +++++++++++++++++- tests/neg/i2001a.scala | 13 +++ tests/neg/i3253.scala | 5 + tests/neg/i4709.scala | 11 ++ 8 files changed, 182 insertions(+), 14 deletions(-) create mode 100644 tests/neg/i2001a.scala create mode 100644 tests/neg/i3253.scala create mode 100644 tests/neg/i4709.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala index 9218dd8a4267..7920d0e14a62 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala @@ -98,13 +98,28 @@ object handleRecursive { } } +/** + * This TypeError signals that completing denot encountered a cycle: it asked for denot.info (or similar), + * so it requires knowing denot already. + * @param denot + */ class CyclicReference private (val denot: SymDenotation) extends TypeError { + var inImplicitSearch: Boolean = false - override def toMessage(implicit ctx: Context) = { + override def toMessage(implicit ctx: Context): Message = { + val cycleSym = denot.symbol - def errorMsg(cx: Context): Message = + /* This CyclicReference might have arisen from asking for `m`'s type while trying to infer it. + * To try to diagnose this, walk the context chain searching for context in + * Mode.InferringReturnType for the innermost member without type + * annotations (!tree.tpt.typeOpt.exists). + */ + def errorMsg(cx: Context): Message = { if (cx.mode is Mode.InferringReturnType) { cx.tree match { + case tree: untpd.ValOrDefDef if inImplicitSearch && !tree.tpt.typeOpt.exists => + // Can happen in implicit defs (#4709) or outside (#3253). + TermMemberNeedsResultTypeForImplicitSearch(cycleSym) case tree: untpd.DefDef if !tree.tpt.typeOpt.exists => OverloadedOrRecursiveMethodNeedsResultType(tree.name) case tree: untpd.ValDef if !tree.tpt.typeOpt.exists => @@ -113,13 +128,14 @@ class CyclicReference private (val denot: SymDenotation) extends TypeError { errorMsg(cx.outer) } } - else CyclicReferenceInvolving(denot) + // Give up and give generic errors. + else if (cycleSym.is(Implicit, butNot = Method) && cycleSym.owner.isTerm) + CyclicReferenceInvolvingImplicit(cycleSym) + else + CyclicReferenceInvolving(denot) + } - val cycleSym = denot.symbol - if (cycleSym.is(Implicit, butNot = Method) && cycleSym.owner.isTerm) - CyclicReferenceInvolvingImplicit(cycleSym) - else - errorMsg(ctx) + errorMsg(ctx) } } diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java index 8233e3305d2a..c7cacfed991d 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java @@ -131,6 +131,7 @@ public enum ErrorMessageID { MatchCaseOnlyNullWarningID, ImportRenamedTwiceID, TypeTestAlwaysSucceedsID, + TermMemberNeedsNeedsResultTypeForImplicitSearchID ; public int errorNumber() { diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 287079e2d2b1..773509f97f8d 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1260,7 +1260,7 @@ object messages { |""".stripMargin } - case class OverloadedOrRecursiveMethodNeedsResultType(method: Names.TermName)(implicit ctx: Context) + case class OverloadedOrRecursiveMethodNeedsResultType(method: Names.Name)(implicit ctx: Context) extends Message(OverloadedOrRecursiveMethodNeedsResultTypeID) { val kind = "Syntax" val msg = hl"""overloaded or recursive method ${method} needs return type""" @@ -1300,8 +1300,10 @@ object messages { val kind = "Syntax" val msg = hl"""cyclic reference involving implicit $cycleSym""" val explanation = - hl"""|This happens when the right hand-side of $cycleSym's definition involves an implicit search. - |To avoid this error, give `${cycleSym.name}` an explicit type. + hl"""|$cycleSym is declared as part of a cycle which makes it impossible for the + |compiler to decide upon ${cycleSym.name}'s type. + |This might happen when the right hand-side of $cycleSym's definition involves an implicit search. + |To avoid this error, try giving `${cycleSym.name}` an explicit type. |""".stripMargin } @@ -2107,4 +2109,15 @@ object messages { } val explanation = "" } + + // Relative of CyclicReferenceInvolvingImplicit and RecursiveValueNeedsResultType + case class TermMemberNeedsResultTypeForImplicitSearch(cycleSym: Symbol)(implicit ctx: Context) + extends Message(TermMemberNeedsNeedsResultTypeForImplicitSearchID) { + val kind = "Syntax" + val msg = hl"""$cycleSym needs result type because its right-hand side attempts implicit search""" + val explanation = + hl"""|The right hand-side of $cycleSym's definition requires an implicit search at the highlighted position. + |To avoid this error, give `${cycleSym.name}` an explicit type. + |""".stripMargin + } } diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 3598db12ad1a..7d8b5e6d66f5 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -847,7 +847,15 @@ trait Implicits { self: Typer => else i"type error: ${argument.tpe} does not conform to $pt${err.whyNoMatchStr(argument.tpe, pt)}") trace(s"search implicit ${pt.show}, arg = ${argument.show}: ${argument.tpe.show}", implicits, show = true) { assert(!pt.isInstanceOf[ExprType]) - val result = new ImplicitSearch(pt, argument, pos).bestImplicit(contextual = true) + val result = + try { + new ImplicitSearch(pt, argument, pos).bestImplicit(contextual = true) + } catch { + case ce: CyclicReference => + ce.inImplicitSearch = true + throw ce + } + result match { case result: SearchSuccess => result.tstate.commit() diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala index 9ead8ee63e70..e703d063392e 100644 --- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala +++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala @@ -260,7 +260,65 @@ class ErrorMessagesTests extends ErrorMessagesTest { assertEquals("value x", denot.show) } - @Test def cyclicReferenceInvolvingImplicit = + @Test def cyclicReferenceInvolving2 = + checkMessagesAfter(FrontEnd.name) { + """ + |class A { + | implicit val x: T = ??? + | type T <: x.type // error: cyclic reference involving value x + |} + """.stripMargin + } + .expect { (ictx, messages) => + implicit val ctx: Context = ictx + + assertMessageCount(1, messages) + val CyclicReferenceInvolving(denot) :: Nil = messages + assertEquals("value x", denot.show) + } + + @Test def mutualRecursionre_i2001 = + checkMessagesAfter(FrontEnd.name) { + """ + |class A { + | def odd(x: Int) = if (x == 0) false else !even(x-1) + | def even(x: Int) = if (x == 0) true else !odd(x-1) // error: overloaded or recursive method needs result type + |} + """.stripMargin + } + .expect { (ictx, messages) => + implicit val ctx: Context = ictx + + assertMessageCount(1, messages) + val OverloadedOrRecursiveMethodNeedsResultType(name) :: Nil = messages + assertEquals("even", name.show) + } + + @Test def mutualRecursion_i2001a = + checkMessagesAfter(FrontEnd.name) { + """ + |class A { + | def odd(x: Int) = if (x == 0) false else !even(x-1) + | def even(x: Int) = { + | def foo = { + | if (x == 0) true else !odd(x-1) // error: overloaded or recursive method needs result type + | } + | false + | } + |} + """.stripMargin + } + .expect { (ictx, messages) => + implicit val ctx: Context = ictx + + assertMessageCount(1, messages) + val OverloadedOrRecursiveMethodNeedsResultType(denot) :: Nil = messages + // Not ideal behavior + assertEquals("foo", denot.show) + } + + + @Test def termMemberNeedsNeedsResultTypeForImplicitSearch = checkMessagesAfter(FrontEnd.name) { """ |object implicitDefs { @@ -276,10 +334,53 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val CyclicReferenceInvolvingImplicit(tree) :: Nil = messages + val TermMemberNeedsResultTypeForImplicitSearch(tree) :: Nil = messages assertEquals("x", tree.name.show) } + @Test def implicitSearchForcesImplicitRetType_i4709 = + checkMessagesAfter(FrontEnd.name) { + """ + |import scala.language.implicitConversions + | + |class Context + |class ContextBase { def settings = 1 } + | + |class Test { + | implicit def toBase(ctx: Context): ContextBase = ??? + | + | def test(ctx0: Context) = { + | implicit val ctx = { ctx0.settings; ??? } + | } + |} + """.stripMargin + } + .expect{ (ictx, messages) => + implicit val ctx: Context = ictx + + assertMessageCount(1, messages) + val TermMemberNeedsResultTypeForImplicitSearch(tree) :: Nil = messages + assertEquals("ctx", tree.name.show) + } + + @Test def implicitSearchForcesNonImplicitRetTypeOnExplicitImport_i3253 = + checkMessagesAfter(FrontEnd.name) { + """ + |import Test.test + | + |object Test { + | def test = " " * 10 + |} + """.stripMargin + } + .expect{ (ictx, messages) => + implicit val ctx: Context = ictx + + assertMessageCount(1, messages) + val TermMemberNeedsResultTypeForImplicitSearch(tree) :: Nil = messages + assertEquals("test", tree.name.show) + } + @Test def superQualMustBeParent = checkMessagesAfter(FrontEnd.name) { """ diff --git a/tests/neg/i2001a.scala b/tests/neg/i2001a.scala new file mode 100644 index 000000000000..5879a8072b2a --- /dev/null +++ b/tests/neg/i2001a.scala @@ -0,0 +1,13 @@ +class A { + def odd(x: Int) = if (x == 0) false else !even(x-1) + def even(x: Int) = { + def foo = { + if (x == 0) true else !odd(x-1) // error: overloaded or recursive method needs result type + } + foo + } + lazy val x = { + def foo = x // error + foo + } +} diff --git a/tests/neg/i3253.scala b/tests/neg/i3253.scala new file mode 100644 index 000000000000..51acaad92706 --- /dev/null +++ b/tests/neg/i3253.scala @@ -0,0 +1,5 @@ +import Test.test + +object Test { + def test = " " * 10 // error +} diff --git a/tests/neg/i4709.scala b/tests/neg/i4709.scala new file mode 100644 index 000000000000..5122f06c9f29 --- /dev/null +++ b/tests/neg/i4709.scala @@ -0,0 +1,11 @@ +class Context +class ContextBase { def settings = 1 } + +class Test { + implicit def toBase(ctx: Context): ContextBase = ??? + + def test(ctx0: Context) = { + implicit val ctx = { ctx0.settings; ??? } // error + } + def f: Unit = { implicitly[Int]; implicit val i = implicitly[Int] } // error +} From b4a8d45d30223cd9fb1b66bee42c34ac903870e1 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Wed, 18 Jul 2018 12:52:00 +0200 Subject: [PATCH 6/9] Analyzed CyclicErrors: report the correct symbol Make reports about cycleSym not tree.name. We need then to use flagsUNSAFE, see description. * Store cycleSym in all relevant errors * Mention cycleSym instead of tree.name in errors Tests pass, and have the correct expectations both for `tree.name` and `cycleSym.name`, tho we just show `cycleSym.name` and `tree.name` is less accurate. But at this point, I might be able to stop using `tree.name` altogether. --- .../dotty/tools/dotc/core/TypeErrors.scala | 22 +++-- .../dotc/reporting/diagnostic/messages.scala | 24 +++--- .../dotc/reporting/ErrorMessagesTests.scala | 83 +++++++++++++++---- tests/neg/i2001b.scala | 9 ++ 4 files changed, 102 insertions(+), 36 deletions(-) create mode 100644 tests/neg/i2001b.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala index 7920d0e14a62..83ce3422477e 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala @@ -109,6 +109,12 @@ class CyclicReference private (val denot: SymDenotation) extends TypeError { override def toMessage(implicit ctx: Context): Message = { val cycleSym = denot.symbol + // cycleSym.flags would try completing denot and would fail, but here we can use flagsUNSAFE to detect flags + // set by the parser. + val unsafeFlags = cycleSym.flagsUNSAFE + val isMethod = unsafeFlags.is(Method) + val isVal = !isMethod && cycleSym.isTerm + /* This CyclicReference might have arisen from asking for `m`'s type while trying to infer it. * To try to diagnose this, walk the context chain searching for context in * Mode.InferringReturnType for the innermost member without type @@ -117,13 +123,15 @@ class CyclicReference private (val denot: SymDenotation) extends TypeError { def errorMsg(cx: Context): Message = { if (cx.mode is Mode.InferringReturnType) { cx.tree match { - case tree: untpd.ValOrDefDef if inImplicitSearch && !tree.tpt.typeOpt.exists => - // Can happen in implicit defs (#4709) or outside (#3253). - TermMemberNeedsResultTypeForImplicitSearch(cycleSym) - case tree: untpd.DefDef if !tree.tpt.typeOpt.exists => - OverloadedOrRecursiveMethodNeedsResultType(tree.name) - case tree: untpd.ValDef if !tree.tpt.typeOpt.exists => - RecursiveValueNeedsResultType(tree.name) + case tree: untpd.ValOrDefDef if !tree.tpt.typeOpt.exists => + if (inImplicitSearch) + TermMemberNeedsResultTypeForImplicitSearch(tree.name, cycleSym) + else if (isMethod) + OverloadedOrRecursiveMethodNeedsResultType(tree.name, cycleSym) + else if (isVal) + RecursiveValueNeedsResultType(tree.name, cycleSym) + else + errorMsg(cx.outer) case _ => errorMsg(cx.outer) } diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 773509f97f8d..13fffd701390 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1260,27 +1260,27 @@ object messages { |""".stripMargin } - case class OverloadedOrRecursiveMethodNeedsResultType(method: Names.Name)(implicit ctx: Context) + case class OverloadedOrRecursiveMethodNeedsResultType(methodName: Names.TermName, cycleSym: Symbol)(implicit ctx: Context) extends Message(OverloadedOrRecursiveMethodNeedsResultTypeID) { val kind = "Syntax" - val msg = hl"""overloaded or recursive method ${method} needs return type""" + val msg = hl"""overloaded or recursive ${cycleSym} needs return type""" val explanation = - hl"""Case 1: ${method} is overloaded - |If there are multiple methods named `${method}` and at least one definition of + hl"""Case 1: ${cycleSym} is overloaded + |If there are multiple methods named `${cycleSym}` and at least one definition of |it calls another, you need to specify the calling method's return type. | - |Case 2: ${method} is recursive - |If `${method}` calls itself on any path (even through mutual recursion), you need to specify the return type - |of `${method}` or of a definition it's mutually recursive with. + |Case 2: ${cycleSym} is recursive + |If `${cycleSym}` calls itself on any path (even through mutual recursion), you need to specify the return type + |of `${cycleSym}` or of a definition it's mutually recursive with. |""".stripMargin } - case class RecursiveValueNeedsResultType(value: Names.TermName)(implicit ctx: Context) + case class RecursiveValueNeedsResultType(valName: Names.TermName, cycleSym: Symbol)(implicit ctx: Context) extends Message(RecursiveValueNeedsResultTypeID) { val kind = "Syntax" - val msg = hl"""recursive value ${value} needs type""" + val msg = hl"""recursive ${cycleSym} needs type""" val explanation = - hl"""The definition of `${value}` is recursive and you need to specify its type. + hl"""The definition of `${cycleSym}` is recursive and you need to specify its type. |""".stripMargin } @@ -2111,13 +2111,13 @@ object messages { } // Relative of CyclicReferenceInvolvingImplicit and RecursiveValueNeedsResultType - case class TermMemberNeedsResultTypeForImplicitSearch(cycleSym: Symbol)(implicit ctx: Context) + case class TermMemberNeedsResultTypeForImplicitSearch(memberName: Names.TermName, cycleSym: Symbol)(implicit ctx: Context) extends Message(TermMemberNeedsNeedsResultTypeForImplicitSearchID) { val kind = "Syntax" val msg = hl"""$cycleSym needs result type because its right-hand side attempts implicit search""" val explanation = hl"""|The right hand-side of $cycleSym's definition requires an implicit search at the highlighted position. - |To avoid this error, give `${cycleSym.name}` an explicit type. + |To avoid this error, give `$cycleSym` an explicit type. |""".stripMargin } } diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala index e703d063392e..f2c5dc074cff 100644 --- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala +++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala @@ -207,8 +207,9 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val OverloadedOrRecursiveMethodNeedsResultType(tree) :: Nil = messages - assertEquals("foo", tree.show) + val OverloadedOrRecursiveMethodNeedsResultType(methodName, cycleSym) :: Nil = messages + assertEquals("foo", methodName.show) + assertEquals("foo", cycleSym.name.show) } @Test def recursiveMethodNeedsReturnType = @@ -223,8 +224,9 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val OverloadedOrRecursiveMethodNeedsResultType(tree) :: Nil = messages - assertEquals("i", tree.show) + val OverloadedOrRecursiveMethodNeedsResultType(methodName, cycleSym) :: Nil = messages + assertEquals("i", methodName.show) + assertEquals("i", cycleSym.name.show) } @Test def recursiveValueNeedsReturnType = @@ -239,10 +241,29 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val RecursiveValueNeedsResultType(tree) :: Nil = messages - assertEquals("i", tree.show) + val RecursiveValueNeedsResultType(valName, cycleSym) :: Nil = messages + assertEquals("i", valName.show) + assertEquals("i", cycleSym.name.show) } + @Test def recursiveValueNeedsReturnType2 = + checkMessagesAfter(FrontEnd.name) { + """ + |class Scope() { + | lazy val i = j + 5 + | lazy val j = i + |} + """.stripMargin + } + .expect { (ictx, messages) => + implicit val ctx: Context = ictx + + assertMessageCount(1, messages) + val RecursiveValueNeedsResultType(valName, cycleSym) :: Nil = messages + assertEquals("j", valName.show) + assertEquals("i", cycleSym.name.show) + } + @Test def cyclicReferenceInvolving = checkMessagesAfter(FrontEnd.name) { """ @@ -290,8 +311,9 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val OverloadedOrRecursiveMethodNeedsResultType(name) :: Nil = messages - assertEquals("even", name.show) + val OverloadedOrRecursiveMethodNeedsResultType(methodName, cycleSym) :: Nil = messages + assertEquals("even", methodName.show) + assertEquals("odd", cycleSym.name.show) } @Test def mutualRecursion_i2001a = @@ -312,13 +334,37 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val OverloadedOrRecursiveMethodNeedsResultType(denot) :: Nil = messages + val OverloadedOrRecursiveMethodNeedsResultType(methodName, cycleSym) :: Nil = messages // Not ideal behavior - assertEquals("foo", denot.show) + assertEquals("foo", methodName.show) + assertEquals("odd", cycleSym.name.show) } + @Test def mutualRecursion_i2001b = + checkMessagesAfter(FrontEnd.name) { + """ + |class A { + | def odd(x: Int) = if (x == 0) false else !even(x-1) + | def even(x: Int) = { + | val foo = { + | if (x == 0) true else !odd(x-1) // error: overloaded or recursive method needs result type + | } + | false + | } + |} + """.stripMargin + } + .expect { (ictx, messages) => + implicit val ctx: Context = ictx + + assertMessageCount(1, messages) + val OverloadedOrRecursiveMethodNeedsResultType(methodName, cycleSym) :: Nil = messages + // Not ideal behavior + assertEquals("foo", methodName.show) + assertEquals("odd", cycleSym.name.show) + } - @Test def termMemberNeedsNeedsResultTypeForImplicitSearch = + @Test def termMemberNeedsResultTypeForImplicitSearch = checkMessagesAfter(FrontEnd.name) { """ |object implicitDefs { @@ -334,8 +380,9 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val TermMemberNeedsResultTypeForImplicitSearch(tree) :: Nil = messages - assertEquals("x", tree.name.show) + val TermMemberNeedsResultTypeForImplicitSearch(memberName, cycleSym) :: Nil = messages + assertEquals("x", cycleSym.name.show) + assertEquals("x", memberName.show) } @Test def implicitSearchForcesImplicitRetType_i4709 = @@ -359,8 +406,9 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val TermMemberNeedsResultTypeForImplicitSearch(tree) :: Nil = messages - assertEquals("ctx", tree.name.show) + val TermMemberNeedsResultTypeForImplicitSearch(memberName, cycleSym) :: Nil = messages + assertEquals("ctx", memberName.show) + assertEquals("ctx", cycleSym.name.show) } @Test def implicitSearchForcesNonImplicitRetTypeOnExplicitImport_i3253 = @@ -377,8 +425,9 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val TermMemberNeedsResultTypeForImplicitSearch(tree) :: Nil = messages - assertEquals("test", tree.name.show) + val TermMemberNeedsResultTypeForImplicitSearch(memberName, cycleSym) :: Nil = messages + assertEquals("test", memberName.show) + assertEquals("test", cycleSym.name.show) } @Test def superQualMustBeParent = diff --git a/tests/neg/i2001b.scala b/tests/neg/i2001b.scala new file mode 100644 index 000000000000..bc489b6ecc7a --- /dev/null +++ b/tests/neg/i2001b.scala @@ -0,0 +1,9 @@ +class A { + def odd(x: Int) = if (x == 0) false else !even(x-1) + def even(x: Int) = { + val foo = { + if (x == 0) true else !odd(x-1) // error: overloaded or recursive method needs result type + } + false + } +} From ba06519ece788a8e48a5f788cc78382288a61ca0 Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Thu, 9 Aug 2018 15:45:24 +0200 Subject: [PATCH 7/9] Review comment: ${cycleSym} -> $cycleSym --- .../dotc/reporting/diagnostic/messages.scala | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 13fffd701390..905d4bd310dd 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1263,24 +1263,24 @@ object messages { case class OverloadedOrRecursiveMethodNeedsResultType(methodName: Names.TermName, cycleSym: Symbol)(implicit ctx: Context) extends Message(OverloadedOrRecursiveMethodNeedsResultTypeID) { val kind = "Syntax" - val msg = hl"""overloaded or recursive ${cycleSym} needs return type""" + val msg = hl"""overloaded or recursive $cycleSym needs return type""" val explanation = - hl"""Case 1: ${cycleSym} is overloaded - |If there are multiple methods named `${cycleSym}` and at least one definition of + hl"""Case 1: $cycleSym is overloaded + |If there are multiple methods named `$cycleSym` and at least one definition of |it calls another, you need to specify the calling method's return type. | - |Case 2: ${cycleSym} is recursive - |If `${cycleSym}` calls itself on any path (even through mutual recursion), you need to specify the return type - |of `${cycleSym}` or of a definition it's mutually recursive with. + |Case 2: $cycleSym is recursive + |If `$cycleSym` calls itself on any path (even through mutual recursion), you need to specify the return type + |of `$cycleSym` or of a definition it's mutually recursive with. |""".stripMargin } case class RecursiveValueNeedsResultType(valName: Names.TermName, cycleSym: Symbol)(implicit ctx: Context) extends Message(RecursiveValueNeedsResultTypeID) { val kind = "Syntax" - val msg = hl"""recursive ${cycleSym} needs type""" + val msg = hl"""recursive $cycleSym needs type""" val explanation = - hl"""The definition of `${cycleSym}` is recursive and you need to specify its type. + hl"""The definition of `$cycleSym` is recursive and you need to specify its type. |""".stripMargin } From e75ee84cd61dffe866cf71c2c9d892907a0e9b0a Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Thu, 9 Aug 2018 15:49:33 +0200 Subject: [PATCH 8/9] Drop {method,val}Name from cyclic errors --- .../dotty/tools/dotc/core/TypeErrors.scala | 6 ++-- .../dotc/reporting/diagnostic/messages.scala | 6 ++-- .../dotc/reporting/ErrorMessagesTests.scala | 32 ++++++------------- 3 files changed, 16 insertions(+), 28 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala index 83ce3422477e..bafa67943887 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErrors.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErrors.scala @@ -125,11 +125,11 @@ class CyclicReference private (val denot: SymDenotation) extends TypeError { cx.tree match { case tree: untpd.ValOrDefDef if !tree.tpt.typeOpt.exists => if (inImplicitSearch) - TermMemberNeedsResultTypeForImplicitSearch(tree.name, cycleSym) + TermMemberNeedsResultTypeForImplicitSearch(cycleSym) else if (isMethod) - OverloadedOrRecursiveMethodNeedsResultType(tree.name, cycleSym) + OverloadedOrRecursiveMethodNeedsResultType(cycleSym) else if (isVal) - RecursiveValueNeedsResultType(tree.name, cycleSym) + RecursiveValueNeedsResultType(cycleSym) else errorMsg(cx.outer) case _ => diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 905d4bd310dd..9c7a6907c280 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1260,7 +1260,7 @@ object messages { |""".stripMargin } - case class OverloadedOrRecursiveMethodNeedsResultType(methodName: Names.TermName, cycleSym: Symbol)(implicit ctx: Context) + case class OverloadedOrRecursiveMethodNeedsResultType(cycleSym: Symbol)(implicit ctx: Context) extends Message(OverloadedOrRecursiveMethodNeedsResultTypeID) { val kind = "Syntax" val msg = hl"""overloaded or recursive $cycleSym needs return type""" @@ -1275,7 +1275,7 @@ object messages { |""".stripMargin } - case class RecursiveValueNeedsResultType(valName: Names.TermName, cycleSym: Symbol)(implicit ctx: Context) + case class RecursiveValueNeedsResultType(cycleSym: Symbol)(implicit ctx: Context) extends Message(RecursiveValueNeedsResultTypeID) { val kind = "Syntax" val msg = hl"""recursive $cycleSym needs type""" @@ -2111,7 +2111,7 @@ object messages { } // Relative of CyclicReferenceInvolvingImplicit and RecursiveValueNeedsResultType - case class TermMemberNeedsResultTypeForImplicitSearch(memberName: Names.TermName, cycleSym: Symbol)(implicit ctx: Context) + case class TermMemberNeedsResultTypeForImplicitSearch(cycleSym: Symbol)(implicit ctx: Context) extends Message(TermMemberNeedsNeedsResultTypeForImplicitSearchID) { val kind = "Syntax" val msg = hl"""$cycleSym needs result type because its right-hand side attempts implicit search""" diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala index f2c5dc074cff..36cd95e6fc17 100644 --- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala +++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala @@ -207,8 +207,7 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val OverloadedOrRecursiveMethodNeedsResultType(methodName, cycleSym) :: Nil = messages - assertEquals("foo", methodName.show) + val OverloadedOrRecursiveMethodNeedsResultType(cycleSym) :: Nil = messages assertEquals("foo", cycleSym.name.show) } @@ -224,8 +223,7 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val OverloadedOrRecursiveMethodNeedsResultType(methodName, cycleSym) :: Nil = messages - assertEquals("i", methodName.show) + val OverloadedOrRecursiveMethodNeedsResultType(cycleSym) :: Nil = messages assertEquals("i", cycleSym.name.show) } @@ -241,8 +239,7 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val RecursiveValueNeedsResultType(valName, cycleSym) :: Nil = messages - assertEquals("i", valName.show) + val RecursiveValueNeedsResultType(cycleSym) :: Nil = messages assertEquals("i", cycleSym.name.show) } @@ -259,8 +256,7 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val RecursiveValueNeedsResultType(valName, cycleSym) :: Nil = messages - assertEquals("j", valName.show) + val RecursiveValueNeedsResultType(cycleSym) :: Nil = messages assertEquals("i", cycleSym.name.show) } @@ -311,8 +307,7 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val OverloadedOrRecursiveMethodNeedsResultType(methodName, cycleSym) :: Nil = messages - assertEquals("even", methodName.show) + val OverloadedOrRecursiveMethodNeedsResultType(cycleSym) :: Nil = messages assertEquals("odd", cycleSym.name.show) } @@ -334,9 +329,7 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val OverloadedOrRecursiveMethodNeedsResultType(methodName, cycleSym) :: Nil = messages - // Not ideal behavior - assertEquals("foo", methodName.show) + val OverloadedOrRecursiveMethodNeedsResultType(cycleSym) :: Nil = messages assertEquals("odd", cycleSym.name.show) } @@ -358,9 +351,7 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val OverloadedOrRecursiveMethodNeedsResultType(methodName, cycleSym) :: Nil = messages - // Not ideal behavior - assertEquals("foo", methodName.show) + val OverloadedOrRecursiveMethodNeedsResultType(cycleSym) :: Nil = messages assertEquals("odd", cycleSym.name.show) } @@ -380,9 +371,8 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val TermMemberNeedsResultTypeForImplicitSearch(memberName, cycleSym) :: Nil = messages + val TermMemberNeedsResultTypeForImplicitSearch(cycleSym) :: Nil = messages assertEquals("x", cycleSym.name.show) - assertEquals("x", memberName.show) } @Test def implicitSearchForcesImplicitRetType_i4709 = @@ -406,8 +396,7 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val TermMemberNeedsResultTypeForImplicitSearch(memberName, cycleSym) :: Nil = messages - assertEquals("ctx", memberName.show) + val TermMemberNeedsResultTypeForImplicitSearch(cycleSym) :: Nil = messages assertEquals("ctx", cycleSym.name.show) } @@ -425,8 +414,7 @@ class ErrorMessagesTests extends ErrorMessagesTest { implicit val ctx: Context = ictx assertMessageCount(1, messages) - val TermMemberNeedsResultTypeForImplicitSearch(memberName, cycleSym) :: Nil = messages - assertEquals("test", memberName.show) + val TermMemberNeedsResultTypeForImplicitSearch(cycleSym) :: Nil = messages assertEquals("test", cycleSym.name.show) } From d8e6d3ef892ca676aa512655e0ea1b233bf930be Mon Sep 17 00:00:00 2001 From: "Paolo G. Giarrusso" Date: Thu, 9 Aug 2018 15:53:03 +0200 Subject: [PATCH 9/9] Reclassify error messages due to CyclicReference as Cyclic As suggested in review. --- .../tools/dotc/reporting/diagnostic/messages.scala | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala index 9c7a6907c280..1c988606b9f9 100644 --- a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala @@ -1262,7 +1262,7 @@ object messages { case class OverloadedOrRecursiveMethodNeedsResultType(cycleSym: Symbol)(implicit ctx: Context) extends Message(OverloadedOrRecursiveMethodNeedsResultTypeID) { - val kind = "Syntax" + val kind = "Cyclic" val msg = hl"""overloaded or recursive $cycleSym needs return type""" val explanation = hl"""Case 1: $cycleSym is overloaded @@ -1277,7 +1277,7 @@ object messages { case class RecursiveValueNeedsResultType(cycleSym: Symbol)(implicit ctx: Context) extends Message(RecursiveValueNeedsResultTypeID) { - val kind = "Syntax" + val kind = "Cyclic" val msg = hl"""recursive $cycleSym needs type""" val explanation = hl"""The definition of `$cycleSym` is recursive and you need to specify its type. @@ -1286,7 +1286,7 @@ object messages { case class CyclicReferenceInvolving(denot: SymDenotation)(implicit ctx: Context) extends Message(CyclicReferenceInvolvingID) { - val kind = "Syntax" + val kind = "Cyclic" val msg = hl"""cyclic reference involving $denot""" val explanation = hl"""|$denot is declared as part of a cycle which makes it impossible for the @@ -1297,7 +1297,7 @@ object messages { case class CyclicReferenceInvolvingImplicit(cycleSym: Symbol)(implicit ctx: Context) extends Message(CyclicReferenceInvolvingImplicitID) { - val kind = "Syntax" + val kind = "Cyclic" val msg = hl"""cyclic reference involving implicit $cycleSym""" val explanation = hl"""|$cycleSym is declared as part of a cycle which makes it impossible for the @@ -2113,7 +2113,7 @@ object messages { // Relative of CyclicReferenceInvolvingImplicit and RecursiveValueNeedsResultType case class TermMemberNeedsResultTypeForImplicitSearch(cycleSym: Symbol)(implicit ctx: Context) extends Message(TermMemberNeedsNeedsResultTypeForImplicitSearchID) { - val kind = "Syntax" + val kind = "Cyclic" val msg = hl"""$cycleSym needs result type because its right-hand side attempts implicit search""" val explanation = hl"""|The right hand-side of $cycleSym's definition requires an implicit search at the highlighted position.