Skip to content

Commit be26204

Browse files
oderskyodersky
committed
Improve error reporting
With the earlier constraints on closure types, we get more localized errors in closures, but these sometimes need additional context to be intelligible.
1 parent 684d5cd commit be26204

File tree

17 files changed

+162
-86
lines changed

17 files changed

+162
-86
lines changed

‎compiler/src/dotty/tools/dotc/cc/CheckCaptures.scala

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -262,30 +262,36 @@ class CheckCaptures extends Recheck, SymTransformer:
262262
def assertSub(cs1: CaptureSet, cs2: CaptureSet)(using Context) =
263263
assert(cs1.subCaptures(cs2, frozen = false).isOK, i"$cs1 is not a subset of $cs2")
264264

265-
def checkOK(res: CompareResult, prefix: => String, pos: SrcPos)(using Context): Unit =
265+
def checkOK(res: CompareResult, prefix: => String, pos: SrcPos, provenance: => String = "")(using Context): Unit =
266266
if !res.isOK then
267267
def toAdd: String = CaptureSet.levelErrors.toAdd.mkString
268-
report.error(em"$prefix included in the allowed capture set ${res.blocking}$toAdd", pos)
268+
def descr: String =
269+
val d = res.blocking.description
270+
if d.isEmpty then provenance else ""
271+
report.error(em"$prefix included in the allowed capture set ${res.blocking}$descr$toAdd", pos)
269272

270273
/** Check subcapturing `{elem} <: cs`, report error on failure */
271-
def checkElem(elem: CaptureRef, cs: CaptureSet, pos: SrcPos)(using Context) =
274+
def checkElem(elem: CaptureRef, cs: CaptureSet, pos: SrcPos, provenance: => String = "")(using Context) =
272275
checkOK(
273276
elem.singletonCaptureSet.subCaptures(cs, frozen = false),
274277
i"$elem cannot be referenced here; it is not",
275-
pos)
278+
pos, provenance)
276279

277280
/** Check subcapturing `cs1 <: cs2`, report error on failure */
278-
def checkSubset(cs1: CaptureSet, cs2: CaptureSet, pos: SrcPos)(using Context) =
281+
def checkSubset(cs1: CaptureSet, cs2: CaptureSet, pos: SrcPos, provenance: => String = "")(using Context) =
279282
checkOK(
280283
cs1.subCaptures(cs2, frozen = false),
281-
if cs1.elems.size == 1 then i"reference ${cs1.elems.toList}%, % is not"
284+
if cs1.elems.size == 1 then i"reference ${cs1.elems.toList.head} is not"
282285
else i"references $cs1 are not all",
283-
pos)
286+
pos, provenance)
284287

285288
/** The current environment */
286289
private var curEnv: Env = inContext(ictx):
287290
Env(defn.RootClass, EnvKind.Regular, CaptureSet.empty, null)
288291

292+
/** Currently checked closures and their expected types, used for error reporting */
293+
private var openClosures: List[(Symbol, Type)] = Nil
294+
289295
private val myCapturedVars: util.EqHashMap[Symbol, CaptureSet] = EqHashMap()
290296

291297
/** If `sym` is a class or method nested inside a term, a capture set variable representing
@@ -312,6 +318,19 @@ class CheckCaptures extends Recheck, SymTransformer:
312318
recur(nextEnv, skip = env.kind == EnvKind.ClosureResult)
313319
recur(curEnv, skip = false)
314320

321+
/** A description where this environment comes from */
322+
private def provenance(env: Env)(using Context): String =
323+
val owner = env.owner
324+
if owner.isAnonymousFunction then
325+
val expected = openClosures
326+
.find(_._1 == owner)
327+
.map(_._2)
328+
.getOrElse(owner.info.toFunctionType(isJava = false))
329+
i"\nof an enclosing function literal with expected type $expected"
330+
else
331+
i"\nof the enclosing ${owner.showLocated}"
332+
333+
315334
/** Include `sym` in the capture sets of all enclosing environments nested in the
316335
* the environment in which `sym` is defined.
317336
*/
@@ -321,7 +340,7 @@ class CheckCaptures extends Recheck, SymTransformer:
321340
if ref.isTracked then
322341
forallOuterEnvsUpTo(sym.enclosure): env =>
323342
capt.println(i"Mark $sym with cs ${ref.captureSet} free in ${env.owner}")
324-
checkElem(ref, env.captured, pos)
343+
checkElem(ref, env.captured, pos, provenance(env))
325344

