Skip to content

Commit 69471e5

Browse files
b-studiosjiribenes
andauthored
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š <[email protected]>
1 parent 9007b05 commit 69471e5

File tree

5 files changed

+278
-5
lines changed

5 files changed

+278
-5
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,12 @@ abstract class ChezSchemeTests extends EffektTests {
5656

5757
examplesDir / "pos" / "io", // async io is only implemented for monadic JS
5858

59-
examplesDir / "pos" / "genericcompare.effekt", // genericCompare is only implemented for JS
6059

6160
examplesDir / "pos" / "issue429.effekt",
61+
62+
// Generic comparison
63+
examplesDir / "pos" / "genericcompare.effekt",
64+
examplesDir / "pos" / "issue733.effekt",
6265
)
6366
}
6467

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,9 @@ class LLVMTests extends EffektTests {
9696

9797
// Generic equality
9898
examplesDir / "pos" / "issue429.effekt",
99+
100+
// Generic comparison
101+
examplesDir / "pos" / "issue733.effekt",
99102
)
100103

101104
override lazy val ignored: List[File] = bugs ++ missingFeatures

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ object Inline {
3131
) {
3232
def ++(other: Map[Id, Definition]): InlineContext = InlineContext(usage, defs ++ other, maxInlineSize, inlineCount)
3333

34+
def ++(other: List[Definition]): InlineContext = ++(other.map(d => d.id -> d).toMap)
35+
3436
def ++=(fresh: Map[Id, Usage]): Unit = { usage ++= fresh }
3537
}
3638

