Skip to content

Commit

Permalink
Merge pull request #518 from adpi2/fix-485
Browse files Browse the repository at this point in the history 
Fix #485: local $outer and public members from private subclass
  • Loading branch information
@adpi2
adpi2 authored Jul 19, 2023
2 parents 78009cc + d850968 commit 2be127b
Show file tree
Hide file tree
Showing 4 changed files with 133 additions and 41 deletions.
1 change: 1 addition & 0 deletions ...expression-compiler/src/main/scala-3/dotty/tools/dotc/evaluation/EvaluationStrategy.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import dotty.tools.dotc.util.Property.*
enum EvaluationStrategy:
case This(cls: ClassSymbol)
case Outer(outerCls: ClassSymbol)
case LocalOuter(outerCls: ClassSymbol) // the $outer param in a constructor
case LocalValue(variable: TermSymbol, isByName: Boolean)
case LocalValueAssign(variable: TermSymbol)
case MethodCapture(variable: TermSymbol, method: TermSymbol, isByName: Boolean)
Expand Down
86 changes: 49 additions & 37 deletions .../expression-compiler/src/main/scala-3/dotty/tools/dotc/evaluation/ExtractExpression.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,12 +100,12 @@ class ExtractExpression(using exprCtx: ExpressionContext) extends MacroTransform
case None => setLocalValue(tree)(variable, rhs)

// inaccessible fields
case tree: Select if isInaccessibleField(tree.symbol) =>
case tree: Select if isInaccessibleField(tree) =>
val qualifier = getTransformedQualifier(tree)
getField(tree)(qualifier, tree.symbol.asTerm)

// assignment to inaccessible fields
case tree @ Assign(lhs, rhs) if isInaccessibleField(lhs.symbol) =>
case tree @ Assign(lhs, rhs) if isInaccessibleField(lhs) =>
val qualifier = getTransformedQualifier(lhs)
setField(tree)(qualifier, lhs.symbol.asTerm, transform(rhs))

Expand All @@ -114,13 +114,13 @@ class ExtractExpression(using exprCtx: ExpressionContext) extends MacroTransform
thisOrOuterValue(tree)(tree.symbol.enclosingClass.asClass)

// inaccessible constructors
case tree: (Select | Apply | TypeApply) if isInaccessibleConstructor(tree.symbol) =>
case tree: (Select | Apply | TypeApply) if isInaccessibleConstructor(tree) =>
val args = getTransformedArgs(tree)
val qualifier = getTransformedQualifierOfNew(tree)
callConstructor(tree)(qualifier, tree.symbol.asTerm, args)

// inaccessible methods
case tree: (Ident | Select | Apply | TypeApply) if isInaccessibleMethod(tree.symbol) =>
case tree: (Ident | Select | Apply | TypeApply) if isInaccessibleMethod(tree) =>
val args = getTransformedArgs(tree)
val qualifier = getTransformedQualifier(tree)
callMethod(tree)(qualifier, tree.symbol.asTerm, args)
Expand All @@ -130,6 +130,38 @@ class ExtractExpression(using exprCtx: ExpressionContext) extends MacroTransform
case tree =>
super.transform(tree)

/**
* The symbol is a field and the expression class cannot access it
* either because it is private or it belongs to an inacessible type
*/
private def isInaccessibleField(tree: Tree)(using Context): Boolean =
val symbol = tree.symbol
symbol.isField &&
symbol.owner.isType &&
!isTermAccessible(symbol.asTerm, getQualifierTypeSymbol(tree))

/**
* The symbol is a real method and the expression class cannot access it
* either because it is private or it belongs to an inaccessible type
*/
private def isInaccessibleMethod(tree: Tree)(using Context): Boolean =
val symbol = tree.symbol
!isOwnedByExpression(symbol) &&
symbol.isRealMethod &&
(!symbol.owner.isType || !isTermAccessible(symbol.asTerm, getQualifierTypeSymbol(tree)))