326345
/** Make sure (projected) `cs` is a subset of the capture sets of all enclosing
327346
* environments. At each stage, only include references from `cs` that are outside
@@ -338,7 +357,7 @@ class CheckCaptures extends Recheck, SymTransformer:
338357
case ref: ThisType => isVisibleFromEnv(ref.cls)
339358
case _ => false
340359
capt.println(i"Include call capture $included in ${env.owner}")
341-
checkSubset(included, env.captured, pos)
360+
checkSubset(included, env.captured, pos, provenance(env))
342361

343362
/** Include references captured by the called method in the current environment stack */
344363
def includeCallCaptures(sym: Symbol, pos: SrcPos)(using Context): Unit =
@@ -585,11 +604,15 @@ class CheckCaptures extends Recheck, SymTransformer:
585604

586605
// Constrain closure's parameters and result from the expected type before
587606
// rechecking the body.
588-
val res = recheckClosure(expr, pt, forceDependent = true)
589-
checkConformsExpr(res, pt, expr)
590-
recheckDef(mdef, mdef.symbol)
591-
//println(i"RECHECK CLOSURE ${mdef.symbol.info}")
592-
res
607+
openClosures = (mdef.symbol, pt) :: openClosures
608+
try
609+
val res = recheckClosure(expr, pt, forceDependent = true)
610+
checkConformsExpr(res, pt, expr)
611+
recheckDef(mdef, mdef.symbol)
612+
//println(i"RECHECK CLOSURE ${mdef.symbol.info}")
613+
res
614+
finally
615+
openClosures = openClosures.tail
593616
end recheckClosureBlock
594617

595618
override def recheckValDef(tree: ValDef, sym: Symbol)(using Context): Unit =
@@ -608,7 +631,8 @@ class CheckCaptures extends Recheck, SymTransformer:
608631
if !Synthetics.isExcluded(sym) then
609632
val saved = curEnv
610633
val localSet = capturedVars(sym)
611-
if !localSet.isAlwaysEmpty then curEnv = Env(sym, EnvKind.Regular, localSet, curEnv)
634+
if !localSet.isAlwaysEmpty then
635+
curEnv = Env(sym, EnvKind.Regular, localSet, curEnv)
612636
try super.recheckDefDef(tree, sym)
613637
finally
614638
interpolateVarsIn(tree.tpt)
@@ -625,7 +649,8 @@ class CheckCaptures extends Recheck, SymTransformer:
625649
val saved = curEnv
626650
val localSet = capturedVars(cls)
627651
for parent <- impl.parents do // (1)
628-
checkSubset(capturedVars(parent.tpe.classSymbol), localSet, parent.srcPos)
652+
checkSubset(capturedVars(parent.tpe.classSymbol), localSet, parent.srcPos,
653+
i"\nof the references allowed to be captured by $cls")
629654
if !localSet.isAlwaysEmpty then curEnv = Env(cls, EnvKind.Regular, localSet, curEnv)
630655
try
631656
val thisSet = cls.classInfo.selfType.captureSet.withDescription(i"of the self type of $cls")

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

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import util.SourcePosition
1515
import scala.util.control.NonFatal
1616
import scala.annotation.switch
1717
import config.{Config, Feature}
18-
import cc.{CapturingType, EventuallyCapturingType, CaptureSet, CaptureRoot, isBoxed, ccNestingLevel}
18+
import cc.{CapturingType, EventuallyCapturingType, CaptureSet, CaptureRoot, isBoxed, ccNestingLevel, levelOwner}
1919

