Skip to content

Commit 3ef8e2f

Browse files
committed
Fix "forward reference from self constructor" checking
The previous check did not consider the case where the self constructor was itself a block that originated from named and default parameters in the actual self constructor call. That gave a false negative for some code in akka. The negative was not discovered before since we did not propagate the correct context information into the expression of a block.
1 parent a6732dc commit 3ef8e2f

File tree

3 files changed

+94
-15
lines changed

3 files changed

+94
-15
lines changed

compiler/src/dotty/tools/dotc/transform/ForwardDepChecks.scala

Lines changed: 35 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,10 @@ object ForwardDepChecks:
2525
}
2626

2727
/** A class to help in forward reference checking */
28-
class LevelInfo(outerLevelAndIndex: LevelAndIndex, stats: List[Tree])(using Context)
28+
class LevelInfo(val outer: OptLevelInfo, val owner: Symbol, stats: List[Tree])(using Context)
2929
extends OptLevelInfo {
3030
override val levelAndIndex: LevelAndIndex =
31-
stats.foldLeft(outerLevelAndIndex, 0) {(mi, stat) =>
31+
stats.foldLeft(outer.levelAndIndex, 0) {(mi, stat) =>
3232
val (m, idx) = mi
3333
val m1 = stat match {
3434
case stat: MemberDef => m.updated(stat.symbol, (this, idx))
@@ -71,7 +71,7 @@ class ForwardDepChecks extends MiniPhase:
7171

7272
override def prepareForStats(trees: List[Tree])(using Context): Context =
7373
if (ctx.owner.isTerm)
74-
ctx.fresh.updateStore(LevelInfo, new LevelInfo(currentLevel.levelAndIndex, trees))
74+
ctx.fresh.updateStore(LevelInfo, new LevelInfo(currentLevel, ctx.owner, trees))
7575
else ctx
7676

7777
override def transformValDef(tree: ValDef)(using Context): ValDef =
@@ -89,19 +89,39 @@ class ForwardDepChecks extends MiniPhase:
8989
tree
9090
}
9191

92-
override def transformApply(tree: Apply)(using Context): Apply = {
93-
if (isSelfConstrCall(tree)) {
94-
assert(currentLevel.isInstanceOf[LevelInfo], s"${ctx.owner}/" + i"$tree")
95-
val level = currentLevel.asInstanceOf[LevelInfo]
96-
if (level.maxIndex > 0) {
97-
// An implementation restriction to avoid VerifyErrors and lazyvals mishaps; see SI-4717
98-
report.debuglog("refsym = " + level.refSym)
99-
report.error("forward reference not allowed from self constructor invocation",
100-
ctx.source.atSpan(level.refSpan))
101-
}
102-
}
92+
/** Check that self constructor call does not contain references to vals or defs
93+
* defined later in the secondary constructor's right hand side. This is tricky
94+
* since the complete self constructor might itself be a block that originated from
95+
* expanding named and default parameters. In that case we have to go outwards
96+
* and find the enclosing expression that consists of that block. Test cases in
97+
* {pos,neg}/complex-self-call.scala.
98+
*/
99+
private def checkSelfConstructorCall()(using Context): Unit =
100+
// Find level info corresponding to constructor's RHS. This is the info of the
101+
// outermost LevelInfo that has the constructor as owner.
102+
def rhsLevelInfo(l: OptLevelInfo): OptLevelInfo = l match
103+
case l: LevelInfo if l.owner == ctx.owner =>
104+
rhsLevelInfo(l.outer) match
105+
case l1: LevelInfo => l1
106+
case _ => l
107+
case _ =>
108+
NoLevelInfo
109+
110+
rhsLevelInfo(currentLevel) match
111+
case level: LevelInfo =>
112+
if level.maxIndex > 0 then
113+
report.debuglog("refsym = " + level.refSym.showLocated)
114+
report.error("forward reference not allowed from self constructor invocation",
115+
ctx.source.atSpan(level.refSpan))
116+
case _ =>
117+
assert(false, s"${ctx.owner.showLocated}")
118+
end checkSelfConstructorCall
119+
120+
override def transformApply(tree: Apply)(using Context): Apply =
121+
if (isSelfConstrCall(tree))
122+
assert(ctx.owner.isConstructor)
123+
checkSelfConstructorCall()
103124
tree
104-
}
105125

106126
override def transformNew(tree: New)(using Context): New = {
107127
currentLevel.enterReference(tree.tpe.typeSymbol, tree.span)

tests/neg/complex-self-call.scala

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// An example extracted from akka that demonstrated a spurious
2+
// "forward reference not allowed from self constructor invocation" error.
3+
class Resizer
4+
class SupervisorStrategy
5+
class Pool
6+
object Pool:
7+
def defaultSupervisorStrategy: SupervisorStrategy = ???
8+
object Dispatchers:
9+
def DefaultDispatcherId = ???
10+
object Resizer:
11+
def fromConfig(config: Config): Option[Resizer] = ???
12+
13+
class Config:
14+
def getInt(str: String): Int = ???
15+
def hasPath(str: String): Boolean = ???
16+
17+
final case class BroadcastPool(
18+
nrOfInstances: Int,
19+
val resizer: Option[Resizer] = None,
20+
val supervisorStrategy: SupervisorStrategy = Pool.defaultSupervisorStrategy,
21+
val routerDispatcher: String = Dispatchers.DefaultDispatcherId,
22+
val usePoolDispatcher: Boolean = false)
23+
extends Pool:
24+
25+
def this(config: Config) =
26+
this(
27+
nrOfInstances = config.getInt("nr-of-instances"),
28+
resizer = resiz, // error: forward reference not allowed from self constructor invocation
29+
usePoolDispatcher = config.hasPath("pool-dispatcher"))
30+
def resiz = Resizer.fromConfig(config)

tests/pos/complex-self-call.scala

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// An example extracted from akka that demonstrated a spurious
2+
// "forward reference not allowed from self constructor invocation" error.
3+
class Resizer
4+
class SupervisorStrategy
5+
class Pool
6+
object Pool:
7+
def defaultSupervisorStrategy: SupervisorStrategy = ???
8+
object Dispatchers:
9+
def DefaultDispatcherId = ???
10+
object Resizer:
11+
def fromConfig(config: Config): Option[Resizer] = ???
12+
13+
class Config:
14+
def getInt(str: String): Int = ???
15+
def hasPath(str: String): Boolean = ???
16+
17+
final case class BroadcastPool(
18+
nrOfInstances: Int,
19+
val resizer: Option[Resizer] = None,
20+
val supervisorStrategy: SupervisorStrategy = Pool.defaultSupervisorStrategy,
21+
val routerDispatcher: String = Dispatchers.DefaultDispatcherId,
22+
val usePoolDispatcher: Boolean = false)
23+
extends Pool:
24+
25+
def this(config: Config) =
26+
this(
27+
nrOfInstances = config.getInt("nr-of-instances"),
28+
resizer = Resizer.fromConfig(config),
29+
usePoolDispatcher = config.hasPath("pool-dispatcher"))

0 commit comments

Comments
 (0)