/**
* The symbol is a constructor and the expression class cannot access it
* either because it is an inaccessible method or it belong to a nested type (not static)
*/
private def isInaccessibleConstructor(tree: Tree)(using
Context
): Boolean =
val symbol = tree.symbol
!isOwnedByExpression(symbol) &&
symbol.isConstructor &&
(isInaccessibleMethod(tree) || !symbol.owner.isStatic)

private def getCapturer(variable: TermSymbol)(using
Context
): Option[Symbol] =
Expand All @@ -148,6 +180,13 @@ class ExtractExpression(using exprCtx: ExpressionContext) extends MacroTransform
case Apply(fun, args) => getTransformedArgs(fun) ++ args.map(transform)
case TypeApply(fun, _) => getTransformedArgs(fun)

private def getQualifierTypeSymbol(tree: Tree)(using Context): TypeSymbol =
tree match
case Ident(_) => tree.symbol.enclosingClass.asClass
case Select(qualifier, _) => qualifier.tpe.widenDealias.typeSymbol.asType
case Apply(fun, _) => getQualifierTypeSymbol(fun)
case TypeApply(fun, _) => getQualifierTypeSymbol(fun)

private def getTransformedQualifier(tree: Tree)(using Context): Tree =
tree match
case Ident(_) =>
Expand Down Expand Up @@ -198,7 +237,8 @@ class ExtractExpression(using exprCtx: ExpressionContext) extends MacroTransform
.drop(1)
.take(target)
.foldLeft(ths) { (innerObj, outerSym) =>
getOuter(tree)(innerObj, outerSym)
if innerObj == ths && exprCtx.localVariables.contains("$outer") then getLocalOuter(tree)(outerSym)
else getOuter(tree)(innerObj, outerSym)
}
else nullLiteral

Expand All @@ -210,6 +250,10 @@ class ExtractExpression(using exprCtx: ExpressionContext) extends MacroTransform
exprCtx.classOwners.head.typeRef
)

private def getLocalOuter(tree: Tree)(outerCls: ClassSymbol)(using Context): Tree =
val strategy = EvaluationStrategy.LocalOuter(outerCls)
reflectEval(tree)(nullLiteral, strategy, List.empty, outerCls.typeRef)

