Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: record calls to constructors in lambdaLift #22487

Merged
merged 2 commits into from
Feb 18, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion compiler/src/dotty/tools/dotc/transform/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ abstract class Dependencies(root: ast.tpd.Tree, @constructorOnly rootContext: Co
if !enclosure.exists then throw NoPath()
if enclosure == sym.enclosure then NoSymbol
else
/** is sym a constructor or a term that is nested in a constructor? */
def nestedInConstructor(sym: Symbol): Boolean =
sym.isConstructor
|| sym.isTerm && nestedInConstructor(sym.enclosure)
Expand Down Expand Up @@ -237,6 +238,10 @@ abstract class Dependencies(root: ast.tpd.Tree, @constructorOnly rootContext: Co
captureImplicitThis(tree.tpe)
case tree: Select =>
if isExpr(sym) && isLocal(sym) then markCalled(sym, enclosure)
case tree: New =>
val constr = tree.tpe.typeSymbol.primaryConstructor
if constr.exists then
symSet(called, enclosure) += constr
case tree: This =>
narrowTo(tree.symbol.asClass)
case tree: MemberDef if isExpr(sym) && sym.owner.isTerm =>
Expand Down Expand Up @@ -291,7 +296,6 @@ abstract class Dependencies(root: ast.tpd.Tree, @constructorOnly rootContext: Co
val calleeOwner = normalizedCallee.owner
if calleeOwner.isTerm then narrowLogicOwner(caller, logicOwner(normalizedCallee))
else
assert(calleeOwner.is(Trait))
// methods nested inside local trait methods cannot be lifted out
// beyond the trait. Note that we can also call a trait method through
// a qualifier; in that case no restriction to lifted owner arises.
Expand Down
4 changes: 1 addition & 3 deletions compiler/src/dotty/tools/dotc/transform/LambdaLift.scala
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,7 @@ object LambdaLift:

private def proxy(sym: Symbol)(using Context): Symbol = {
def liftedEnclosure(sym: Symbol) =
if sym.is(Method)
then deps.logicalOwner.getOrElse(sym, sym.enclosure)
else sym.enclosure
deps.logicalOwner.getOrElse(sym, sym.enclosure)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was the isMethod test dropped?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was just my previous approach for solving this issue, so this is basically a revert. Turns out that picking the enclosure (if there is a logicalOwner set) here just isn't correct in some cases.

def searchIn(enclosure: Symbol): Symbol = {
if (!enclosure.exists) {
def enclosures(encl: Symbol): List[Symbol] =
Expand Down
25 changes: 15 additions & 10 deletions tests/pos/i21931.scala
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
def f() =
val NotFound: Char = 'a'
class crashing() {
class issue() {
NotFound
}
class Module() {
val obligatory =
class anonIssue() {
issue()
object Test {
def f() = {
val NotFound: Char = 'a'
class crashing() {
class issue() {
NotFound
}
class Module() {
val obligatory = {
def anonIssue = {
issue()
}
anonIssue
}
}
}
}
}
17 changes: 17 additions & 0 deletions tests/pos/i22470.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
trait A
trait OuterClass
trait MidClass
trait InnerClass

object Obj:
def outerDef(a: A) =
new OuterClass {
def midDef(): Unit = {
new MidClass {
val valdef = new InnerClass {
def innerDef() =
println(a)
}
}
}
}
Loading