Fix handling of top-level and builtin names

Author: timsueberkrueb

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

@@ -17,6 +17,8 @@ class TestRenamer(names: Names = Names(Map.empty)) extends core.Tree.Rewrite {
 
   // list of scopes that map bound symbols to their renamed variants.
   private var scopes: List[Map[Id, Id]] = List.empty
+  // Top-level items in the current module. Collected to check for free variables.
+  private var toplevelScope: Map[Id, Id] = Map.empty
 
   // Here we track ALL renamings
   var renamed: Map[Id, Id] = Map.empty
@@ -39,27 +41,25 @@ class TestRenamer(names: Names = Names(Map.empty)) extends core.Tree.Rewrite {
   /** Alias for withBindings(List(id)){...} */
   def withBinding[R](id: Id)(f: => R): R = withBindings(List(id))(f)
 
-  // free variables cannot be left untouched, because top-level items may be mutually recursive.
-  // This means that a bound occurrence may precede its binding.
+  // Top-level items may be mutually recursive. This means that a bound occurrence may precede its binding.
+  // We use a separate pass to collect all top-level ids, so that we can distinguish them from free variables.
   override def id: PartialFunction[core.Id, core.Id] = {
-    case id => {
+    case id =>
       if (isBuiltin(id)) {
+        // builtin, do not rename
         id
+      } else if (toplevelScope.contains(id)) {
+        // id references a top-level item
+        toplevelScope(id)
       } else {
         scopes.collectFirst {
+          // locally bound variable
           case bnds if bnds.contains(id) => bnds(id)
         }.getOrElse {
-          scopes match {
-            case Nil => id
-            case _ =>
-              val freshId = freshIdFor(id)
-              val updatedLast = scopes.last + (id -> freshId)
-              scopes = scopes.init :+ updatedLast
-              freshId
-          }
+          // free variable, do not rename
+          id
         }
       }
-    }
   }
 
   override def stmt: PartialFunction[Stmt, Stmt] = {
@@ -229,13 +229,31 @@ class TestRenamer(names: Names = Names(Map.empty)) extends core.Tree.Rewrite {
 
   def apply(m: core.ModuleDecl): core.ModuleDecl =
     suffix = 0
+    scopes = List.empty
     m match {
       case core.ModuleDecl(path, includes, declarations, externs, definitions, exports) =>
+        // Collect toplevel ids, so that we can distinguish a top-level definition bound later
+        // from a free variable when deciding on whether to freshen an id or not.
+        toplevelScope = collectToplevelIds(m).map(id => id -> freshIdFor(id)).toMap
         core.ModuleDecl(path, includes, declarations map rewrite, externs map rewrite, definitions map rewrite, exports map rewrite)
     }
 
   def apply(s: Stmt): Stmt = {
     suffix = 0
+    toplevelScope = Map.empty
+    scopes = List.empty
     rewrite(s)
   }
+
+  def collectToplevelIds(m: core.ModuleDecl): Iterable[Id] =
+    m match {
+    case core.ModuleDecl (path, includes, declarations, externs, definitions, exports) =>
+      declarations.flatMap {
+        case Declaration.Data(id, tparams, constructors) => constructors.map(_.id) :+ id
+        case Interface(id, tparams, properties) => properties.map(_.id) :+ id
+      } ++ definitions.map(_.id) ++ externs.flatMap {
+        case Extern.Def(id, _, _, _, _, _, _, _) => Some(id)
+        case Extern.Include(_, _) => None
+      }
+    }
 }

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

@@ -14,166 +14,229 @@ class TestRenamerTests extends CoreTests {
     val pExpected = parse(renamed, "expected", names)
     val renamer = new TestRenamer(names)
     val obtained = renamer(pInput)
-    shouldBeEqual(obtained, pExpected, clue)
+    val prettyInput = effekt.core.PrettyPrinter.format(pInput).layout
+    assertEquals(prettyInput, renamed)
+    assertEquals(effekt.util.PrettyPrinter.format(pInput).layout, effekt.util.PrettyPrinter.format(pExpected).layout)
+    assertAlphaEquivalent(obtained, pExpected, clue)
   }
 
   test("No bound local variables"){
-    val code =
+    val input =
       """module main
         |
-        |def foo = { () =>
-        |  return (bar: (Int) => Int @ {})(baz:Int)
+        |def foo$1 = { () =>
+        |  return (bar$2: (Int) => Int @ {})(baz$3:Int)
         |}
         |""".stripMargin
-    assertRenamedTo(code, code)
+
+    val expected =
+      """module main
+        |
+        |
+        |
+        |
+        |
+        |
+        |
+        |def foo$1() = {
+        |  return (bar$2: (Int) => Int @ {})(baz$3: Int)
+        |}""".stripMargin
+    assertRenamedTo(input, expected)
   }
 
   test("val binding"){
     val input =
       """module main
         |
-        |def foo = { () =>
-        |  val x = (foo:(Int)=>Int@{})(4) ;
-        |  return x:Int
+        |def foo$1 = { () =>
+        |  val x$2 = (foo$1:(Int)=>Int@{})(4) ;
+        |  return x$2:Int
         |}
         |""".stripMargin
     val expected =
       """module main
         |
-        |def foo = { () =>
-        |  val $1 = (foo:(Int)=>Int@{})(4);
-        |  return $1:Int
-        |}
-        |""".stripMargin
+        |
+        |
+        |
+        |
+        |
+        |
+        |def foo$1() = {
+        |  val x$2: Int = {
+        |    foo$1: (Int) => Int @ {}(4)
+        |  };
+        |  return x$2: Int
+        |}""".stripMargin
     assertRenamedTo(input, expected)
   }
 
   test("var binding"){
     val input =
       """module main
         |
-        |def foo = { () =>
-        |  var x @ global = (foo:(Int)=>Int@{})(4) ;
-        |  return x:Int
+        |def foo$1 = { () =>
+        |  var x$2 @ global = (foo$1:(Int)=>Int@{})(4) ;
+        |  return x$2:Int
         |}
         |""".stripMargin
     val expected =
       """module main
         |
-        |def foo = { () =>
-        |  var $1 @ global = (foo:(Int)=>Int@{})(4);
-        |  return $1:Int
-        |}
-        |""".stripMargin
+        |
+        |
+        |
+        |
+        |
+        |
+        |def foo$1() = {
+        |  var x$2 @ global = (foo$1: (Int) => Int @ {})(4);
+        |  return x$2: Int
+        |}""".stripMargin
     assertRenamedTo(input, expected)
   }
 
   test("function (value) parameters"){
     val input =
       """module main
         |
-        |def foo = { (x:Int) =>
-        |  return x:Int
+        |def foo$1 = { (x$2:Int) =>
+        |  return x$2:Int
         |}
         |""".stripMargin
     val expected =
       """module main
         |
-        |def foo = { ($1:Int) =>
-        |  return $1:Int
-        |}
-        |""".stripMargin
+        |
+        |
+        |
+        |
+        |
+        |
+        |def foo$1(x$2: Int) = {
+        |  return x$2: Int
+        |}""".stripMargin
     assertRenamedTo(input, expected)
   }
 
   test("match clauses"){
     val input =
       """module main
         |
-        |type Data { X(a:Int, b:Int) }
-        |def foo = { () =>
+        |type Data$1 { X$2(a$3:Int, b$3:Int) }
+        |def foo$4 = { () =>
         |  12 match {
-        |    X : {(aa:Int, bb:Int) => return aa:Int }
+        |    X$2 : {(aa$5:Int, bb$6:Int) => return aa$5:Int }
         |  }
         |}
         |""".stripMargin
     val expected =
         """module main
           |
-          |type Data { X(a:Int, b:Int) }
-          |def foo = { () =>
+          |
+          |
+          |type Data$1 {
+          |  X$2(a$3: Int, b$3: Int)
+          |}
+          |
+          |
+          |
+          |def foo$4() = {
           |  12 match {
-          |    X : {($1:Int, $2:Int) => return $1:Int }
+          |    X$2 : { (aa$5: Int, bb$6: Int) =>
+          |      return aa$5: Int
+          |    }
           |  }
-          |}
-          |""".stripMargin
+          |}""".stripMargin
     assertRenamedTo(input, expected)
   }
 
   test("type parameters"){
     val input =
       """module main
         |
-        |def foo = { ['A](a: A) =>
-        |  return a:Identity[A]
+        |def foo$1 = { ['A$2](a$3: A$2) =>
+        |  return a$3: Identity$4[A$2]
         |}
         |""".stripMargin
     val expected =
       """module main
         |
-        |def foo = { ['$1]($2: $1) =>
-        |  return $2:Identity[$1]
-        |}
-        |""".stripMargin
+        |
+        |
+        |
+        |
+        |
+        |
+        |def foo$1['A$2](a$3: A$2) = {
+        |  return a$3: Identity$4[A$2]
+        |}""".stripMargin
     assertRenamedTo(input, expected)
   }
 
   test("pseudo recursive"){
     val input =
       """ module main
         |
-        | def bar = { () => return 1 }
-        | def main = { () =>
-        |   def foo = { () => (bar : () => Unit @ {})() }
-        |   def bar = { () => return 2 }
-        |   (foo : () => Unit @ {})()
+        | def bar$1 = { () => return 1 }
+        | def main$2 = { () =>
+        |   def foo$3 = { () => (bar$1 : () => Unit @ {})() }
+        |   def bar$4 = { () => return 2 }
+        |   (foo$3 : () => Unit @ {})()
         | }
         |""".stripMargin
 
     val expected =
-      """ module main
+      """module main
         |
-        | def bar = { () => return 1 }
-        | def main = { () =>
-        |   def $1 = { () => (bar : () => Unit @ {})() }
-        |   def $2 = { () => return 2 }
-        |   ($1 : () => Unit @ {})()
-        | }
-        |""".stripMargin
+        |
+        |
+        |
+        |
+        |
+        |
+        |def bar$1() = {
+        |  return 1
+        |}
+        |def main$2() = {
+        |  def foo$3() = {
+        |    bar$1: () => Unit @ {}()
+        |  }
+        |  def bar$4() = {
+        |    return 2
+        |  }
+        |  foo$3: () => Unit @ {}()
+        |}""".stripMargin
 
     assertRenamedTo(input, expected)
   }
+
   test("shadowing let bindings"){
-    val input =
+    val code =
       """ module main
         |
-        | def main = { () =>
-        |   let x = 1
-        |   let x = 2
-        |   return x:Int
+        | def main$1 = { () =>
+        |   let x$2 = 1
+        |   let x$3 = 2
+        |   return x$3:Int
         | }
         |""".stripMargin
 
     val expected =
-      """ module main
+      """module main
         |
-        | def main = { () =>
-        |   let $1 = 1
-        |   let $2 = 2
-        |   return $2:Int
-        | }
-        |""".stripMargin
+        |
+        |
+        |
+        |
+        |
+        |
+        |def main$1() = {
+        |  let x$2 = 1
+        |  let x$3 = 2
+        |  return x$3: Int
+        |}""".stripMargin
 
-    assertRenamedTo(input, expected)
+    assertRenamedTo(code, expected)
   }
 }