Fixes #733 by bringing the rewritten definitions into scope, instead …

…of old ones (#754)

See discussion in #733 for how I traced down the bug. Summary:


At the point where we dealias the binding `l = m`, the core tree looks
like:

```
let l = m;
def b_k_1(r, xs) =
  // note the l here:
  let v_3 = run {link(k, v, l, r)}
  go(bitwiseShl225(level, 1), v_3, xs);
...
```
As it can be clearly seen, `l` is used in the join points `b_k_...`.
The bindings, after rewriting look like this. 
```
let l = m;
def b_k_1(r, xs) =
  // note the m here:
  let v_3 = run {link(k, v, m, r)}
  go(bitwiseShl225(level, 1), v_3, xs);
...
```
It appears the renaming was successful. However, the rewritten defs are
updated in the `InlineContext`, which can be seen by inspecting
`rewrite(List[Definition])`


https://github.com/effekt-lang/effekt/blob/9007b059291153c7d9279ec729d62803a7b47275/effekt/shared/src/main/scala/effekt/core/Inline.scala#L88C7-L97

and its callsite:


https://github.com/effekt-lang/effekt/blob/9007b059291153c7d9279ec729d62803a7b47275/effekt/shared/src/main/scala/effekt/core/Inline.scala#L124-L126

So, the next time a `def` is inlined, the old `def` is inlined. It still
refers to the old, aliased but now removed, binding... Hence causing the
error.

In this PR, I update the defs in the context to now contain the
rewritten (dealiased) right hand sides.

---------

Co-authored-by: Jiří Beneš <mail@jiribenes.com>

effekt/jvm/src/test/scala/effekt/ChezSchemeTests.scala

@@ -56,9 +56,12 @@ abstract class ChezSchemeTests extends EffektTests {
 
     examplesDir / "pos" / "io", // async io is only implemented for monadic JS
 
-    examplesDir / "pos" / "genericcompare.effekt", // genericCompare is only implemented for JS
 
     examplesDir / "pos" / "issue429.effekt",
+
+    // Generic comparison
+    examplesDir / "pos" / "genericcompare.effekt",
+    examplesDir / "pos" / "issue733.effekt",
   )
 }
 

effekt/jvm/src/test/scala/effekt/LLVMTests.scala

@@ -96,6 +96,9 @@ class LLVMTests extends EffektTests {
 
     // Generic equality
     examplesDir / "pos" / "issue429.effekt",
+
+    // Generic comparison
+    examplesDir / "pos" / "issue733.effekt",
   )
 
   override lazy val ignored: List[File] = bugs ++ missingFeatures

effekt/shared/src/main/scala/effekt/core/Inline.scala

@@ -31,6 +31,8 @@ object Inline {
   ) {
     def ++(other: Map[Id, Definition]): InlineContext = InlineContext(usage, defs ++ other, maxInlineSize, inlineCount)
 
+    def ++(other: List[Definition]): InlineContext = ++(other.map(d => d.id -> d).toMap)
+
     def ++=(fresh: Map[Id, Usage]): Unit = { usage ++= fresh }
   }
 