2020
class PlainPrinter(_ctx: Context) extends Printer {
2121

@@ -173,8 +173,11 @@ class PlainPrinter(_ctx: Context) extends Printer {
173173
case tp: TypeType =>
174174
toTextRHS(tp)
175175
case tp: TermRef
176-
if !tp.denotationIsCurrent && !homogenizedView || // always print underlying when testing picklers
177-
tp.symbol.is(Module) || tp.symbol.name == nme.IMPORT =>
176+
if !tp.denotationIsCurrent
177+
&& !homogenizedView // always print underlying when testing picklers
178+
&& !tp.isRootCapability
179+
|| tp.symbol.is(Module)
180+
|| tp.symbol.name == nme.IMPORT =>
178181
toTextRef(tp) ~ ".type"
179182
case tp: TermRef if tp.denot.isOverloaded =>
180183
"<overloaded " ~ toTextRef(tp) ~ ">"
@@ -224,12 +227,21 @@ class PlainPrinter(_ctx: Context) extends Printer {
224227
}.close
225228
case tp @ EventuallyCapturingType(parent, refs) =>
226229
val boxText: Text = Str("box ") provided tp.isBoxed //&& ctx.settings.YccDebug.value
227-
val refsText =
228-
if refs.isUniversal &&
229-
(refs.elems.size == 1
230-
|| !ctx.settings.YccDebug.value && !refs.elems.exists(_.isLocalRootCapability))
231-
then rootSetText
232-
else toTextCaptureSet(refs)
230+
val rootsInRefs = refs.elems.filter(_.isRootCapability).toList
231+
val showAsCap = rootsInRefs match
232+
case (tp: TermRef) :: Nil =>
233+
if tp.symbol == defn.captureRoot then
234+
refs.elems.size == 1 || !printDebug
235+
// {caps.cap} gets printed as `{cap}` even under printDebug as long as there
236+
// are no other elements in the set
237+
else
238+
tp.symbol.name == nme.LOCAL_CAPTURE_ROOT
239+
&& ctx.owner.levelOwner == tp.localRootOwner
240+
&& !printDebug
241+
// local roots get printed as themselves under printDebug
242+
case _ =>
243+
false
244+
val refsText = if showAsCap then rootSetText else toTextCaptureSet(refs)
233245
toTextCapturing(parent, refsText, boxText)
234246
case tp: PreviousErrorType if ctx.settings.XprintTypes.value =>
235247
"<error>" // do not print previously reported error message because they may try to print this error type again recuresevely
@@ -361,8 +373,12 @@ class PlainPrinter(_ctx: Context) extends Printer {
361373
def toTextRef(tp: SingletonType): Text = controlled {
362374
tp match {
363375
case tp: TermRef =>
364-
if tp.symbol.name == nme.LOCAL_CAPTURE_ROOT then
365-
Str(s"cap[${tp.localRootOwner.name}]@${tp.symbol.ccNestingLevel}")
376+
if tp.symbol.name == nme.LOCAL_CAPTURE_ROOT then // TODO: Move to toTextCaptureRef
377+
if ctx.owner.levelOwner == tp.localRootOwner && !printDebug then
378+
Str("cap")
379+
else
380+
Str(s"cap[${tp.localRootOwner.name}]") ~
381+
Str(s"%${tp.symbol.ccNestingLevel}").provided(showNestingLevel)
366382
else toTextPrefixOf(tp) ~ selectionString(tp)
367383
case tp: ThisType =>
368384
nameString(tp.cls) + ".this"

‎tests/neg-custom-args/captures/box-adapt-boxing.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
trait Cap
22

33
def main(io: Cap^, fs: Cap^): Unit = {
4-
val test1: Unit -> Unit = _ => { // error
4+
val test1: Unit -> Unit = _ => {
55
type Op = [T] -> (T ->{io} Unit) -> Unit
66
val f: (Cap^{io}) -> Unit = ???
77
val op: Op = ???
8-
op[Cap^{io}](f)
8+
op[Cap^{io}](f) // error
99
// expected type of f: {io} (box {io} Cap) -> Unit
1010
// actual type: ({io} Cap) -> Unit
1111
// adapting f to the expected type will also

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,8 @@
55
| Required: (x$0: Int) ->{cap2} Int
66
|
77
| longer explanation available when compiling with `-explain`
8-
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/byname.scala:19:5 ----------------------------------------
8+
-- Error: tests/neg-custom-args/captures/byname.scala:19:5 -------------------------------------------------------------
99
19 | h(g()) // error
1010
| ^^^
11-
| Found: () ?->{cap2} I
12-
| Required: () ?->{cap1} I
13-
|
14-
| longer explanation available when compiling with `-explain`
11+
| reference (cap2 : Cap^) is not included in the allowed capture set {cap1}
12+
| of an enclosing function literal with expected type () ?->{cap1} I

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

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,13 @@
1-
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/capt1.scala:4:2 ------------------------------------------
1+
-- Error: tests/neg-custom-args/captures/capt1.scala:4:11 --------------------------------------------------------------
22
4 | () => if x == null then y else y // error
3-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4-
| Found: () ->{x} C^?
5-
| Required: () -> C
6-
|
7-
| longer explanation available when compiling with `-explain`
8-
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/capt1.scala:7:2 ------------------------------------------
3+
| ^
4+
| (x : C^) cannot be referenced here; it is not included in the allowed capture set {}
5+
| of an enclosing function literal with expected type () -> C
6+
-- Error: tests/neg-custom-args/captures/capt1.scala:7:11 --------------------------------------------------------------
97
7 | () => if x == null then y else y // error
10-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
11-
| Found: () ->{x} C^?
12-
| Required: Matchable
13-
|
14-
| longer explanation available when compiling with `-explain`
8+
| ^
9+
| (x : C^) cannot be referenced here; it is not included in the allowed capture set {}
10+
| of an enclosing function literal with expected type Matchable
1511
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/capt1.scala:14:2 -----------------------------------------
1612
14 | def f(y: Int) = if x == null then y else y // error
1713
| ^
@@ -37,10 +33,8 @@
3733
27 | def m() = if x == null then y else y
3834
|
3935
| longer explanation available when compiling with `-explain`
40-
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/capt1.scala:32:24 ----------------------------------------
36+
-- Error: tests/neg-custom-args/captures/capt1.scala:32:30 -------------------------------------------------------------
4137
32 | val z2 = h[() -> Cap](() => x) // error
42-
| ^^^^^^^
43-
| Found: () ->{x} box C^
44-
| Required: () -> box C^
45-
|
46-
| longer explanation available when compiling with `-explain`
38+
| ^
39+
| (x : C^) cannot be referenced here; it is not included in the allowed capture set {}
40+
| of an enclosing function literal with expected type () -> box C^

‎tests/neg-custom-args/captures/cc-this.check

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88
-- Error: tests/neg-custom-args/captures/cc-this.scala:10:15 -----------------------------------------------------------
99
10 | class C2(val x: () => Int): // error
1010
| ^
11-
| reference (C2.this.x : () => Int) is not included in allowed capture set {} of the self type of class C2
11+
| reference (C2.this.x : () => Int) is not included in the allowed capture set {} of the self type of class C2
1212
-- Error: tests/neg-custom-args/captures/cc-this.scala:17:8 ------------------------------------------------------------
1313
17 | class C4(val f: () => Int) extends C3 // error
1414
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
15-
| reference (C4.this.f : () => Int) is not included in allowed capture set {} of pure base class class C3
15+
| reference (C4.this.f : () => Int) is not included in the allowed capture set {} of pure base class class C3

‎tests/neg-custom-args/captures/cc-this2.check

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22
-- Error: tests/neg-custom-args/captures/cc-this2/D_2.scala:2:6 --------------------------------------------------------
33
2 |class D extends C: // error
44
|^
5-
|reference (caps.cap : caps.Root) is not included in allowed capture set {} of pure base class class C
5+
|reference (caps.cap : caps.Root) is not included in the allowed capture set {} of pure base class class C
66
3 | this: D^ =>

‎tests/neg-custom-args/captures/cc-this5.check

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
16 | def f = println(c) // error
33
| ^
44
| (c : Cap^) cannot be referenced here; it is not included in the allowed capture set {}
5+
| of the enclosing class A
56
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/cc-this5.scala:21:15 -------------------------------------
67
21 | val x: A = this // error
78
| ^^^^

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,8 @@
55
| Required: () -> Proc^{f}
66
|
77
| longer explanation available when compiling with `-explain`
8-
-- [E007] Type Mismatch Error: tests/neg-custom-args/captures/eta.scala:6:14 -------------------------------------------
8+
-- Error: tests/neg-custom-args/captures/eta.scala:6:20 ----------------------------------------------------------------
99
6 | bar( () => f ) // error
10-
| ^^^^^^^
11-
| Found: () ->{f} box () ->{f} Unit
12-
| Required: () -> box () ->? Unit
13-
|
14-
| longer explanation available when compiling with `-explain`
10+
| ^
11+
| (f : Proc^) cannot be referenced here; it is not included in the allowed capture set {}
12+
| of an enclosing function literal with expected type () -> box () ->? Unit

‎tests/neg-custom-args/captures/exception-definitions.check

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
-- Error: tests/neg-custom-args/captures/exception-definitions.scala:2:6 -----------------------------------------------
22
2 |class Err extends Exception: // error
33
|^
4-
|reference (caps.cap : caps.Root) is not included in allowed capture set {} of pure base class class Throwable
4+
|reference (caps.cap : caps.Root) is not included in the allowed capture set {} of pure base class class Throwable
55
3 | self: Err^ =>
66
-- Error: tests/neg-custom-args/captures/exception-definitions.scala:7:12 ----------------------------------------------
77
7 | val x = c // error
@@ -10,4 +10,4 @@
1010
-- Error: tests/neg-custom-args/captures/exception-definitions.scala:8:8 -----------------------------------------------
1111
8 | class Err3(c: Any^) extends Exception // error
1212
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
13-
| reference (Err3.this.c : Any^) is not included in allowed capture set {} of pure base class class Throwable
13+
| reference (Err3.this.c : Any^) is not included in the allowed capture set {} of pure base class class Throwable

0 commit comments

Comments
 (0)