@@ -85,21 +87,27 @@ object Inline {
8587
def used(id: Id)(using ctx: InlineContext): Boolean =
8688
ctx.usage.isDefinedAt(id)
8789

90+
/**
91+
* Rewrites the list of definition and returns:
92+
* 1. the updated list
93+
* 2. definitions
94+
* a. original defnitions: in case we need to dealias elsewhere
95+
* b. the updated definitions, where the rhs might have been dealiased already (see #733)
96+
*/
8897
def rewrite(definitions: List[Definition])(using ctx: InlineContext): (List[Definition], InlineContext) =
89-
given allDefs: InlineContext = ctx ++ definitions.map(d => d.id -> d).toMap
98+
given allDefs: InlineContext = ctx ++ definitions
9099

91100
val filtered = definitions.collect {
92101
case Definition.Def(id, block) => Definition.Def(id, rewrite(block))
93102
// we drop aliases
94103
case Definition.Let(id, tpe, binding) if !binding.isInstanceOf[ValueVar] =>
95104
Definition.Let(id, tpe, rewrite(binding))
96105
}
97-
(filtered, allDefs)
106+
(filtered, allDefs ++ filtered)
98107

99108
def blockDefFor(id: Id)(using ctx: InlineContext): Option[Block] =
100109
ctx.defs.get(id) map {
101-
// TODO rewriting here leads to a stack overflow in one test, why?
102-
case Definition.Def(id, block) => block //rewrite(block)
110+
case Definition.Def(id, block) => block
103111
case Definition.Let(id, _, binding) => INTERNAL_ERROR("Should not happen")
104112
}
105113

examples/pos/issue733.check

Whitespace-only changes.

examples/pos/issue733.effekt

Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
effect emit[A](value: A): Unit
2+
3+
namespace newset {
4+
record Set[A](internal: newmap::Map[A, Unit])
5+
6+
def empty[A](): Set[A] = {
7+
Set(newmap::empty())
8+
}
9+
10+
def fromList[A](list: List[A]): Set[A] = {
11+
val m: newmap::Map[A, Unit] = list.map { k => (k, ()) }.newmap::fromList
12+
Set(m)
13+
}
14+
}
15+
16+
namespace newmap {
17+
type Map[K, V] {
18+
Bin(size: Int, k: K, v: V, left: Map[K, V], right: Map[K, V]);
19+
Tip()
20+
}
21+
22+
def empty[K, V](): Map[K, V] = {
23+
Tip()
24+
}
25+
26+
def singleton[K, V](k: K, v: V): Map[K, V] = {
27+
Bin(1, k, v, Tip(), Tip())
28+
}
29+
30+
def size[K, V](m: Map[K, V]): Int = {
31+
m match {
32+
case Tip() => 0
33+
case Bin(size, _, _, _, _) => size
34+
}
35+
}
36+
37+
val ratio = 2
38+
val delta = 3
39+
40+
def bin[K, V](k: K, v: V, l: Map[K, V], r: Map[K, V]): Map[K, V] = {
41+
Bin(l.size() + r.size() + 1, k, v, l, r)
42+
}
43+
44+
def balance[K, V](k: K, v: V, l: Map[K, V], r: Map[K, V]): Map[K, V] = {
45+
def singleL[A, B](k1: A, v1: B, t1: Map[A, B], m: Map[A, B]): Map[A, B] = {
46+
m match {
47+
case Bin(_, k2, v2, t2, t3) => bin(k2, v2, bin(k1, v1, t1, t2), t3)
48+
case _ => <{ "impossible: singleL: Tip" }>
49+
}
50+
}
51+
52+
def singleR[A, B](k1: A, v1: B, m: Map[A, B], t3: Map[A, B]): Map[A, B] = {
53+
m match {
54+
case Bin(_, k2, v2, t1, t2) => bin(k2, v2, t1, bin(k1, v1, t2, t3))
55+
case _ => <{ "impossible: singleR: Tip" }>
56+
}
57+
}
58+
59+
def doubleL[A, B](k1: A, v1: B, t1: Map[A, B], m: Map[A, B]): Map[A, B] = {
60+
m match {
61+
case Bin(_, k2, v2, Bin(_, k3, v3, t2, t3), t4) =>
62+
bin(k3, v3, bin(k1, v1, t1, t2), bin(k2, v2, t3, t4))
63+
case _ => <{ "impossible: doubleL: Tip" }>
64+
}
65+
}
66+
67+
def doubleR[A, B](k1: A, v1: B, m: Map[A, B], t4: Map[A, B]): Map[A, B] = {
68+
m match {
69+
case Bin(_, k2, v2, t1, Bin(_, k3, v3, t2, t3)) =>
70+
bin(k3, v3, bin(k2, v2, t1, t2), bin(k1, v1, t3, t4))
71+
case _ =>
72+
<{ "impossible: doubleR: Tip" }>
73+
}
74+
}
75+
76+
def rotateL[A, B](k: A, v: B, l: Map[A, B], r: Map[A, B]): Map[A, B] = {
77+
r match {
78+
case Bin(_, _, _, rl, rr) and (rl.size() < ratio * rr.size()) => singleL(k, v, l, r)
79+
case _ => doubleL(k, v, l, r)
80+
}
81+
}
82+
def rotateR[A, B](k: A, v: B, l: Map[A, B], r: Map[A, B]): Map[A, B] = {
83+
l match {
84+
case Bin(_, _, _, ll, lr) and (lr.size() < ratio * ll.size()) => singleR(k, v, l, r)
85+
case _ => doubleR(k, v, l, r)
86+
}
87+
}
88+
89+
val sizeL = l.size()
90+
val sizeR = r.size()
91+
val sizeCombined = sizeL + sizeR + 1
92+
93+
if ((sizeL + sizeR) <= 1) { Bin(sizeCombined, k, v, l, r) }
94+
else if (sizeR > (delta * sizeL)) { rotateL(k, v, l, r) }
95+
else if (sizeL > (delta * sizeR)) { rotateR(k, v, l, r) }
96+
else { Bin(sizeCombined, k, v, l, r)}
97+
}
98+
99+
def put[K, V](m: Map[K, V], k: K, v: V): Map[K, V] = m match {
100+
case Tip() => singleton(k, v)
101+
case Bin(size, k2, v2, l, r) =>
102+
genericCompare(k, k2) match {
103+
case Less() => balance(k2, v2, put(l, k, v), r)
104+
case Greater() => balance(k2, v2, l, put(r, k, v))
105+
case Equal() => Bin(size, k, v, l, r)
106+
}
107+
}
108+
109+
110+
def putMax[K, V](m: Map[K, V], k: K, v: V): Map[K, V] = {
111+
m match {
112+
case Tip() => singleton(k, v)
113+
case Bin(_, k2, v2, l, r) =>
114+
balance(k2, v2, l, r.putMax(k, v))
115+
}
116+
}
117+
118+
119+
def putMin[K, V](m: Map[K, V], k: K, v: V): Map[K, V] = {
120+
m match {
121+
case Tip() => singleton(k, v)
122+
case Bin(_, k2, v2, l, r) =>
123+
balance(k2, v2, l.putMin(k, v), r)
124+
}
125+
}
126+
127+
def link[K, V](k: K, v: V, l: Map[K, V], r: Map[K, V]): Map[K, V] = {
128+
(l, r) match {
129+
case (Tip(), r) => r.putMin(k, v)
130+
case (l, Tip()) => l.putMax(k, v)
131+
case (Bin(sizeL, kl, vl, ll, lr), Bin(sizeR, kr, vr, rl, rr)) =>
132+
if ((delta * sizeL) < sizeR) { balance(kr, vr, link(k, v, l, rl), rr) }
133+
else if ((delta * sizeR) < sizeL) { balance(kl, vl, ll, link(k, v, lr, r)) }
134+
else { bin(k, v, l, r) }
135+
}
136+
}
137+
138+
def fromList[K, V](pairs: List[(K, V)]): Map[K, V] = {
139+
pairs match {
140+
case Nil() => Tip()
141+
case Cons((k, v), Nil()) => singleton(k, v)
142+
case Cons((k, v), rest) =>
143+
def notOrdered(k: K, pairs: List[(K, V)]) = {
144+
pairs match {
145+
case Nil() => false
146+
case Cons((k2, _), _) => // k >= k2
147+
genericCompare(k, k2) match {
148+
case Less() => false
149+
case Greater() => true
150+
case Equal() => true
151+
}
152+
}
153+
}
154+
155+
def insertMany(m: Map[K, V], pairs: List[(K, V)]) = {
156+
var mapSoFar = m
157+
pairs.foreach { case (k, v) =>
158+
mapSoFar = mapSoFar.put(k, v)
159+
}
160+
mapSoFar
161+
}
162+
163+
def create(level: Int, pairs: List[(K, V)]): (Map[K, V], List[(K, V)], List[(K, V)]) = {
164+
pairs match {
165+
case Nil() => (Tip(), [], [])
166+
case Cons((k, v), rest) =>
167+
if (level == 1) {
168+
val singleton = Bin(1, k, v, Tip(), Tip())
169+
if (notOrdered(k, rest)) {
170+
(singleton, [], rest)
171+
} else {
172+
(singleton, rest, [])
173+
}
174+
} else {
175+
val res = create(level.bitwiseShr(1), pairs)
176+
res match {
177+
case (_, Nil(), _) => res
178+
case (l, Cons((k2, v2), Nil()), zs) => (l.putMax(k2, v2), [], zs)
179+
case (l, Cons((k2, v2), rest2), _) =>
180+
val xs = Cons((k2, v2), rest2) // @-pattern
181+
182+
if (notOrdered(k2, rest2)) { (l, [], xs) }
183+
else {
184+
val (r, zs, ws) = create(level.bitwiseShr(1), rest2);
185+
(link(k2, v2, l, r), zs, ws)
186+
}
187+
}
188+
}
189+
}
190+
}
191+
192+
def go(level: Int, m: Map[K, V], pairs: List[(K, V)]): Map[K, V] = {
193+
pairs match {
194+
case Nil() => m
195+
case Cons((k, v), Nil()) => m.putMax(k, v)
196+
case Cons((k, v), rest) =>
197+
if (notOrdered(k, rest)) { insertMany(m, pairs) }
198+
else {
199+
val l = m; // m is the left subtree here
200+
val cr = create(level, rest)
201+
cr match {
202+
case (r, xs, Nil()) => go(level.bitwiseShl(1), link(k, v, l, r), xs)
203+
case (r, Nil(), ys) => insertMany(link(k, v, l, r), ys)
204+
case _ => panic("create: go: cannot happen, invariant broken!")
205+
}
206+
}
207+
}
208+
}
209+
210+
if (notOrdered(k, rest)) { insertMany(singleton(k, v), rest) }
211+
else { go(1, singleton(k, v), rest) }
212+
}
213+
}
214+
215+
def foreach[K, V](m: Map[K, V]) { action: (K, V) => Unit }: Unit = {
216+
def go(m: Map[K, V]): Unit = {
217+
m match {
218+
case Tip() => ()
219+
case Bin(_, k, v, l, r) =>
220+
go(l)
221+
action(k, v)
222+
go(r)
223+
}
224+
}
225+
go(m)
226+
}
227+
228+
def keys[K, V](m: Map[K, V]): List[K] = {
229+
var acc = Nil()
230+
m.foreach { (k, _v) =>
231+
acc = Cons(k, acc)
232+
}
233+
acc.reverse
234+
}
235+
}
236+
237+
def collectMap[K, V, R] { stream: () => R / emit[(K, V)] }: (R, newmap::Map[K, V]) =
238+
try {
239+
(stream(), newmap::empty())
240+
} with emit[(K, V)] { case (k, v) =>
241+
val (r, map) = resume(());
242+
(r, map.newmap::put(k, v))
243+
}
244+
245+
def collectMap[K, V] { stream: () => Unit / emit[(K, V)] }: newmap::Map[K, V] =
246+
collectMap[K, V, Unit]{stream}.second
247+
248+
///// === the actual problem ===
249+
250+
def main(): Unit = {
251+
val m = collectMap[Int, Char] {
252+
do emit((42, 'a'))
253+
do emit((1, 'b'))
254+
do emit((-6, 'c')) // comment this out to make everything work again, ...
255+
}
256+
val ignored = newset::fromList(m.newmap::keys)
257+
// ... or comment this ^^^ out!
258+
()
259+
}

0 commit comments

Comments
 (0)