private def getOuter(
tree: Tree
)(qualifier: Tree, outerCls: ClassSymbol)(using
Expand Down Expand Up @@ -375,38 +419,6 @@ class ExtractExpression(using exprCtx: ExpressionContext) extends MacroTransform
!symbol.isRoot &&
!isOwnedByExpression(symbol)

/**
* The symbol is a field and the expression class cannot access it
* either because it is private or it belongs to an inacessible type
*/
private def isInaccessibleField(symbol: Symbol)(using Context): Boolean =
symbol.isField &&
symbol.owner.isType &&
!isTermAccessible(symbol.asTerm, symbol.owner.asType)

/**
* The symbol is a real method and the expression class cannot access it
* either because it is private or it belongs to an inaccessible type
*/
private def isInaccessibleMethod(symbol: Symbol)(using Context): Boolean =
!isOwnedByExpression(symbol) &&
symbol.isRealMethod &&
(!symbol.owner.isType || !isTermAccessible(
symbol.asTerm,
symbol.owner.asType
))

/**
* The symbol is a constructor and the expression class cannot access it
* either because it is an inaccessible method or it belong to a nested type (not static)
*/
private def isInaccessibleConstructor(symbol: Symbol)(using
Context
): Boolean =
!isOwnedByExpression(symbol) &&
symbol.isConstructor &&
(isInaccessibleMethod(symbol) || !symbol.owner.isStatic)

private def isLocalVariable(symbol: Symbol)(using Context): Boolean =
!symbol.is(Method) && symbol.isLocalToBlock && !isOwnedByExpression(symbol)

Expand Down
2 changes: 2 additions & 0 deletions ...expression-compiler/src/main/scala-3/dotty/tools/dotc/evaluation/ResolveReflectEval.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ class ResolveReflectEval(using exprCtx: ExpressionContext) extends MiniPhase:
// but we expect an instance of the value class instead
gen.boxValueClass(cls, gen.getLocalValue("$this"))
else gen.getLocalValue("$this")
case EvaluationStrategy.LocalOuter(cls) =>
gen.getLocalValue("$outer")
case EvaluationStrategy.Outer(outerCls) =>
gen.getOuter(qualifier, outerCls)
case EvaluationStrategy.LocalValue(variable, isByName) =>
Expand Down
85 changes: 81 additions & 4 deletions modules/tests/src/test/scala/ch/epfl/scala/debugadapter/ScalaEvaluationTests.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,12 +171,12 @@ abstract class ScalaEvaluationTests(scalaVersion: ScalaVersion) extends DebugTes
Evaluation.success("B.b3", "b3"),
Evaluation.success("A.B.b3", "b3"),
Evaluation.success("B.b4", "b4"),
Evaluation.success("C")(result => assert(result.startsWith("A$C$@"))),
Evaluation.success("D")(result => assert(result.startsWith("A$D$@"))),
Evaluation.success("C", ObjectRef("A$C$")),
Evaluation.success("D", ObjectRef("A$D$")),
Evaluation.success("F.f1", "f1"),
Evaluation.success("F.f2", "f2"),
Evaluation.success("F.G")(result => assert(result.startsWith("F$G$@"))),
Evaluation.success("F.H")(result => assert(result.startsWith("F$H$@")))
Evaluation.success("F.G", ObjectRef("F$G$")),
Evaluation.success("F.H", ObjectRef("F$H$"))
)
)
}
Expand Down Expand Up @@ -2039,6 +2039,83 @@ abstract class ScalaEvaluationTests(scalaVersion: ScalaVersion) extends DebugTes
Evaluation.success("patch.span", 1)
)
}

test("i485: public members from private subclass") {
val source =
"""|package example
|
|class A {
| val a1 = "a1"
| var a2 = 1
| def m = "m"
| class D
| object D
|}
|
|object Main {
| private class B extends A
|
| def main(args: Array[String]): Unit = {
| val b = new B
| println("foo")
| }
|}
|""".stripMargin
implicit val debuggee: TestingDebuggee = TestingDebuggee.mainClass(source, "example.Main", scalaVersion)
check(
Breakpoint(16),
Evaluation.success("b.a1", "a1"),
Evaluation.success("b.a2 = 2", ()),
Evaluation.success("b.a2", 2),
Evaluation.success("b.m", "m"),
Evaluation.success("new b.D", ObjectRef("A$D")),
Evaluation.success("b.D", ObjectRef("A$D$"))
)
}

test("i485: $outer from contstructor") {
val source =
"""|package example
|
|class A {
| val x = "x"
| class B {
| println(x)
| class C {
| println(x)
| }
| new C
| }
| new B
|}
|
|object Main {
| def main(args: Array[String]): Unit = {
| new A
| }
|}
|""".stripMargin
implicit val debuggee: TestingDebuggee = TestingDebuggee.mainClass(source, "example.Main", scalaVersion)
if (isScala2) {
check(
Breakpoint(6),
Evaluation.success("x", "x"),
Breakpoint(8),
Evaluation.success("x", "x")
)
} else {
check(
Breakpoint(6),
Evaluation.success("x", "x"),
Breakpoint(6),
Evaluation.success("x", "x"),
Breakpoint(8),
Evaluation.success("x", "x"),
Breakpoint(8),
Evaluation.success("x", "x")
)
}
}
}

abstract class Scala2EvaluationTests(val scalaVersion: ScalaVersion) extends ScalaEvaluationTests(scalaVersion) {
Expand Down

0 comments on commit 2be127b

Please sign in to comment.