Skip to content

Commit a91d82a

Browse files
BlaisorbladeBlaisorblade
authored
Merge pull request #4729 from dotty-staging/fix-4709
Fix #4709: report if local implicits need type annotation
2 parents ff5e69c + d8e6d3e commit a91d82a

File tree

10 files changed

+264
-39
lines changed

10 files changed

+264
-39
lines changed

‎compiler/src/dotty/tools/dotc/ast/Trees.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -373,6 +373,7 @@ object Trees {
373373

374374
/** A ValDef or DefDef tree */
375375
trait ValOrDefDef[-T >: Untyped] extends MemberDef[T] with WithLazyField[Tree[T]] {
376+
def name: TermName
376377
def tpt: Tree[T]
377378
def unforcedRhs: LazyTree = unforced
378379
def rhs(implicit ctx: Context): Tree[T] = forceIfLazy

‎compiler/src/dotty/tools/dotc/core/TypeErrors.scala

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -98,28 +98,52 @@ object handleRecursive {
9898
}
9999
}
100100

101+
/**
102+
* This TypeError signals that completing denot encountered a cycle: it asked for denot.info (or similar),
103+
* so it requires knowing denot already.
104+
* @param denot
105+
*/
101106
class CyclicReference private (val denot: SymDenotation) extends TypeError {
107+
var inImplicitSearch: Boolean = false
102108

103-
override def toMessage(implicit ctx: Context) = {
109+
override def toMessage(implicit ctx: Context): Message = {
110+
val cycleSym = denot.symbol
104111

105-
def errorMsg(cx: Context): Message =
112+
// cycleSym.flags would try completing denot and would fail, but here we can use flagsUNSAFE to detect flags
113+
// set by the parser.
114+
val unsafeFlags = cycleSym.flagsUNSAFE
115+
val isMethod = unsafeFlags.is(Method)
116+
val isVal = !isMethod && cycleSym.isTerm
117+
118+
/* This CyclicReference might have arisen from asking for `m`'s type while trying to infer it.
119+
* To try to diagnose this, walk the context chain searching for context in
120+
* Mode.InferringReturnType for the innermost member without type
121+
* annotations (!tree.tpt.typeOpt.exists).
122+
*/
123+
def errorMsg(cx: Context): Message = {
106124
if (cx.mode is Mode.InferringReturnType) {
107125
cx.tree match {
108-
case tree: untpd.DefDef if !tree.tpt.typeOpt.exists =>
109-
OverloadedOrRecursiveMethodNeedsResultType(tree.name)
110-
case tree: untpd.ValDef if !tree.tpt.typeOpt.exists =>
111-
RecursiveValueNeedsResultType(tree.name)
126+
case tree: untpd.ValOrDefDef if !tree.tpt.typeOpt.exists =>
127+
if (inImplicitSearch)
128+
TermMemberNeedsResultTypeForImplicitSearch(cycleSym)
129+
else if (isMethod)
130+
OverloadedOrRecursiveMethodNeedsResultType(cycleSym)
131+
else if (isVal)
132+
RecursiveValueNeedsResultType(cycleSym)
133+
else
134+
errorMsg(cx.outer)
112135
case _ =>
113136
errorMsg(cx.outer)
114137
}
115138
}
116-
else CyclicReferenceInvolving(denot)
139+
// Give up and give generic errors.
140+
else if (cycleSym.is(Implicit, butNot = Method) && cycleSym.owner.isTerm)
141+
CyclicReferenceInvolvingImplicit(cycleSym)
142+
else
143+
CyclicReferenceInvolving(denot)
144+
}
117145

118-
val cycleSym = denot.symbol
119-
if (cycleSym.is(Implicit, butNot = Method) && cycleSym.owner.isTerm)
120-
CyclicReferenceInvolvingImplicit(cycleSym)
121-
else
122-
errorMsg(ctx)
146+
errorMsg(ctx)
123147
}
124148
}
125149

‎compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ public enum ErrorMessageID {
131131
MatchCaseOnlyNullWarningID,
132132
ImportRenamedTwiceID,
133133
TypeTestAlwaysSucceedsID,
134+
TermMemberNeedsNeedsResultTypeForImplicitSearchID
134135
;
135136

136137
public int errorNumber() {

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

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1260,46 +1260,50 @@ object messages {
12601260
|""".stripMargin
12611261
}
12621262

1263-
case class OverloadedOrRecursiveMethodNeedsResultType(method: Names.TermName)(implicit ctx: Context)
1263+
case class OverloadedOrRecursiveMethodNeedsResultType(cycleSym: Symbol)(implicit ctx: Context)
12641264
extends Message(OverloadedOrRecursiveMethodNeedsResultTypeID) {
1265-
val kind = "Syntax"
1266-
val msg = hl"""overloaded or recursive method ${method} needs return type"""
1265+
val kind = "Cyclic"
1266+
val msg = hl"""overloaded or recursive $cycleSym needs return type"""
12671267
val explanation =
1268-
hl"""Case 1: ${method} is overloaded
1269-
|If there are multiple methods named `${method}` and at least one definition of
1268+
hl"""Case 1: $cycleSym is overloaded
1269+
|If there are multiple methods named `$cycleSym` and at least one definition of
12701270
|it calls another, you need to specify the calling method's return type.
12711271
|
1272-
|Case 2: ${method} is recursive
1273-
|If `${method}` calls itself on any path, you need to specify its return type.
1272+
|Case 2: $cycleSym is recursive
1273+
|If `$cycleSym` calls itself on any path (even through mutual recursion), you need to specify the return type
1274+
|of `$cycleSym` or of a definition it's mutually recursive with.
12741275
|""".stripMargin
12751276
}
12761277

1277-
case class RecursiveValueNeedsResultType(value: Names.TermName)(implicit ctx: Context)
1278+
case class RecursiveValueNeedsResultType(cycleSym: Symbol)(implicit ctx: Context)
12781279
extends Message(RecursiveValueNeedsResultTypeID) {
1279-
val kind = "Syntax"
1280-
val msg = hl"""recursive value ${value} needs type"""
1280+
val kind = "Cyclic"
1281+
val msg = hl"""recursive $cycleSym needs type"""
12811282
val explanation =
1282-
hl"""The definition of `${value}` is recursive and you need to specify its type.
1283+
hl"""The definition of `$cycleSym` is recursive and you need to specify its type.
12831284
|""".stripMargin
12841285
}
12851286

12861287
case class CyclicReferenceInvolving(denot: SymDenotation)(implicit ctx: Context)
12871288
extends Message(CyclicReferenceInvolvingID) {
1288-
val kind = "Syntax"
1289+
val kind = "Cyclic"
12891290
val msg = hl"""cyclic reference involving $denot"""
12901291
val explanation =
12911292
hl"""|$denot is declared as part of a cycle which makes it impossible for the
12921293
|compiler to decide upon ${denot.name}'s type.
1294+
|To avoid this error, try giving `${denot.name}` an explicit type.
12931295
|""".stripMargin
12941296
}
12951297

12961298
case class CyclicReferenceInvolvingImplicit(cycleSym: Symbol)(implicit ctx: Context)
12971299
extends Message(CyclicReferenceInvolvingImplicitID) {
1298-
val kind = "Syntax"
1300+
val kind = "Cyclic"
12991301
val msg = hl"""cyclic reference involving implicit $cycleSym"""
13001302
val explanation =
1301-
hl"""|This happens when the right hand-side of $cycleSym's definition involves an implicit search.
1302-
|To avoid this error, give `${cycleSym.name}` an explicit type.
1303+
hl"""|$cycleSym is declared as part of a cycle which makes it impossible for the
1304+
|compiler to decide upon ${cycleSym.name}'s type.
1305+
|This might happen when the right hand-side of $cycleSym's definition involves an implicit search.
1306+
|To avoid this error, try giving `${cycleSym.name}` an explicit type.
13031307
|""".stripMargin
13041308
}
13051309

@@ -2105,4 +2109,15 @@ object messages {
21052109
}
21062110
val explanation = ""
21072111
}
2112+
2113+
// Relative of CyclicReferenceInvolvingImplicit and RecursiveValueNeedsResultType
2114+
case class TermMemberNeedsResultTypeForImplicitSearch(cycleSym: Symbol)(implicit ctx: Context)
2115+
extends Message(TermMemberNeedsNeedsResultTypeForImplicitSearchID) {
2116+
val kind = "Cyclic"
2117+
val msg = hl"""$cycleSym needs result type because its right-hand side attempts implicit search"""
2118+
val explanation =
2119+
hl"""|The right hand-side of $cycleSym's definition requires an implicit search at the highlighted position.
2120+
|To avoid this error, give `$cycleSym` an explicit type.
2121+
|""".stripMargin
2122+
}
21082123
}

‎compiler/src/dotty/tools/dotc/typer/Implicits.scala

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -847,7 +847,15 @@ trait Implicits { self: Typer =>
847847
else i"type error: ${argument.tpe} does not conform to $pt${err.whyNoMatchStr(argument.tpe, pt)}")
848848
trace(s"search implicit ${pt.show}, arg = ${argument.show}: ${argument.tpe.show}", implicits, show = true) {
849849
assert(!pt.isInstanceOf[ExprType])
850-
val result = new ImplicitSearch(pt, argument, pos).bestImplicit(contextual = true)
850+
val result =
851+
try {
852+
new ImplicitSearch(pt, argument, pos).bestImplicit(contextual = true)
853+
} catch {
854+
case ce: CyclicReference =>
855+
ce.inImplicitSearch = true
856+
throw ce
857+
}
858+
851859
result match {
852860
case result: SearchSuccess =>
853861
result.tstate.commit()

‎compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala

Lines changed: 149 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,8 @@ class ErrorMessagesTests extends ErrorMessagesTest {
207207
implicit val ctx: Context = ictx
208208

209209
assertMessageCount(1, messages)
210-
val OverloadedOrRecursiveMethodNeedsResultType(tree) :: Nil = messages
211-
assertEquals("foo", tree.show)
210+
val OverloadedOrRecursiveMethodNeedsResultType(cycleSym) :: Nil = messages
211+
assertEquals("foo", cycleSym.name.show)
212212
}
213213

214214
@Test def recursiveMethodNeedsReturnType =
@@ -223,8 +223,8 @@ class ErrorMessagesTests extends ErrorMessagesTest {
223223
implicit val ctx: Context = ictx
224224

225225
assertMessageCount(1, messages)
226-
val OverloadedOrRecursiveMethodNeedsResultType(tree) :: Nil = messages
227-
assertEquals("i", tree.show)
226+
val OverloadedOrRecursiveMethodNeedsResultType(cycleSym) :: Nil = messages
227+
assertEquals("i", cycleSym.name.show)
228228
}
229229

230230
@Test def recursiveValueNeedsReturnType =
@@ -239,10 +239,27 @@ class ErrorMessagesTests extends ErrorMessagesTest {
239239
implicit val ctx: Context = ictx
240240

241241
assertMessageCount(1, messages)
242-
val RecursiveValueNeedsResultType(tree) :: Nil = messages
243-
assertEquals("i", tree.show)
242+
val RecursiveValueNeedsResultType(cycleSym) :: Nil = messages
243+
assertEquals("i", cycleSym.name.show)
244244
}
245245

246+
@Test def recursiveValueNeedsReturnType2 =
247+
checkMessagesAfter(FrontEnd.name) {
248+
"""
249+
|class Scope() {
250+
| lazy val i = j + 5
251+
| lazy val j = i
252+
|}
253+
""".stripMargin
254+
}
255+
.expect { (ictx, messages) =>
256+
implicit val ctx: Context = ictx
257+
258+
assertMessageCount(1, messages)
259+
val RecursiveValueNeedsResultType(cycleSym) :: Nil = messages
260+
assertEquals("i", cycleSym.name.show)
261+
}
262+
246263
@Test def cyclicReferenceInvolving =
247264
checkMessagesAfter(FrontEnd.name) {
248265
"""
@@ -260,7 +277,85 @@ class ErrorMessagesTests extends ErrorMessagesTest {
260277
assertEquals("value x", denot.show)
261278
}
262279

263-
@Test def cyclicReferenceInvolvingImplicit =
280+
@Test def cyclicReferenceInvolving2 =
281+
checkMessagesAfter(FrontEnd.name) {
282+
"""
283+
|class A {
284+
| implicit val x: T = ???
285+
| type T <: x.type // error: cyclic reference involving value x
286+
|}
287+
""".stripMargin
288+
}
289+
.expect { (ictx, messages) =>
290+
implicit val ctx: Context = ictx
291+
292+
assertMessageCount(1, messages)
293+
val CyclicReferenceInvolving(denot) :: Nil = messages
294+
assertEquals("value x", denot.show)
295+
}
296+
297+
@Test def mutualRecursionre_i2001 =
298+
checkMessagesAfter(FrontEnd.name) {
299+
"""
300+
|class A {
301+
| def odd(x: Int) = if (x == 0) false else !even(x-1)
302+
| def even(x: Int) = if (x == 0) true else !odd(x-1) // error: overloaded or recursive method needs result type
303+
|}
304+
""".stripMargin
305+
}
306+
.expect { (ictx, messages) =>
307+
implicit val ctx: Context = ictx
308+
309+
assertMessageCount(1, messages)
310+
val OverloadedOrRecursiveMethodNeedsResultType(cycleSym) :: Nil = messages
311+
assertEquals("odd", cycleSym.name.show)
312+
}
313+
314+
@Test def mutualRecursion_i2001a =
315+
checkMessagesAfter(FrontEnd.name) {
316+
"""
317+
|class A {
318+
| def odd(x: Int) = if (x == 0) false else !even(x-1)
319+
| def even(x: Int) = {
320+
| def foo = {
321+
| if (x == 0) true else !odd(x-1) // error: overloaded or recursive method needs result type
322+
| }
323+
| false
324+
| }
325+
|}
326+
""".stripMargin
327+
}
328+
.expect { (ictx, messages) =>
329+
implicit val ctx: Context = ictx
330+
331+
assertMessageCount(1, messages)
332+
val OverloadedOrRecursiveMethodNeedsResultType(cycleSym) :: Nil = messages
333+
assertEquals("odd", cycleSym.name.show)
334+
}
335+
336+
@Test def mutualRecursion_i2001b =
337+
checkMessagesAfter(FrontEnd.name) {
338+
"""
339+
|class A {
340+
| def odd(x: Int) = if (x == 0) false else !even(x-1)
341+
| def even(x: Int) = {
342+
| val foo = {
343+
| if (x == 0) true else !odd(x-1) // error: overloaded or recursive method needs result type
344+
| }
345+
| false
346+
| }
347+
|}
348+
""".stripMargin
349+
}
350+
.expect { (ictx, messages) =>
351+
implicit val ctx: Context = ictx
352+
353+
assertMessageCount(1, messages)
354+
val OverloadedOrRecursiveMethodNeedsResultType(cycleSym) :: Nil = messages
355+
assertEquals("odd", cycleSym.name.show)
356+
}
357+
358+
@Test def termMemberNeedsResultTypeForImplicitSearch =
264359
checkMessagesAfter(FrontEnd.name) {
265360
"""
266361
|object implicitDefs {
@@ -276,10 +371,53 @@ class ErrorMessagesTests extends ErrorMessagesTest {
276371
implicit val ctx: Context = ictx
277372

278373
assertMessageCount(1, messages)
279-
val CyclicReferenceInvolvingImplicit(tree) :: Nil = messages
280-
assertEquals("x", tree.name.show)
374+
val TermMemberNeedsResultTypeForImplicitSearch(cycleSym) :: Nil = messages
375+
assertEquals("x", cycleSym.name.show)
281376
}
282377

378+
@Test def implicitSearchForcesImplicitRetType_i4709 =
379+
checkMessagesAfter(FrontEnd.name) {
380+
"""
381+
|import scala.language.implicitConversions
382+
|
383+
|class Context
384+
|class ContextBase { def settings = 1 }
385+
|
386+
|class Test {
387+
| implicit def toBase(ctx: Context): ContextBase = ???
388+
|
389+
| def test(ctx0: Context) = {
390+
| implicit val ctx = { ctx0.settings; ??? }
391+
| }
392+
|}
393+
""".stripMargin
394+
}
395+
.expect{ (ictx, messages) =>
396+
implicit val ctx: Context = ictx
397+
398+
assertMessageCount(1, messages)
399+
val TermMemberNeedsResultTypeForImplicitSearch(cycleSym) :: Nil = messages
400+
assertEquals("ctx", cycleSym.name.show)
401+
}
402+
403+
@Test def implicitSearchForcesNonImplicitRetTypeOnExplicitImport_i3253 =
404+
checkMessagesAfter(FrontEnd.name) {
405+
"""
406+
|import Test.test
407+
|
408+
|object Test {
409+
| def test = " " * 10
410+
|}
411+
""".stripMargin
412+
}
413+
.expect{ (ictx, messages) =>
414+
implicit val ctx: Context = ictx
415+
416+
assertMessageCount(1, messages)
417+
val TermMemberNeedsResultTypeForImplicitSearch(cycleSym) :: Nil = messages
418+
assertEquals("test", cycleSym.name.show)
419+
}
420+
283421
@Test def superQualMustBeParent =
284422
checkMessagesAfter(FrontEnd.name) {
285423
"""
@@ -334,7 +472,7 @@ class ErrorMessagesTests extends ErrorMessagesTest {
334472
assertEquals(namedImport, prevPrec)
335473
}
336474

337-
@Test def methodDoesNotTakePrameters =
475+
@Test def methodDoesNotTakeParameters =
338476
checkMessagesAfter(FrontEnd.name) {
339477
"""
340478
|object Scope {
@@ -353,7 +491,7 @@ class ErrorMessagesTests extends ErrorMessagesTest {
353491
assertEquals("method foo", msg.methodSymbol.show)
354492
}
355493

356-
@Test def methodDoesNotTakeMorePrameters =
494+
@Test def methodDoesNotTakeMoreParameters =
357495
checkMessagesAfter(FrontEnd.name) {
358496
"""
359497
|object Scope{

0 commit comments

Comments
 (0)