Merge pull request #683 from adpi2/fix-663

Fix Scala 3 evaluation of capture of by-name arg

Author: adpi2

build.sbt

@@ -194,8 +194,8 @@ lazy val expressionCompiler = projectMatrix
     },
     Test / unmanagedSourceDirectories ++= {
       val sourceDir = (Test / sourceDirectory).value
-      CrossVersion.partialVersion(scalaVersion.value).collect {
-        case (3, x) if x >= 3 => sourceDir / "scala-3.3+"
+      CrossVersion.partialVersion(scalaVersion.value).collect { case (3, 3) =>
+        sourceDir / "scala-3.3"
       }
     },
     Compile / doc / sources := Seq.empty,

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.0/dotty/tools/dotc/ExpressionCompiler.scala

@@ -3,15 +3,18 @@ package dotty.tools.dotc
 import dotty.tools.dotc.core.Contexts.Context
 import dotty.tools.dotc.core.Phases.Phase
 import dotty.tools.dotc.evaluation.*
+import dotty.tools.dotc.transform.ElimByName
 import dotty.tools.dotc.util.SourceFile
 
 class ExpressionCompiler(using ExpressionContext)(using Context) extends Compiler:
 
   override protected def frontendPhases: List[List[Phase]] =
     List(EvaluationFrontEnd()) :: super.frontendPhases.tail
 
-  override protected def picklerPhases: List[List[Phase]] =
-    super.picklerPhases :+ List(ExtractExpression())
-
   override protected def transformPhases: List[List[Phase]] =
-    super.transformPhases :+ List(ResolveReflectEval())
+    // the ExtractExpression phase should be after ElimByName and ExtensionMethods,
+    // and before LambdaLift
+    val transformPhases = super.transformPhases
+    val index = transformPhases.indexWhere(_.exists(_.phaseName == ElimByName.name))
+    val (before, after) = transformPhases.splitAt(index + 1)
+    (before :+ List(ExtractExpression())) ++ (after :+ List(ResolveReflectEval()))

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

@@ -3,15 +3,18 @@ package dotty.tools.dotc
 import dotty.tools.dotc.core.Contexts.Context
 import dotty.tools.dotc.core.Phases.Phase
 import dotty.tools.dotc.evaluation.*
+import dotty.tools.dotc.transform.ElimByName
 
 class ExpressionCompiler(using ExpressionContext)(using Context) extends Compiler:
 
   override protected def frontendPhases: List[List[Phase]] =
     val parser :: others = super.frontendPhases: @unchecked
     parser :: List(InsertExpression()) :: others
 
-  override protected def picklerPhases: List[List[Phase]] =
-    super.picklerPhases :+ List(ExtractExpression())
-
   override protected def transformPhases: List[List[Phase]] =
-    super.transformPhases :+ List(ResolveReflectEval())
+    // the ExtractExpression phase should be after ElimByName and ExtensionMethods,
+    // and before LambdaLift
+    val transformPhases = super.transformPhases
+    val index = transformPhases.indexWhere(_.exists(_.phaseName == ElimByName.name))
+    val (before, after) = transformPhases.splitAt(index + 1)
+    (before :+ List(ExtractExpression())) ++ (after :+ List(ResolveReflectEval()))

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.4/dotty/tools/dotc/ExpressionCompiler.scala

@@ -3,15 +3,18 @@ package dotty.tools.dotc
 import dotty.tools.dotc.core.Contexts.Context
 import dotty.tools.dotc.core.Phases.Phase
 import dotty.tools.dotc.evaluation.*
+import dotty.tools.dotc.transform.ElimByName
 
 class ExpressionCompiler(using ExpressionContext)(using Context) extends Compiler:
 
   override protected def frontendPhases: List[List[Phase]] =
     val parser :: others = super.frontendPhases: @unchecked
     parser :: List(InsertExpression()) :: others
 
-  override protected def picklerPhases: List[List[Phase]] =
-    super.picklerPhases :+ List(ExtractExpression())
-
   override protected def transformPhases: List[List[Phase]] =
-    super.transformPhases :+ List(ResolveReflectEval())
+    // the ExtractExpression phase should be after ElimByName and ExtensionMethods,
+    // and before LambdaLift
+    val transformPhases = super.transformPhases
+    val index = transformPhases.indexWhere(_.exists(_.phaseName == ElimByName.name))
+    val (before, after) = transformPhases.splitAt(index + 1)
+    (before :+ List(ExtractExpression())) ++ (after :+ List(ResolveReflectEval()))

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)