Refactoring: Introduce LocalScope{Transfomer,Traverser}.

sjrd · sjrd · commit 23032648f150 · 2024-12-29T13:00:33.000+01:00
In several places, we transform or traverse (part of) a method body
in a way that is dependent on the local scope. In those situations,
we must stop at `Closure` boundaries.

We now introduce dedicated subclasses that handle this particular
behavior. This factors out the correct handling of `Closure`s, and
at the same time makes the intent clearer.

One use of `Traverser` in `OptimizerCore` did not handle `Closure`s
in a special way, but it should also be about the local scope.
diff --git a/compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala b/compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala
@@ -1108,16 +1108,10 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
         }
 
         // After the super call, substitute `selfRef` for `this`
-        val afterSuper = new ir.Transformers.Transformer {
+        val afterSuper = new ir.Transformers.LocalScopeTransformer {
           override def transform(tree: js.Tree): js.Tree = tree match {
             case js.This() =>
               selfRef(tree.pos)
-
-            // Don't traverse closure boundaries
-            case closure: js.Closure =>
-              val newCaptureValues = transformTrees(closure.captureValues)
-              closure.copy(captureValues = newCaptureValues)(closure.pos)
-
             case tree =>
               super.transform(tree)
           }
@@ -2171,14 +2165,11 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
       } yield {
         js.ParamDef(name, originalName, ptpe, newMutable(name.name, mutable))(p.pos)
       }
-      val transformer = new ir.Transformers.Transformer {
+      val transformer = new ir.Transformers.LocalScopeTransformer {
         override def transform(tree: js.Tree): js.Tree = tree match {
           case js.VarDef(name, originalName, vtpe, mutable, rhs) =>
             super.transform(js.VarDef(name, originalName, vtpe,
                 newMutable(name.name, mutable), rhs)(tree.pos))
-          case js.Closure(arrow, captureParams, params, restParam, body, captureValues) =>
-            js.Closure(arrow, captureParams, params, restParam, body,
-                transformTrees(captureValues))(tree.pos)
           case _ =>
             super.transform(tree)
         }
@@ -2207,13 +2198,10 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
       } yield {
         js.ParamDef(name, originalName, newType(name.name, ptpe), mutable)(p.pos)
       }
-      val transformer = new ir.Transformers.Transformer {
+      val transformer = new ir.Transformers.LocalScopeTransformer {
         override def transform(tree: js.Tree): js.Tree = tree match {
           case tree @ js.VarRef(name) =>
             js.VarRef(name)(newType(name, tree.tpe))(tree.pos)
-          case js.Closure(arrow, captureParams, params, restParam, body, captureValues) =>
-            js.Closure(arrow, captureParams, params, restParam, body,
-                transformTrees(captureValues))(tree.pos)
           case _ =>
             super.transform(tree)
         }
diff --git a/ir/shared/src/main/scala/org/scalajs/ir/Transformers.scala b/ir/shared/src/main/scala/org/scalajs/ir/Transformers.scala
@@ -277,4 +277,19 @@ object Transformers {
     }
   }
 
+  /** Transformer that only transforms in the local scope.
+   *
+   *  In practice, this means stopping at `Closure` boundaries: their
+   *  `captureValues` are transformed, but not their other members.
+   */
+  abstract class LocalScopeTransformer extends Transformer {
+    override def transform(tree: Tree): Tree = tree match {
+      case Closure(arrow, captureParams, params, restParam, body, captureValues) =>
+        Closure(arrow, captureParams, params, restParam, body,
+            transformTrees(captureValues))(tree.pos)
+      case _ =>
+        super.transform(tree)
+    }
+  }
+
 }
diff --git a/ir/shared/src/main/scala/org/scalajs/ir/Traversers.scala b/ir/shared/src/main/scala/org/scalajs/ir/Traversers.scala
@@ -245,4 +245,18 @@ object Traversers {
     }
   }
 
+  /** Traverser that only traverses the local scope.
+   *
+   *  In practice, this means stopping at `Closure` boundaries: their
+   *  `captureValues` are traversed, but not their other members.
+   */
+  abstract class LocalScopeTraverser extends Traverser {
+    override def traverse(tree: Tree): Unit = tree match {
+      case Closure(_, _, _, _, _, captureValues) =>
+        captureValues.foreach(traverse(_))
+      case _ =>
+        super.traverse(tree)
+    }
+  }
+
 }
diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala
@@ -1713,7 +1713,7 @@ private[optimizer] abstract class OptimizerCore(
   private def tryInsertAtFirstEvalContext(valName: LocalName, valTree: Tree, body: Tree): Option[Tree] = {
     import EvalContextInsertion._
 
-    object valTreeInfo extends Traversers.Traverser {
+    object valTreeInfo extends Traversers.LocalScopeTraverser {
       val mutatedLocalVars = mutable.Set.empty[LocalName]
 
       traverse(valTree)

Original file line number	Diff line number	Diff line change
`@@ -277,4 +277,19 @@ object Transformers {`
`277`	`277`	`}`
`278`	`278`	`}`
`279`	`279`
	`280`	`+ /** Transformer that only transforms in the local scope.`
	`281`	`+ *`
	`282`	+ * In practice, this means stopping at `Closure` boundaries: their
	`283`	+ * `captureValues` are transformed, but not their other members.
	`284`	`+ */`
	`285`	`+ abstract class LocalScopeTransformer extends Transformer {`
	`286`	`+ override def transform(tree: Tree): Tree = tree match {`
	`287`	`+ case Closure(arrow, captureParams, params, restParam, body, captureValues) =>`
	`288`	`+ Closure(arrow, captureParams, params, restParam, body,`
	`289`	`+ transformTrees(captureValues))(tree.pos)`
	`290`	`+ case _ =>`
	`291`	`+ super.transform(tree)`
	`292`	`+ }`
	`293`	`+ }`
	`294`	`+`
`280`	`295`	`}`
Original file line number	Diff line number	Diff line change
`@@ -245,4 +245,18 @@ object Traversers {`
`245`	`245`	`}`
`246`	`246`	`}`
`247`	`247`
	`248`	`+ /** Traverser that only traverses the local scope.`
	`249`	`+ *`
	`250`	+ * In practice, this means stopping at `Closure` boundaries: their
	`251`	+ * `captureValues` are traversed, but not their other members.
	`252`	`+ */`
	`253`	`+ abstract class LocalScopeTraverser extends Traverser {`
	`254`	`+ override def traverse(tree: Tree): Unit = tree match {`
	`255`	`+ case Closure(_, _, _, _, _, captureValues) =>`
	`256`	`+ captureValues.foreach(traverse(_))`
	`257`	`+ case _ =>`
	`258`	`+ super.traverse(tree)`
	`259`	`+ }`
	`260`	`+ }`
	`261`	`+`
`248`	`262`	`}`