Skip to content

Commit 6a60840

Browse files
authored
bugfix: issue-5283 (repro and fix) (#5412)
Fix for issue #5283. The await translation would crash in obscure cases. Turns out its due to a bug in ir/rename.ml that wasn't properly distinguishing labels from identifiers when renaming them. The counter-example is this code from the repro ``` label satset for (satset in [1,2,4].vals()) { if (4==5) { break satset; } }; ``` This uses a label and identifier with the same name, and unless kept distinct, after broken renaming, would result in the inner break referencing the new identifier for (inner) satset rather than the new label for (outer) satset. The fix is for renaming to use a map with a domain that distinguishes identifers from labels. Even better would be if we didn't use the same type (string, yuck) for identifiers and labels, but that is a bigger refactoring for later.
1 parent 33988d3 commit 6a60840

File tree

6 files changed

+56
-11
lines changed

6 files changed

+56
-11
lines changed

Changelog.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Motoko compiler changelog
22

3+
* motoko (`moc`)
4+
5+
* bugfix: fix rare compiler crash when using a label and identifier of the same name in the same scope (#5283, #5412).
6+
37
## 0.16.0 (2025-08-19)
48

59
* motoko (`moc`)
@@ -20,7 +24,7 @@
2024
Weak reference operations are only supported with --enhanced-orthogonal-persistence and cannot be used with the classic compiler.
2125
2226
* bugfix: the EOP dynamic stable compatibility check incorrectly rejected upgrades from `Null` to `?T` (#5404).
23-
27+
2428
* More explanatory upgrade error messages with detailing of cause (#5391).
2529
2630
* Improved type inference for calling generic functions (#5180).

src/ir_def/ir.ml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ let replace_obj_pat pfs pats =
273273

274274
(* Helper for transforming prims, without missing embedded typs and ids *)
275275

276-
let map_prim t_typ t_id p =
276+
let map_prim t_typ t_lab p =
277277
match p with
278278
| CallPrim ts -> CallPrim (List.map t_typ ts)
279279
| UnPrim (ot, op) -> UnPrim (t_typ ot, op)
@@ -292,7 +292,7 @@ let map_prim t_typ t_id p =
292292
| EqArrayOffset
293293
| DerefArrayOffset
294294
| GetLastArrayOffset -> p
295-
| BreakPrim id -> BreakPrim (t_id id)
295+
| BreakPrim id -> BreakPrim (t_lab id)
296296
| RetPrim
297297
| AwaitPrim _
298298
| AssertPrim

src/ir_def/rename.ml

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,56 @@
11
open Source
22
open Ir
33

4-
module Renaming = Map.Make(String)
4+
(* Identifiers and labels reside in different namespaces
5+
so we need to distinguish them when renaming to avoid capture *)
6+
7+
module Binder = struct
8+
type t = Id of id | Lab of id
9+
let compare t1 t2 =
10+
match t1, t2 with
11+
| (Id id1, Id id2) -> String.compare id1 id2
12+
| (Lab lab1, Lab lab2) -> String.compare lab1 lab2
13+
| (Id _, Lab _) -> -1
14+
| (Lab _, Id _) -> 1
15+
end
16+
17+
module Renaming = Map.Make(Binder)
18+
519

620
(* One traversal for each syntactic category, named by that category *)
721

822
let fresh_id id = Construct.fresh_id id ()
923

1024
let id rho i =
11-
try Renaming.find i rho
12-
with Not_found -> i
25+
match Renaming.find_opt (Binder.Id i) rho with
26+
| Some i1 -> i1
27+
| None -> i
28+
29+
let lab rho l =
30+
match Renaming.find_opt (Binder.Lab l) rho with
31+
| Some l1 -> l1
32+
| None -> l
1333

1434
let id_bind rho i =
1535
let i' = fresh_id i in
16-
(i', Renaming.add i i' rho)
36+
(i', Renaming.add (Binder.Id i) i' rho)
1737

1838
let rec ids_bind rho = function
1939
| [] -> rho
2040
| i::is' ->
2141
let (i', rho') = id_bind rho i in
2242
ids_bind rho' is'
2343

44+
let lab_bind rho i =
45+
let i' = fresh_id i in
46+
(i', Renaming.add (Binder.Lab i) i' rho)
47+
2448
let arg_bind rho a =
25-
let i' = fresh_id a.it in
26-
({a with it = i'}, Renaming.add a.it i' rho)
49+
let i', rho' = id_bind rho a.it in
50+
({a with it = i'}, rho')
2751

2852
let rec prim rho p =
29-
Ir.map_prim Fun.id (id rho) p (* rename BreakPrim id etc *)
53+
Ir.map_prim Fun.id (lab rho) p (* rename BreakPrim id etc *)
3054

3155
and exp rho e = {e with it = exp' rho e.it}
3256
and exp' rho = function
@@ -56,7 +80,7 @@ and exp' rho = function
5680
| IfE (e1, e2, e3) -> IfE (exp rho e1, exp rho e2, exp rho e3)
5781
| SwitchE (e, cs) -> SwitchE (exp rho e, cases rho cs)
5882
| LoopE e1 -> LoopE (exp rho e1)
59-
| LabelE (i, t, e) -> let i',rho' = id_bind rho i in
83+
| LabelE (i, t, e) -> let i', rho' = lab_bind rho i in
6084
LabelE(i', t, exp rho' e)
6185
| AsyncE (s, tb, e, t) -> AsyncE (s, tb, exp rho e, t)
6286
| DeclareE (i, t, e) -> let i',rho' = id_bind rho i in

test/run-drun/issue-5283.mo

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
persistent actor {
2+
3+
class C() = this {
4+
public func m() : async () {
5+
label l {
6+
for (l in [0, 1, 2, 3].values()) {
7+
ignore l == 0;
8+
break l; // label l, not identifier l!
9+
}
10+
}
11+
}
12+
}
13+
14+
};
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ingress Completed: Reply: 0x4449444c016c01b3c4b1f204680100010a00000000000000000101
2+
ingress Completed: Reply: 0x4449444c0000

test/run-drun/ok/issue-5283.tc.ok

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
issue-5283.mo:3.8-3.9: warning [M0194], unused identifier C (delete or rename to wildcard `_` or `_C`)

0 commit comments

Comments
 (0)