Simplify core.Renamer to produce smaller symbols (#938)

General idea: instead of building
`name_123_109238_190238_109283019283_1092830192838019283` chains, what
if we use the automagic `fresh.next` to get just a darn fresh number?
This will limit the identifiers to stuff like `b_k2_123_10928310283`,
which is--at least to me--a pretty big improvement.

We use the full capabilities of a `Symbol`: it has a `name` (which is
really a `String`) and a fresh `id` that gets assigned lazily. Instead
of changing the name to be `${oldName}_${oldId}` and then putting a
fresh `id` on top of that (to get `${oldName}_${oldId}_${newId}`), we
just make a new `Symbol` with the same name, which forces a new,
_fresher_, `id`.

---------

Co-authored-by: Marcial Gaißert <[email protected]>

Author: jiribenes

effekt/jvm/src/test/scala/effekt/core/CoreTests.scala

@@ -46,14 +46,14 @@ trait CoreTests extends munit.FunSuite {
                             expected: ModuleDecl,
                             clue: => Any = "values are not alpha-equivalent",
                             names: Names = Names(defaultNames))(using Location): Unit = {
-    val renamer = Renamer(names, "$")
+    val renamer = TestRenamer(names, "$")
     shouldBeEqual(renamer(obtained), renamer(expected), clue)
   }
   def assertAlphaEquivalentStatements(obtained: Stmt,
                             expected: Stmt,
                             clue: => Any = "values are not alpha-equivalent",
                             names: Names = Names(defaultNames))(using Location): Unit = {
-    val renamer = Renamer(names, "$")
+    val renamer = TestRenamer(names, "$")
     shouldBeEqual(renamer(obtained), renamer(expected), clue)
   }
   def parse(input: String,

effekt/jvm/src/test/scala/effekt/core/OptimizerTests.scala

@@ -22,7 +22,7 @@ class OptimizerTests extends CoreTests {
     val pExpected = parse(moduleHeader + transformed, "expected", names)
 
     // the parser is not assigning symbols correctly, so we need to run renamer first
-    val renamed = Renamer(names).rewrite(pInput)
+    val renamed = TestRenamer(names).rewrite(pInput)
 
     val obtained = transform(renamed)
     assertAlphaEquivalent(obtained, pExpected, "Not transformed to")

effekt/jvm/src/test/scala/effekt/core/RenamerTests.scala

@@ -1,17 +1,20 @@
 package effekt.core
-import effekt.symbols
 
+/**
+ * This is testing the main/core.Renamer using the test/core.TestRenamer.
+ */
 class RenamerTests extends CoreTests {
 
-  def assertRenamedTo(input: String,
-                      renamed: String,
-                      clue: => Any = "Not renamed to given value",
-                      names: Names = Names(defaultNames))(using munit.Location) = {
+  /**
+   * Check that the renamed input preserves alpha-equivalence using [[assertAlphaEquivalent]]
+   */
+  def assertRenamingPreservesAlpha(input: String,
+                                   clue: => Any = "Not renamed to given value",
+                                   names: Names = Names(defaultNames))(using munit.Location) = {
     val pInput = parse(input, "input", names)
-    val pExpected = parse(renamed, "expected", names)
-    val renamer = new Renamer(names, "renamed") // use "renamed" as prefix so we can refer to it
+    val renamer = new Renamer(names, "renamed")
     val obtained = renamer(pInput)
-    shouldBeEqual(obtained, pExpected, clue)
+    assertAlphaEquivalent(obtained, pInput, clue)
   }
 
   test("No bound local variables"){
@@ -22,69 +25,46 @@ class RenamerTests extends CoreTests {
         |  return (bar: (Int) => Int @ {})(baz:Int)
         |}
         |""".stripMargin
-    assertRenamedTo(code, code)
+    assertRenamingPreservesAlpha(code)
   }
 
   test("val binding"){
-    val input =
+    val code =
       """module main
         |
         |def foo = { () =>
         |  val x = (foo:(Int)=>Int@{})(4) ;
         |  return x:Int
         |}
         |""".stripMargin
-    val expected =
-      """module main
-        |
-        |def foo = { () =>
-        |  val renamed1 = (foo:(Int)=>Int@{})(4);
-        |  return renamed1:Int
-        |}
-        |""".stripMargin
-    assertRenamedTo(input, expected)
+    assertRenamingPreservesAlpha(code)
   }
 
   test("var binding"){
-    val input =
+    val code =
       """module main
         |
         |def foo = { () =>
         |  var x @ global = (foo:(Int)=>Int@{})(4) ;
         |  return x:Int
         |}
         |""".stripMargin
-    val expected =
-      """module main
-        |
-        |def foo = { () =>
-        |  var renamed1 @ global = (foo:(Int)=>Int@{})(4);
-        |  return renamed1:Int
-        |}
-        |""".stripMargin
-    assertRenamedTo(input, expected)
+    assertRenamingPreservesAlpha(code)
   }
 
   test("function (value) parameters"){
-    val input =
+    val code =
       """module main
         |
         |def foo = { (x:Int) =>
         |  return x:Int
         |}
         |""".stripMargin
-    val expected =
-      """module main
-        |
-        |def foo = { (renamed1:Int) =>
-        |  return renamed1:Int
-        |}
-        |""".stripMargin
-    assertRenamedTo(input, expected)
+    assertRenamingPreservesAlpha(code)
   }
 
   test("match clauses"){
-    val input =
+    val code =
       """module main
         |
         |type Data { X(a:Int, b:Int) }
@@ -94,39 +74,22 @@ class RenamerTests extends CoreTests {
         |  }
         |}
         |""".stripMargin
-    val expected =
-        """module main
-          |
-          |type Data { X(a:Int, b:Int) }
-          |def foo = { () =>
-          |  12 match {
-          |    X : {(renamed1:Int, renamed2:Int) => return renamed1:Int }
-          |  }
-          |}
-          |""".stripMargin
-    assertRenamedTo(input, expected)
+    assertRenamingPreservesAlpha(code)
   }
 
   test("type parameters"){
-    val input =
+    val code =
       """module main
         |
         |def foo = { ['A](a: A) =>
         |  return a:Identity[A]
         |}
         |""".stripMargin
-    val expected =
-      """module main
-        |
-        |def foo = { ['renamed1](renamed2: renamed1) =>
-        |  return renamed2:Identity[renamed1]
-        |}
-        |""".stripMargin
-    assertRenamedTo(input, expected)
+    assertRenamingPreservesAlpha(code)
   }
 
   test("pseudo recursive"){
-    val input =
+    val code =
       """ module main
         |
         | def bar = { () => return 1 }
@@ -136,22 +99,10 @@ class RenamerTests extends CoreTests {
         |   (foo : () => Unit @ {})()
         | }
         |""".stripMargin
-
-    val expected =
-      """ module main
-        |
-        | def bar = { () => return 1 }
-        | def main = { () =>
-        |   def renamed1 = { () => (bar : () => Unit @ {})() }
-        |   def renamed2 = { () => return 2 }
-        |   (renamed1 : () => Unit @ {})()
-        | }
-        |""".stripMargin
-
-    assertRenamedTo(input, expected)
+    assertRenamingPreservesAlpha(code)
   }
   test("shadowing let bindings"){
-    val input =
+    val code =
       """ module main
         |
         | def main = { () =>
@@ -160,17 +111,6 @@ class RenamerTests extends CoreTests {
         |   return x:Int
         | }
         |""".stripMargin
-
-    val expected =
-      """ module main
-        |
-        | def main = { () =>
-        |   let renamed1 = 1
-        |   let renamed2 = 2
-        |   return renamed2:Int
-        | }
-        |""".stripMargin
-
-    assertRenamedTo(input, expected)
+    assertRenamingPreservesAlpha(code)
   }
 }

effekt/jvm/src/test/scala/effekt/core/TestRenamer.scala

@@ -0,0 +1,120 @@
+package effekt.core
+
+import effekt.{ core, symbols }
+import effekt.context.Context
+
+/**
+ * Freshens bound names in a given term for tests.
+ * Please use this _only_ for tests. Otherwise, prefer [[effekt.core.Renamer]].
+ *
+ * @param names used to look up a reference by name to resolve to the same symbols.
+ *              This is only used by tests to deterministically rename terms and check for
+ *              alpha-equivalence.
+ * @param prefix if the prefix is empty, the original name will be used as a prefix
+ *
+ * @param C the context is used to copy annotations from old symbols to fresh symbols
+ */
+class TestRenamer(names: Names = Names(Map.empty), prefix: String = "") extends core.Tree.Rewrite {
+
+  // list of scopes that map bound symbols to their renamed variants.
+  private var scopes: List[Map[Id, Id]] = List.empty
+
+  // Here we track ALL renamings
+  var renamed: Map[Id, Id] = Map.empty
+
+  private var suffix: Int = 0
+
+  def freshIdFor(id: Id): Id =
+    suffix = suffix + 1
+    val uniqueName = if prefix.isEmpty then id.name.name + "_" + suffix.toString else prefix + suffix.toString
+    names.idFor(uniqueName)
+
+  def withBindings[R](ids: List[Id])(f: => R): R =
+    val before = scopes
+    try {
+      val newScope = ids.map { x => x -> freshIdFor(x) }.toMap
+      scopes = newScope :: scopes
+      renamed = renamed ++ newScope
+      f
+    } finally { scopes = before }
+
+  /** Alias for withBindings(List(id)){...} */
+  def withBinding[R](id: Id)(f: => R): R = withBindings(List(id))(f)
+
+  // free variables are left untouched
+  override def id: PartialFunction[core.Id, core.Id] = {
+    case id => scopes.collectFirst {
+      case bnds if bnds.contains(id) => bnds(id)
+    }.getOrElse(id)
+  }
+
+  override def stmt: PartialFunction[Stmt, Stmt] = {
+    case core.Def(id, block, body) =>
+      // can be recursive
+      withBinding(id) { core.Def(rewrite(id), rewrite(block), rewrite(body)) }
+
+    case core.Let(id, tpe, binding, body) =>
+      val resolvedBinding = rewrite(binding)
+      withBinding(id) { core.Let(rewrite(id), rewrite(tpe), resolvedBinding, rewrite(body)) }
+
+    case core.Val(id, tpe, binding, body) =>
+      val resolvedBinding = rewrite(binding)
+      withBinding(id) { core.Val(rewrite(id), rewrite(tpe), resolvedBinding, rewrite(body)) }
+
+    case core.Alloc(id, init, reg, body) =>
+      val resolvedInit = rewrite(init)
+      val resolvedReg = rewrite(reg)
+      withBinding(id) { core.Alloc(rewrite(id), resolvedInit, resolvedReg, rewrite(body)) }
+
+    case core.Var(ref, init, capt, body) =>
+      val resolvedInit = rewrite(init)
+      val resolvedCapt = rewrite(capt)
+      withBinding(ref) { core.Var(rewrite(ref), resolvedInit, resolvedCapt, rewrite(body)) }
+
+    case core.Get(id, tpe, ref, capt, body) =>
+      val resolvedRef = rewrite(ref)
+      val resolvedCapt = rewrite(capt)
+      withBinding(id) { core.Get(rewrite(id), rewrite(tpe), resolvedRef, resolvedCapt, rewrite(body)) }
+
+  }
+
+  override def block: PartialFunction[Block, Block] = {
+    case Block.BlockLit(tparams, cparams, vparams, bparams, body) =>
+      withBindings(tparams ++ cparams ++ vparams.map(_.id) ++ bparams.map(_.id)) {
+        Block.BlockLit(tparams map rewrite, cparams map rewrite, vparams map rewrite, bparams map rewrite,
+          rewrite(body))
+      }
+  }
+
+  override def rewrite(o: Operation): Operation = o match {
+    case Operation(name, tparams, cparams, vparams, bparams, body) =>
+      withBindings(tparams ++ cparams ++ vparams.map(_.id) ++ bparams.map(_.id)) {
+        Operation(name,
+          tparams map rewrite,
+          cparams map rewrite,
+          vparams map rewrite,
+          bparams map rewrite,
+          rewrite(body))
+      }
+  }
+
+  def apply(m: core.ModuleDecl): core.ModuleDecl =
+    suffix = 0
+    m match {
+      case core.ModuleDecl(path, includes, declarations, externs, definitions, exports) =>
+        core.ModuleDecl(path, includes, declarations, externs, definitions map rewrite, exports)
+    }
+
+  def apply(s: Stmt): Stmt = {
+    suffix = 0
+    rewrite(s)
+  }
+}
+
+object TestRenamer {
+  def rename(b: Block): Block = Renamer().rewrite(b)
+  def rename(b: BlockLit): (BlockLit, Map[Id, Id]) =
+    val renamer = Renamer()
+    val res = renamer.rewrite(b)
+    (res, renamer.renamed)
+}