@@ -85,21 +87,27 @@ object Inline {
   def used(id: Id)(using ctx: InlineContext): Boolean =
     ctx.usage.isDefinedAt(id)
 
+  /**
+   * Rewrites the list of definition and returns:
+   * 1. the updated list
+   * 2. definitions
+   *   a. original defnitions: in case we need to dealias elsewhere
+   *   b. the updated definitions, where the rhs might have been dealiased already (see #733)
+   */
   def rewrite(definitions: List[Definition])(using ctx: InlineContext): (List[Definition], InlineContext) =
-    given allDefs: InlineContext = ctx ++ definitions.map(d => d.id -> d).toMap
+    given allDefs: InlineContext = ctx ++ definitions
 
     val filtered = definitions.collect {
       case Definition.Def(id, block) => Definition.Def(id, rewrite(block))
       // we drop aliases
       case Definition.Let(id, tpe, binding) if !binding.isInstanceOf[ValueVar] =>
         Definition.Let(id, tpe, rewrite(binding))
     }
-    (filtered, allDefs)
+    (filtered, allDefs ++ filtered)
 
   def blockDefFor(id: Id)(using ctx: InlineContext): Option[Block] =
     ctx.defs.get(id) map {
-      // TODO rewriting here leads to a stack overflow in one test, why?
-      case Definition.Def(id, block) => block //rewrite(block)
+      case Definition.Def(id, block) => block
       case Definition.Let(id, _, binding) => INTERNAL_ERROR("Should not happen")
     }
 

examples/pos/issue733.effekt

@@ -0,0 +1,259 @@
+effect emit[A](value: A): Unit
+
+namespace newset {
+  record Set[A](internal: newmap::Map[A, Unit])
+
+  def empty[A](): Set[A] = {
+    Set(newmap::empty())
+  }
+
+  def fromList[A](list: List[A]): Set[A] = {
+    val m: newmap::Map[A, Unit] = list.map { k => (k, ()) }.newmap::fromList
+    Set(m)
+  }
+}
+
+namespace newmap {
+  type Map[K, V] {
+    Bin(size: Int, k: K, v: V, left: Map[K, V], right: Map[K, V]);
+    Tip()
+  }
+
+  def empty[K, V](): Map[K, V] = {
+    Tip()
+  }
+
+  def singleton[K, V](k: K, v: V): Map[K, V] = {
+    Bin(1, k, v, Tip(), Tip())
+  }
+
+  def size[K, V](m: Map[K, V]): Int = {
+    m match {
+      case Tip() => 0
+      case Bin(size, _, _, _, _) => size
+    }
+  }
+
+  val ratio = 2
+  val delta = 3
+
+  def bin[K, V](k: K, v: V, l: Map[K, V], r: Map[K, V]): Map[K, V] = {
+    Bin(l.size() + r.size() + 1, k, v, l, r)
+  }
+
+  def balance[K, V](k: K, v: V, l: Map[K, V], r: Map[K, V]): Map[K, V] = {
+    def singleL[A, B](k1: A, v1: B, t1: Map[A, B], m: Map[A, B]): Map[A, B] = {
+      m match {
+        case Bin(_, k2, v2, t2, t3) => bin(k2, v2, bin(k1, v1, t1, t2), t3)
+        case _ => <{ "impossible: singleL: Tip" }>
+      }
+    }
+
+    def singleR[A, B](k1: A, v1: B, m: Map[A, B], t3: Map[A, B]): Map[A, B] = {
+      m match {
+        case Bin(_, k2, v2, t1, t2) => bin(k2, v2, t1, bin(k1, v1, t2, t3))
+        case _ => <{ "impossible: singleR: Tip" }>
+      }
+    }
+
+    def doubleL[A, B](k1: A, v1: B, t1: Map[A, B], m: Map[A, B]): Map[A, B] = {
+      m match {
+        case Bin(_, k2, v2, Bin(_, k3, v3, t2, t3), t4) =>
+          bin(k3, v3, bin(k1, v1, t1, t2), bin(k2, v2, t3, t4))
+        case _ => <{ "impossible: doubleL: Tip" }>
+      }
+    }
+
+    def doubleR[A, B](k1: A, v1: B, m: Map[A, B], t4: Map[A, B]): Map[A, B] = {
+      m match {
+        case Bin(_, k2, v2, t1, Bin(_, k3, v3, t2, t3)) =>
+          bin(k3, v3, bin(k2, v2, t1, t2), bin(k1, v1, t3, t4))
+        case _ =>
+          <{ "impossible: doubleR: Tip" }>
+      }
+    }
+
+    def rotateL[A, B](k: A, v: B, l: Map[A, B], r: Map[A, B]): Map[A, B] = {
+      r match {
+        case Bin(_, _, _, rl, rr) and (rl.size() < ratio * rr.size()) => singleL(k, v, l, r)
+        case _ => doubleL(k, v, l, r)
+      }
+    }
+    def rotateR[A, B](k: A, v: B, l: Map[A, B], r: Map[A, B]): Map[A, B] = {
+      l match {
+        case Bin(_, _, _, ll, lr) and (lr.size() < ratio * ll.size()) => singleR(k, v, l, r)
+        case _ => doubleR(k, v, l, r)
+      }
+    }
+
+    val sizeL = l.size()
+    val sizeR = r.size()
+    val sizeCombined = sizeL + sizeR + 1
+
+    if ((sizeL + sizeR) <= 1) { Bin(sizeCombined, k, v, l, r) }
+    else if (sizeR > (delta * sizeL)) { rotateL(k, v, l, r) }
+    else if (sizeL > (delta * sizeR)) { rotateR(k, v, l, r) }
+    else { Bin(sizeCombined, k, v, l, r)}
+  }
+
+  def put[K, V](m: Map[K, V], k: K, v: V): Map[K, V] = m match {
+    case Tip() => singleton(k, v)
+    case Bin(size, k2, v2, l, r) =>
+      genericCompare(k, k2) match {
+        case Less() => balance(k2, v2, put(l, k, v), r)
+        case Greater() => balance(k2, v2, l, put(r, k, v))
+        case Equal() => Bin(size, k, v, l, r)
+      }
+  }
+
+
+  def putMax[K, V](m: Map[K, V], k: K, v: V): Map[K, V] = {
+    m match {
+      case Tip() => singleton(k, v)
+      case Bin(_, k2, v2, l, r) =>
+        balance(k2, v2, l, r.putMax(k, v))
+    }
+  }
+
+
+  def putMin[K, V](m: Map[K, V], k: K, v: V): Map[K, V] = {
+    m match {
+      case Tip() => singleton(k, v)
+      case Bin(_, k2, v2, l, r) =>
+        balance(k2, v2, l.putMin(k, v), r)
+    }
+  }
+
+  def link[K, V](k: K, v: V, l: Map[K, V], r: Map[K, V]): Map[K, V] = {
+    (l, r) match {
+      case (Tip(), r) => r.putMin(k, v)
+      case (l, Tip()) => l.putMax(k, v)
+      case (Bin(sizeL, kl, vl, ll, lr), Bin(sizeR, kr, vr, rl, rr)) =>
+        if ((delta * sizeL) < sizeR) { balance(kr, vr, link(k, v, l, rl), rr) }
+        else if ((delta * sizeR) < sizeL) { balance(kl, vl, ll, link(k, v, lr, r)) }
+        else { bin(k, v, l, r) }
+    }
+  }
+
+  def fromList[K, V](pairs: List[(K, V)]): Map[K, V] = {
+    pairs match {
+      case Nil() => Tip()
+      case Cons((k, v), Nil()) => singleton(k, v)
+      case Cons((k, v), rest) =>
+        def notOrdered(k: K, pairs: List[(K, V)]) = {
+          pairs match {
+            case Nil() => false
+            case Cons((k2, _), _) =>  // k >= k2
+              genericCompare(k, k2) match {
+                case Less() => false
+                case Greater() => true
+                case Equal() => true
+              }
+          }
+        }
+
+        def insertMany(m: Map[K, V], pairs: List[(K, V)]) = {
+          var mapSoFar = m
+          pairs.foreach { case (k, v) =>
+            mapSoFar = mapSoFar.put(k, v)
+          }
+          mapSoFar
+        }
+
+        def create(level: Int, pairs: List[(K, V)]): (Map[K, V], List[(K, V)], List[(K, V)]) = {
+          pairs match {
+            case Nil() => (Tip(), [], [])
+            case Cons((k, v), rest) =>
+              if (level == 1) {
+                val singleton = Bin(1, k, v, Tip(), Tip())
+                if (notOrdered(k, rest)) {
+                  (singleton, [], rest)
+                } else {
+                  (singleton, rest, [])
+                }
+              } else {
+                val res = create(level.bitwiseShr(1), pairs)
+                res match {
+                  case (_, Nil(), _) => res
+                  case (l, Cons((k2, v2), Nil()), zs) => (l.putMax(k2, v2), [], zs)
+                  case (l, Cons((k2, v2), rest2), _) =>
+                    val xs = Cons((k2, v2), rest2) // @-pattern
+
+                    if (notOrdered(k2, rest2)) { (l, [], xs) }
+                    else {
+                      val (r, zs, ws) = create(level.bitwiseShr(1), rest2);
+                      (link(k2, v2, l, r), zs, ws)
+                    }
+                }
+              }
+          }
+        }
+
+        def go(level: Int, m: Map[K, V], pairs: List[(K, V)]): Map[K, V] = {
+          pairs match {
+            case Nil() => m
+            case Cons((k, v), Nil()) => m.putMax(k, v)
+            case Cons((k, v), rest) =>
+              if (notOrdered(k, rest)) { insertMany(m, pairs) }
+              else {
+                val l = m; // m is the left subtree here
+                val cr = create(level, rest)
+                cr match {
+                  case (r, xs, Nil()) => go(level.bitwiseShl(1), link(k, v, l, r), xs)
+                  case (r, Nil(), ys) => insertMany(link(k, v, l, r), ys)
+                  case _ => panic("create: go: cannot happen, invariant broken!")
+                }
+              }
+          }
+        }
+
+        if (notOrdered(k, rest)) { insertMany(singleton(k, v), rest) }
+        else { go(1, singleton(k, v), rest) }
+    }
+  }
+
+  def foreach[K, V](m: Map[K, V]) { action: (K, V) => Unit }: Unit = {
+    def go(m: Map[K, V]): Unit = {
+      m match {
+        case Tip() => ()
+        case Bin(_, k, v, l, r) =>
+          go(l)
+          action(k, v)
+          go(r)
+      }
+    }
+    go(m)
+  }
+
+  def keys[K, V](m: Map[K, V]): List[K] = {
+    var acc = Nil()
+    m.foreach { (k, _v) =>
+      acc = Cons(k, acc)
+    }
+    acc.reverse
+  }
+}
+
+def collectMap[K, V, R] { stream: () => R / emit[(K, V)] }: (R, newmap::Map[K, V]) =
+  try {
+    (stream(), newmap::empty())
+  } with emit[(K, V)] { case (k, v) =>
+    val (r, map) = resume(());
+    (r, map.newmap::put(k, v))
+  }
+
+def collectMap[K, V] { stream: () => Unit / emit[(K, V)] }: newmap::Map[K, V] =
+  collectMap[K, V, Unit]{stream}.second
+
+///// === the actual problem ===
+
+def main(): Unit = {
+  val m = collectMap[Int, Char] {
+    do emit((42, 'a'))
+    do emit((1, 'b'))
+    do emit((-6, 'c')) // comment this out to make everything work again, ...
+  }
+  val ignored = newset::fromList(m.newmap::keys)
+  // ... or comment this ^^^ out!
+  ()
+}