Skip to content

Commit 7ac22d7

Browse files
fix in-place set bitwise assignment and del of closure variable
1 parent f806d5f commit 7ac22d7

10 files changed

Lines changed: 76 additions & 31 deletions

File tree

bugs.json

Lines changed: 2 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
"_meta": {
33
"source": "MicroPython tests/basics differential vs CPython 3.13",
44
"verified": "each repro re-run through VM + python3",
5-
"count": 9,
6-
"fixed": "91 bugs fixed and migrated to vm.json (try/finally + with unwinding cluster; 11 builtin/parser/format divergences from rows 1-11 of the low-NEWLINE triage; generator_return, string_format_modulo_int %*d, seq_unpack list targets, class_getattr dunder-on-type, builtin_slice type identity, containment dict/set-led tuple, range sequence-unpack; yield-from-as-expression cluster: gen_yield_from + gen_yield_from_ducktype + gen_yield_from_exc + gen_yield_from_stopped via a parse_atom Yield arm plus a LoadYieldFrom opcode that carries the subiterator return/StopIteration value, 40 regression cases; range_iter_overflow: AFL sig:06 abort on a range whose step increment crossed the i64 edge, fixed by checked_add in the three range walkers (iter_to_vec_for_spread, IterFrame::next_item, IterCursor::Range::next), 10 regression cases; range_iter_truncation: for-loop/comprehension (IterFrame::next_item) and spread/unpack (iter_to_vec_for_spread) yielded Val::int past the 47-bit inline range instead of promoting, routed both through range_int (pub(crate)) so values auto-promote to LongInt, 10 regression cases; range_contains: `x in range(...)` always returned False (contains had a catch-all Ok(false) for Range) — replaced with an O(1) bounds+step check over i128 that also matches integral floats like CPython, 10 regression cases)"
5+
"count": 7,
6+
"fixed": "93 bugs fixed and migrated to vm.json (try/finally + with unwinding cluster; 11 builtin/parser/format divergences from rows 1-11 of the low-NEWLINE triage; generator_return, string_format_modulo_int %*d, seq_unpack list targets, class_getattr dunder-on-type, builtin_slice type identity, containment dict/set-led tuple, range sequence-unpack; yield-from-as-expression cluster: gen_yield_from + gen_yield_from_ducktype + gen_yield_from_exc + gen_yield_from_stopped via a parse_atom Yield arm plus a LoadYieldFrom opcode that carries the subiterator return/StopIteration value, 40 regression cases; range_iter_overflow: AFL sig:06 abort on a range whose step increment crossed the i64 edge, fixed by checked_add in the three range walkers (iter_to_vec_for_spread, IterFrame::next_item, IterCursor::Range::next), 10 regression cases; range_iter_truncation: for-loop/comprehension (IterFrame::next_item) and spread/unpack (iter_to_vec_for_spread) yielded Val::int past the 47-bit inline range instead of promoting, routed both through range_int (pub(crate)) so values auto-promote to LongInt, 10 regression cases; range_contains: `x in range(...)` always returned False (contains had a catch-all Ok(false) for Range) — replaced with an O(1) bounds+step check over i128 that also matches integral floats like CPython, 10 regression cases; set_binop: augmented set `|=`/`&=`/`^=` rebound to a new set instead of mutating in place — added InPlaceBitOr/And/Xor opcodes (parser maps the augmented ops, handle_bitwise routes to set_iop_and_push) that rewrite the left set in place, frozenset still rebinds, 10 regression cases; del_deref: `del` of a closed-over variable cleared only the local slot, not the shared cell, so closures still read the old value — the Del handler now also unbinds the frame's cell for that slot, 10 regression cases)"
77
},
88
"bugs": [
99
{
@@ -14,14 +14,6 @@
1414
"expected": "8",
1515
"actual": "19"
1616
},
17-
{
18-
"id": "del_deref",
19-
"category": "scoping",
20-
"summary": "del of a closed-over (nonlocal) variable does not unbind it; subsequent read returns the old value instead of raising NameError.",
21-
"repro": "def f():\n x = 1\n def g():\n nonlocal x\n try:\n print(x)\n except NameError:\n print(\"NameError\")\n del x\n g()\nf()",
22-
"expected": "NameError",
23-
"actual": "1"
24-
},
2517
{
2618
"id": "gen_yield_from_executing",
2719
"category": "scoping",
@@ -62,14 +54,6 @@
6254
"expected": "[1, 2, 3]",
6355
"actual": "__VMERR__ TypeError: '<' not supported between instances of 'object' and 'object'"
6456
},
65-
{
66-
"id": "set_binop",
67-
"category": "wrong-value",
68-
"summary": "Augmented set assignment (|= etc.) rebinds to a new set instead of mutating in place, so an aliasing identity check is False instead of True.",
69-
"repro": "s1 = s2 = set('abc')\ns1 |= set('ad')\nprint(s1 is s2, len(s1))",
70-
"expected": "True 4",
71-
"actual": "False 4"
72-
},
7357
{
7458
"id": "gen_next_nested_arg",
7559
"category": "stack-corruption",

compiler/src/modules/parser/stmt.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -380,8 +380,15 @@ impl<'src, I: Iterator<Item = Token>> Parser<'src, I> {
380380
self.advance();
381381
self.emit_load_ssa(name.clone());
382382
self.expr();
383-
// `+=` on a name uses the in-place variant so list targets mutate the shared object instead of rebinding (alias-visible, like CPython).
384-
self.chunk.emit(if op == OpCode::Add { OpCode::InPlaceAdd } else { op }, 0);
383+
// In-place variants so list `+=` and set `|=`/`&=`/`^=` mutate the shared object (alias-visible).
384+
let op = match op {
385+
OpCode::Add => OpCode::InPlaceAdd,
386+
OpCode::BitOr => OpCode::InPlaceBitOr,
387+
OpCode::BitAnd => OpCode::InPlaceBitAnd,
388+
OpCode::BitXor => OpCode::InPlaceBitXor,
389+
other => other,
390+
};
391+
self.chunk.emit(op, 0);
385392
self.store_name(name);
386393
false
387394
}

compiler/src/modules/parser/types.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ pub enum OpCode {
4545
Pos,
4646
/* Pushes the value `yield from` produces: the exhausted subiterator's return / StopIteration value. */
4747
LoadYieldFrom,
48+
// Augmented set bitwise `|=` `&=` `^=`: mutate the left set in place.
49+
InPlaceBitOr, InPlaceBitAnd, InPlaceBitXor,
4850
}
4951

5052
// Python builtin name -> (specialised OpCode, `leaves_value_on_stack`).

compiler/src/modules/vm/dispatch.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,8 @@ impl<'a> VM<'a> {
560560
fn dispatch_generic(&mut self, opcode: OpCode, operand: u16, chunk: &SSAChunk, slots: &mut [Val], ip: &mut usize, exc_base: usize) -> Result<Option<Val>, VmErr> {
561561
match opcode {
562562
OpCode::BitAnd | OpCode::BitOr | OpCode::BitXor
563-
| OpCode::BitNot | OpCode::Shl | OpCode::Shr => self.handle_bitwise(opcode, chunk, slots)?,
563+
| OpCode::BitNot | OpCode::Shl | OpCode::Shr
564+
| OpCode::InPlaceBitOr | OpCode::InPlaceBitAnd | OpCode::InPlaceBitXor => self.handle_bitwise(opcode, chunk, slots)?,
564565
OpCode::In | OpCode::NotIn | OpCode::Is | OpCode::IsNot => self.handle_identity(opcode, chunk, slots)?,
565566

566567
OpCode::BuildList | OpCode::BuildTuple | OpCode::BuildDict

compiler/src/modules/vm/handlers/arith.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,14 @@ impl<'a> VM<'a> {
269269

270270
/* i128 bitwise + Shl/Shr (overflow trap); BitNot unary. Set/Set on |/&/^ means union/intersection/symmetric-diff; other types use the bitwise path. */
271271
pub(crate) fn handle_bitwise(&mut self, op: OpCode, chunk: &SSAChunk, slots: &mut [Val]) -> Result<(), VmErr> {
272+
// Augmented set bitwise reuses the plain path but mutates the left set in place.
273+
let inplace = matches!(op, OpCode::InPlaceBitOr | OpCode::InPlaceBitAnd | OpCode::InPlaceBitXor);
274+
let op = match op {
275+
OpCode::InPlaceBitOr => OpCode::BitOr,
276+
OpCode::InPlaceBitAnd => OpCode::BitAnd,
277+
OpCode::InPlaceBitXor => OpCode::BitXor,
278+
other => other,
279+
};
272280
if op == OpCode::BitNot {
273281
let v = self.pop()?;
274282
let i = self.as_i128(v).ok_or(cold_type("~ requires an integer"))?;
@@ -289,7 +297,7 @@ impl<'a> VM<'a> {
289297

290298
if self.is_set_like(a) && self.is_set_like(b)
291299
&& matches!(op, OpCode::BitAnd | OpCode::BitOr | OpCode::BitXor) {
292-
return self.set_binop_and_push(a, b, op);
300+
return if inplace { self.set_iop_and_push(a, b, op) } else { self.set_binop_and_push(a, b, op) };
293301
}
294302
// `dict | dict` (and `|=`) merges, right operand winning.
295303
if op == OpCode::BitOr && a.is_heap() && b.is_heap()

compiler/src/modules/vm/handlers/data.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,12 @@ impl<'a> VM<'a> {
199199
if core::ptr::eq(chunk, self.chunk) && let Some(n) = chunk.names.get(slot) {
200200
self.module_state.remove(ssa_strip(n));
201201
}
202+
// Unbind the shared closure cell too, so closures over this name see the deletion.
203+
let cell = self.call_stack.last()
204+
.and_then(|f| f.cells.iter().find(|(s, _)| *s == slot).map(|&(_, c)| c));
205+
if let Some(cell) = cell && cell.is_heap() && let HeapObj::List(rc) = self.heap.get(cell) {
206+
rc.borrow_mut()[0] = Val::undef(); // cells are 1-element boxes
207+
}
202208
}
203209
OpCode::Global | OpCode::Nonlocal => self.mark_impure(),
204210
OpCode::Raise | OpCode::RaiseFrom => {

compiler/src/modules/vm/ops.rs

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -158,24 +158,40 @@ impl<'a> VM<'a> {
158158
}
159159

160160
/* Set bitwise ops (|, &, ^) over set/frozenset; result frozen iff `a` is frozen. */
161-
pub(crate) fn set_binop_and_push(&mut self, a: Val, b: Val, op: OpCode) -> Result<(), VmErr> {
161+
// Union/intersection/symmetric-diff items; content membership lets alloc dedup distinct-handle equals.
162+
fn set_binop_items(&self, a: Val, b: Val, op: OpCode) -> Result<Vec<Val>, VmErr> {
162163
let (sa, sb) = match (self.clone_set_items(a), self.clone_set_items(b)) {
163164
(Some(x), Some(y)) => (x, y),
164165
_ => return Err(cold_runtime("set_binop on non-set operands")),
165166
};
166-
// Content membership so distinct-handle equal elements combine correctly; alloc dedups.
167-
let items: Vec<Val> = match op {
167+
Ok(match op {
168168
OpCode::BitOr => sa.iter().chain(sb.iter()).copied().collect(),
169169
OpCode::BitAnd => sa.iter().filter(|&&v| sb.contains(v, &self.heap)).copied().collect(),
170170
OpCode::BitXor => sa.iter().filter(|&&v| !sb.contains(v, &self.heap))
171171
.chain(sb.iter().filter(|&&v| !sa.contains(v, &self.heap))).copied().collect(),
172172
_ => return Err(cold_runtime("set_binop with non-bitwise opcode")),
173-
};
173+
})
174+
}
175+
176+
pub(crate) fn set_binop_and_push(&mut self, a: Val, b: Val, op: OpCode) -> Result<(), VmErr> {
177+
let items = self.set_binop_items(a, b, op)?;
174178
let frozen = matches!(self.heap.get(a), HeapObj::FrozenSet(_));
175179
let v = self.alloc_set_result(items, frozen)?;
176180
self.push(v); Ok(())
177181
}
178182

183+
// Augmented set bitwise: rewrite the left set in place (identity preserved); frozenset rebinds.
184+
pub(crate) fn set_iop_and_push(&mut self, a: Val, b: Val, op: OpCode) -> Result<(), VmErr> {
185+
if !matches!(self.heap.get(a), HeapObj::Set(_)) {
186+
return self.set_binop_and_push(a, b, op);
187+
}
188+
let items = self.set_binop_items(a, b, op)?;
189+
let mut s = ValSet::with_capacity(items.len());
190+
for v in items { s.insert(v, &self.heap); }
191+
if let HeapObj::Set(rc) = self.heap.get(a) { *rc.borrow_mut() = s; }
192+
self.push(a); Ok(())
193+
}
194+
179195
/* Set comparisons with subset/superset semantics over set/frozenset. */
180196
pub(crate) fn set_compare_and_push(&mut self, a: Val, b: Val, op: OpCode) -> Result<(), VmErr> {
181197
let (sa, sb) = match (self.clone_set_items(a), self.clone_set_items(b)) {

compiler/tests/cases/parser.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1229,21 +1229,21 @@
12291229
"src": "x = 7\nx &= 3",
12301230
"constants": ["7", "3"],
12311231
"names": ["x_1", "x_2"],
1232-
"instructions": [["LoadConst",0], ["StoreName",0], ["LoadName",0], ["LoadConst",1], ["BitAnd",0], ["StoreName",0], ["ReturnValue",0]],
1232+
"instructions": [["LoadConst",0], ["StoreName",0], ["LoadName",0], ["LoadConst",1], ["InPlaceBitAnd",0], ["StoreName",0], ["ReturnValue",0]],
12331233
"annotations": {}
12341234
},
12351235
{
12361236
"src": "x = 5\nx |= 3",
12371237
"constants": ["5", "3"],
12381238
"names": ["x_1", "x_2"],
1239-
"instructions": [["LoadConst",0], ["StoreName",0], ["LoadName",0], ["LoadConst",1], ["BitOr",0], ["StoreName",0], ["ReturnValue",0]],
1239+
"instructions": [["LoadConst",0], ["StoreName",0], ["LoadName",0], ["LoadConst",1], ["InPlaceBitOr",0], ["StoreName",0], ["ReturnValue",0]],
12401240
"annotations": {}
12411241
},
12421242
{
12431243
"src": "x = 7\nx ^= 3",
12441244
"constants": ["7", "3"],
12451245
"names": ["x_1", "x_2"],
1246-
"instructions": [["LoadConst",0], ["StoreName",0], ["LoadName",0], ["LoadConst",1], ["BitXor",0], ["StoreName",0], ["ReturnValue",0]],
1246+
"instructions": [["LoadConst",0], ["StoreName",0], ["LoadName",0], ["LoadConst",1], ["InPlaceBitXor",0], ["StoreName",0], ["ReturnValue",0]],
12471247
"annotations": {}
12481248
},
12491249
{

compiler/tests/cases/vm.json

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3566,5 +3566,26 @@
35663566
{"src": "print(sorted(['bb', 'a', 'ccc'], key=len))", "output": ["['a', 'bb', 'ccc']"]},
35673567
{"src": "xs = [3, 1, 2]\nxs.sort(reverse=True)\nprint(xs)", "output": ["[3, 2, 1]"]},
35683568
{"src": "try:\n print('x', bad=1)\nexcept TypeError:\n print('te')", "output": ["te"]},
3569-
{"src": "try:\n list(5)\nexcept TypeError:\n print('ni')", "output": ["ni"]}
3569+
{"src": "try:\n list(5)\nexcept TypeError:\n print('ni')", "output": ["ni"]},
3570+
{"src": "s1 = s2 = set('abc')\ns1 |= set('ad')\nprint(s1 is s2, len(s1))", "output": ["True 4"]},
3571+
{"src": "s1 = s2 = set('abc')\ns1 &= set('ab')\nprint(s1 is s2, len(s1))", "output": ["True 2"]},
3572+
{"src": "s1 = s2 = set('abc')\ns1 ^= set('ad')\nprint(s1 is s2, len(s1))", "output": ["True 3"]},
3573+
{"src": "s = {1, 2}\ns |= {2, 3}\nprint(sorted(s))", "output": ["[1, 2, 3]"]},
3574+
{"src": "s = {1, 2, 3}\ns &= {2, 3, 4}\nprint(sorted(s))", "output": ["[2, 3]"]},
3575+
{"src": "s = {1, 2, 3}\ns ^= {3, 4}\nprint(sorted(s))", "output": ["[1, 2, 4]"]},
3576+
{"src": "s1 = {1}\ns2 = s1 | {2}\nprint(s1 is s2, sorted(s2))", "output": ["False [1, 2]"]},
3577+
{"src": "f = frozenset({1})\ng = f\nf |= {2}\nprint(f is g, sorted(f))", "output": ["False [1, 2]"]},
3578+
{"src": "s = {1, 2}\ns |= set()\nprint(sorted(s))", "output": ["[1, 2]"]},
3579+
{"src": "a = b = c = {1}\na |= {2}\nprint(b is c, sorted(a))", "output": ["True [1, 2]"]},
3580+
{"src":"def f():\n x = 1\n def g():\n nonlocal x\n try:\n print(x)\n except NameError:\n print(\"NameError\")\n del x\n g()\nf()","output":["NameError"]},
3581+
{"src":"def f():\n x = 5\n def g():\n return x\n return g()\nprint(f())","output":["5"]},
3582+
{"src":"def f():\n x = 1\n def g():\n nonlocal x\n x = 9\n g()\n return x\nprint(f())","output":["9"]},
3583+
{"src":"def f():\n y = 3\n del y\n try:\n return y\n except NameError:\n return 'NE'\nprint(f())","output":["NE"]},
3584+
{"src":"def f():\n z = 1\n del z\n try:\n del z\n except NameError:\n print('twice')\nf()","output":["twice"]},
3585+
{"src":"g = 5\ndel g\ntry:\n print(g)\nexcept NameError:\n print('gone')","output":["gone"]},
3586+
{"src":"def f():\n x = 1\n def g():\n return x\n a = g()\n del x\n return a\nprint(f())","output":["1"]},
3587+
{"src":"def f():\n x = 1\n y = 2\n def g():\n nonlocal x, y\n try:\n print(x)\n except NameError:\n print('x gone')\n print(y)\n del x\n g()\nf()","output":["x gone","2"]},
3588+
{"src":"def f():\n x = 1\n def g():\n nonlocal x\n try:\n return x\n except NameError:\n return 'NE'\n def h():\n nonlocal x\n try:\n return x\n except NameError:\n return 'NE'\n del x\n print(g(), h())\nf()","output":["NE NE"]},
3589+
{"src":"def f():\n x = 1\n def g():\n nonlocal x\n return x\n del x\n try:\n g()\n except NameError:\n return 'caught'\nprint(f())","output":["caught"]},
3590+
{"src":"def f():\n x = 1\n def g():\n return x\n del x\n x = 99\n return g()\nprint(f())","output":["99"]}
35703591
]

docs/content/language/data-types.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ y
302302

303303
## Set
304304

305-
Unordered, no duplicates, hashable values. Mutators (`add`, `remove`, `discard`, `pop`, `clear`, `update`) and algebraic operators (`|`, `&`, `-`, `^` and named methods). See [Methods](/reference/methods).
305+
Unordered, no duplicates, hashable values. Mutators (`add`, `remove`, `discard`, `pop`, `clear`, `update`) and algebraic operators (`|`, `&`, `-`, `^` and named methods). See [Methods](/reference/methods). Augmented `|=` `&=` `^=` mutate the set in place (aliases observe it); plain `|` `&` `^` build a new set.
306306

307307
```python
308308
s = {1, 2, 2, 3}

0 commit comments

Comments
 (0)