Fix Scala 3 evaluation in value class

Author: adpi2

modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/JdiNull.scala

@@ -0,0 +1,3 @@
+package ch.epfl.scala.debugadapter.internal.evaluator
+
+object JdiNull extends JdiObject(null, null)

modules/core/src/main/scala/ch/epfl/scala/debugadapter/internal/evaluator/ScalaEvaluator.scala

@@ -53,7 +53,8 @@ private[internal] class ScalaEvaluator(
       (names, values) <- extractValuesAndNames(classLoader)
       namesArray <- classLoader.createArray("java.lang.String", names)
       valuesArray <- classLoader.createArray("java.lang.Object", values)
-      args = List(namesArray, valuesArray)
+      thisObject = frame.thisObject.getOrElse(JdiNull)
+      args = List(thisObject, namesArray, valuesArray)
       expressionInstance <- createExpressionInstance(classLoader, classDir, className, args)
       evaluatedValue <- evaluateExpression(expressionInstance)
       _ <- updateVariables(valuesArray)
@@ -94,10 +95,7 @@ private[internal] class ScalaEvaluator(
   private def extractValuesAndNames(classLoader: JdiClassLoader): Safe[(Seq[JdiString], Seq[JdiValue])] = {
     def extractVariablesFromFrame(): Safe[(Seq[JdiString], Seq[JdiValue])] = {
       val localVariables = frame.variablesAndValues().map { case (variable, value) => (variable.name, value) }
-      // Exclude the this object if there already is a local $this variable
-      // The Scala compiler uses `$this` in the extension methods of AnyVal classes
-      val thisObject = frame.thisObject.filter(_ => !localVariables.contains("$this")).map("$this".->)
-      (localVariables ++ thisObject)
+      localVariables
         .map { case (name, value) =>
           for {
             name <- classLoader.mirrorOf(name)
@@ -109,26 +107,22 @@ private[internal] class ScalaEvaluator(
         .map(xs => (xs.map(_._1), xs.map(_._2)))
     }
     // Only useful in Scala 2
-    def extractFields(thisObject: JdiObject): Safe[(Seq[JdiString], Seq[JdiValue])] = {
-      val fields = thisObject.fields
-      val names = fields.map(_._1).map(classLoader.mirrorOf).traverse
-      val values = fields.map(_._2).map(value => classLoader.boxIfPrimitive(value)).traverse
+    def extractThisAndFields(thisObject: JdiObject): Safe[(Seq[JdiString], Seq[JdiValue])] = {
+      val thisAndFieds = ("$this", thisObject) +: thisObject.fields
+      val names = thisAndFieds.map(_._1).map(classLoader.mirrorOf).traverse
+      val values = thisAndFieds.map(_._2).map(value => classLoader.boxIfPrimitive(value)).traverse
       Safe.join(names, values)
     }
 
     for {
       (variableNames, variableValues) <- extractVariablesFromFrame()
-      // Currently we only need to load the fields for the Scala 2
-      // expression evaluator.
-      // It is dangerous because local values can shadow fields
-      // TODO: adapt Scala 2 expression compiler
+      // Currently we need to load the this object and the fields for the Scala 2 expression evaluator.
+      // It is dangerous because local values can shadow them
+      // TODO: rewrite Scala 2 expression compiler based on Scala 3
       (fieldNames, fieldValues) <- frame.thisObject
         .filter(_ => compiler.scalaVersion.isScala2)
-        .map(extractFields)
+        .map(extractThisAndFields)
         .getOrElse(Safe((Nil, Nil)))
-      // If `this` is a value class then the breakpoint must be in
-      // a generated method of its companion object which takes
-      // the erased value`$this` as argument.
     } yield {
       val names = variableNames ++ fieldNames
       val values = variableValues ++ fieldValues

modules/expression-compiler/src/main/scala-2/scala/tools/nsc/InsertExpression.scala

@@ -23,7 +23,7 @@ class InsertExpression(override val global: ExpressionGlobal) extends Transform
    */
   private class Inserter extends Transformer {
     private val expressionClassSource =
-      s"""class $expressionClassName(names: Array[String], values: Array[Object]) {
+      s"""class $expressionClassName(thisObject: Any, names: Array[String], values: Array[Object]) {
          |  val valuesByName = names.reverse.map(_.asInstanceOf[String]).zip(values.reverse).toMap
          |
          |  def evaluate() = {

modules/expression-compiler/src/main/scala-3.3/dotty/tools/dotc/ExpressionReporter.scala

@@ -3,10 +3,14 @@ package dotty.tools.dotc
 import dotty.tools.dotc.core.Contexts.*
 import dotty.tools.dotc.reporting.AbstractReporter
 import dotty.tools.dotc.reporting.Diagnostic
+import scala.util.matching.Regex
 
 class ExpressionReporter(reportError: String => Unit) extends AbstractReporter:
   override def doReport(dia: Diagnostic)(using Context): Unit =
-    // println(messageAndPos(dia))
+    // When printing trees, we trim what comes after 'def evaluate'
+    // val message = messageAndPos(dia)
+    // val shortMessage = message.split(Regex.quote("\u001b[33mdef\u001b[0m \u001b[36mgetLocalValue\u001b[0m"))(0)
+    // println(shortMessage)
     dia match
       case error: Diagnostic.Error =>
         val newPos = error.pos.source.positionInUltimateSource(error.pos)

modules/expression-compiler/src/main/scala-3/dotty/tools/dotc/ExpressionCompilerBridge.scala

@@ -29,7 +29,7 @@ class ExpressionCompilerBridge:
       classPath,
       "-Yskip:pureStats"
       // Debugging: Print the tree after phases of the debugger
-      // "-Vprint:typer,extract-expression,resolve-reflect-eval",
+      // "-Vprint:elimByName,extract-expression,resolve-reflect-eval",
     ) ++ options :+ sourceFile.toString
     val exprCtx =
       ExpressionContext(expressionClassName, line, expression, localVariables.asScala.toSet, pckg, testMode)

modules/expression-compiler/src/main/scala-3/dotty/tools/dotc/evaluation/InsertExpression.scala

@@ -30,13 +30,15 @@ class InsertExpression(using exprCtx: ExpressionContext) extends Phase:
   override def isCheckable: Boolean = false
 
   private val evaluationClassSource =
-    s"""|class ${exprCtx.expressionClassName}(names: Array[String], values: Array[Any]):
+    s"""|class ${exprCtx.expressionClassName}(thisObject: Any, names: Array[String], values: Array[Any]):
         |  import java.lang.reflect.InvocationTargetException
         |  val classLoader = getClass.getClassLoader.nn
         |
         |  def evaluate(): Any =
         |    ()
         |
+        |  def getThisObject(): Any = thisObject
+        |
         |  def getLocalValue(name: String): Any =
         |    val idx = names.indexOf(name)
         |    if idx == -1 then throw new NoSuchElementException(name)

modules/expression-compiler/src/main/scala-3/dotty/tools/dotc/evaluation/ResolveReflectEval.scala

@@ -42,12 +42,9 @@ class ResolveReflectEval(using exprCtx: ExpressionContext) extends MiniPhase:
           val args = argsTree.asInstanceOf[JavaSeqLiteral].elems
           val gen = new Gen(reflectEval)
           tree.attachment(EvaluationStrategy) match
-            case EvaluationStrategy.This(cls) =>
-              gen.getLocalValue("$this")
-            case EvaluationStrategy.LocalOuter(cls) =>
-              gen.getLocalValue("$outer")
-            case EvaluationStrategy.Outer(outerCls) =>
-              gen.getOuter(qualifier, outerCls)
+            case EvaluationStrategy.This(cls) => gen.getThisObject
+            case EvaluationStrategy.LocalOuter(cls) => gen.getLocalValue("$outer")
+            case EvaluationStrategy.Outer(outerCls) => gen.getOuter(qualifier, outerCls)
             case EvaluationStrategy.LocalValue(variable, isByName) =>
               val variableName = JavaEncoding.encode(variable.name)
               val rawLocalValue = gen.getLocalValue(variableName)
@@ -109,8 +106,7 @@ class ResolveReflectEval(using exprCtx: ExpressionContext) extends MiniPhase:
               val typeSymbol = variable.info.typeSymbol
               val elemField = typeSymbol.info.decl(termName("elem")).symbol
               gen.setField(capture, elemField.asTerm, value)
-            case EvaluationStrategy.StaticObject(obj) =>
-              gen.getStaticObject(obj)
+            case EvaluationStrategy.StaticObject(obj) => gen.getStaticObject(obj)
             case EvaluationStrategy.Field(field, isByName) =>
               // if the field is lazy, if it is private in a value class or a trait
               // then we must call the getter method
@@ -125,15 +121,12 @@ class ResolveReflectEval(using exprCtx: ExpressionContext) extends MiniPhase:
               val arg = gen.unboxIfValueClass(field, args.head)
               if field.owner.is(Trait) then gen.callMethod(qualifier, field.setter.asTerm, List(arg))
               else gen.setField(qualifier, field, arg)
-            case EvaluationStrategy.MethodCall(method) =>
-              gen.callMethod(qualifier, method, args)
-            case EvaluationStrategy.ConstructorCall(ctr, cls) =>
-              gen.callConstructor(qualifier, ctr, args)
+            case EvaluationStrategy.MethodCall(method) => gen.callMethod(qualifier, method, args)
+            case EvaluationStrategy.ConstructorCall(ctr, cls) => gen.callConstructor(qualifier, ctr, args)
         case _ => super.transform(tree)
 
   private def isReflectEval(symbol: Symbol)(using Context): Boolean =
-    symbol.name == termName("reflectEval") &&
-      symbol.owner == exprCtx.expressionClass
+    symbol.name == termName("reflectEval") && symbol.owner == exprCtx.expressionClass
 
   class Gen(reflectEval: Apply)(using Context):
     private val expressionThis = reflectEval.fun.asInstanceOf[Select].qualifier
@@ -172,6 +165,9 @@ class ResolveReflectEval(using exprCtx: ExpressionContext) extends MiniPhase:
       val unboxMethod = ValueClasses.valueClassUnbox(cls).asTerm
       callMethod(tree, unboxMethod, Nil)
 
+    def getThisObject: Tree =
+      Apply(Select(expressionThis, termName("getThisObject")), List.empty)
+
     def getLocalValue(name: String): Tree =
       Apply(
         Select(expressionThis, termName("getLocalValue")),
@@ -323,21 +319,19 @@ class ResolveReflectEval(using exprCtx: ExpressionContext) extends MiniPhase:
 
     private def capturedValue(sym: Symbol, originalName: TermName): Option[Tree] =
       val encodedName = JavaEncoding.encode(originalName)
-      if exprCtx.classOwners.contains(sym)
-      then capturedByClass(sym.asClass, originalName)
+      if exprCtx.classOwners.contains(sym) then capturedByClass(sym.asClass, originalName)
+      else if exprCtx.localVariables.contains(encodedName) then Some(getLocalValue(encodedName))
       else
-      // if the captured value is not a local variables
-      // then it must have been captured by the outer method
-      if exprCtx.localVariables.contains(encodedName)
-      then Some(getLocalValue(encodedName))
-      else exprCtx.capturingMethod.flatMap(getMethodCapture(_, originalName))
+        // if the captured value is not a local variables
+        // then it must have been captured by the outer method
+        exprCtx.capturingMethod.flatMap(getMethodCapture(_, originalName))
 
     private def capturedByClass(cls: ClassSymbol, originalName: TermName): Option[Tree] =
       val target = exprCtx.classOwners.indexOf(cls)
       val qualifier = exprCtx.classOwners
         .drop(1)
         .take(target)
-        .foldLeft(getLocalValue("$this"))((q, cls) => getOuter(q, cls))
+        .foldLeft(getThisObject)((q, cls) => getOuter(q, cls))
       getClassCapture(qualifier, originalName, cls)
 
 object ResolveReflectEval:

modules/expression-compiler/src/test/scala-3.3/ch/epfl/scala/debugadapter/ExpressionCompilerDebug.scala

@@ -34,30 +34,6 @@ class ExpressionCompilerDebug extends munit.FunSuite:
     evaluate(6, "msg", localVariables = Set("msg$1"))
   }
 
-  test("local method in value class".only) {
-    val source =
-      """|package example
-         |
-         |class A(val self: String) extends AnyVal {
-         |  def m(size: Int): String = {
-         |    def m(mul: Int): String = {
-         |      self.take(size) * mul
-         |    }
-         |    m(2)
-         |  }
-         |}
-         |
-         |object Main {
-         |  def main(args: Array[String]): Unit = {
-         |    val a = new A("foo")
-         |    println(a.m(2))
-         |  }
-         |}
-         |""".stripMargin
-    implicit val debuggee: TestingDebuggee = TestingDebuggee.mainClass(source, "example.Main", scalaVersion)
-    evaluate(8, "this.m(2)", localVariables = Set("$this"))
-  }
-
   private def evaluate(line: Int, expression: String, localVariables: Set[String] = Set.empty)(using
       debuggee: TestingDebuggee
   ): Unit =

modules/tests/src/test/scala/ch/epfl/scala/debugadapter/ScalaEvaluationTests.scala

@@ -551,7 +551,7 @@ abstract class ScalaEvaluationTests(scalaVersion: ScalaVersion) extends DebugTes
       Breakpoint(11),
       DebugStepAssert.inParallel(
         // B captures the local value x1
-        Evaluation.successOrIgnore("new B", isScala2)(result => assert(result.startsWith("A$B$1@"))),
+        Evaluation.successOrIgnore("new B", ObjectRef("A$B$1"), isScala2),
         // x1 is captured by B
         Evaluation.successOrIgnore("x1", "x1", isScala2),
         Evaluation.success("x2", "x2"),
@@ -1363,10 +1363,10 @@ abstract class ScalaEvaluationTests(scalaVersion: ScalaVersion) extends DebugTes
       ),
       Breakpoint(6),
       DebugStepAssert.inParallel(
-        Evaluation.success("self", "foo"),
-        Evaluation.success("size", 2),
-        // Evaluation.success("m(1)", "fo"),
-        Evaluation.success("this.m(1)", "ff")
+        Evaluation.successOrIgnore("self", "foo", isScala2),
+        Evaluation.successOrIgnore("size", 2, isScala2),
+        Evaluation.successOrIgnore("m(1)", "fo", isScala2),
+        Evaluation.successOrIgnore("this.m(1)", "ff", isScala2)
       )
     )
   }