Skip to content

Commit e1c1ec3

Browse files
jgarzikclaude
andcommitted
Fix dict.update/popitem/set.update, add more tests
- Fix dict.update() no-args crash: add nargs guard - Fix dict.popitem() key tag: read from entry instead of hardcoding TAG_PTR - Implement set.update() method and register in tp_dict - Add test_popitem to test_dict.py (25 tests now) - Restore dict.update() no-args test - Update set.update test to use real method - Update bugs.md with fixes #17-#19 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 2f68cf7 commit e1c1ec3

4 files changed

Lines changed: 108 additions & 13 deletions

File tree

bugs.md

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,14 +81,28 @@
8181
**Root cause**: `dict_type.tp_call = 0`, so `type_call` used generic `instance_new` which allocated a PyDictObject-sized block but didn't initialize the hash table entries array.
8282
**Fix**: Implement `dict_type_call` that calls `dict_new` for no-args case and copies entries for dict(other_dict) case.
8383

84+
### 17. dict.update() no-args crash (methods.asm)
85+
**Symptom**: `d.update()` segfaults
86+
**Root cause**: Always reads `args[1]` without checking nargs first
87+
**Fix**: Add `cmp rsi, 1; jle .du_done` guard at start
88+
89+
### 18. dict.popitem() key tag hardcoded to TAG_PTR (methods.asm)
90+
**Symptom**: `d.popitem()` segfaults on dicts with non-string keys
91+
**Root cause**: Key tag hardcoded to TAG_PTR, `INCREF` used instead of `INCREF_VAL`, key tag not read from entry
92+
**Fix**: Read key_tag from entry, use INCREF_VAL, pass correct tag to dict_del
93+
94+
### 19. set.update() method missing (methods.asm)
95+
**Symptom**: `s.update({3,4})` raises AttributeError
96+
**Root cause**: Method not implemented, not registered in set tp_dict
97+
**Fix**: Implement set_method_update, register in init_builtin_methods
98+
8499
### Known Bugs Not Yet Fixed
85-
- `dict.update()` with no args segfaults (methods.asm)
86100
- `dict.update(x=1, y=2)` with kwargs segfaults (methods.asm)
87-
- `dict.popitem()` segfaults (methods.asm)
88-
- `set.update()` method not implemented (use |= instead)
89101
- `repr(d.keys())` returns wrong value (dict view repr not implemented)
90102
- `(1,1) in d.items()` fails (dict_items __contains__ not implemented)
91103
- `tuple(t) is t` identity optimization not implemented
104+
- `list.append()` no-args segfaults (method arg count validation missing)
105+
- `assertRaises(fn)` double-free crash (exception handling memory issue)
92106

93107
## New Infrastructure Added
94108
- `list_copy()` - standalone shallow copy function

src/methods.asm

Lines changed: 73 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5846,6 +5846,11 @@ DEF_FUNC dict_method_update
58465846
push r14
58475847

58485848
mov rbx, [rdi] ; self
5849+
5850+
; If nargs == 1 (just self, no args), return None immediately
5851+
cmp rsi, 1
5852+
jle .du_done
5853+
58495854
mov r12, [rdi + 16] ; other dict
58505855

58515856
mov r13, [r12 + PyDictObject.capacity]
@@ -6145,29 +6150,34 @@ DEF_FUNC dict_method_popitem
61456150

61466151
.dpopitem_found:
61476152
; r13 = key, r14 = value, rcx = value_tag
6153+
; Also save key_tag from the entry
6154+
mov rax, [rbx + PyDictObject.entries]
6155+
imul rdx, r12, DICT_ENTRY_SIZE
6156+
add rax, rdx
6157+
movzx r8d, byte [rax + DictEntry.key_tag]
6158+
push r8 ; save key_tag
61486159
push rcx ; save value_tag across tuple_new
61496160
; Create 2-tuple
61506161
mov rdi, 2
61516162
call tuple_new
61526163
pop rcx ; restore value_tag
6164+
pop r8 ; restore key_tag
61536165
mov r12, rax ; r12 = tuple
61546166

6155-
; Set tuple[0] = key, tuple[1] = value
6167+
; Set tuple[0] = key with correct tag, tuple[1] = value
61566168
mov r9, [r12 + PyTupleObject.ob_item]
61576169
mov r10, [r12 + PyTupleObject.ob_item_tags]
6158-
; Key from dict entries — always heap ptr (strings)
61596170
mov [r9], r13
6160-
mov byte [r10], TAG_PTR
6161-
INCREF r13
6171+
mov byte [r10], r8b ; key tag from entry
6172+
INCREF_VAL r13, r8
61626173
mov [r9 + 8], r14
6163-
; Use stored value_tag from dict entry
6164-
mov byte [r10 + 1], cl
6174+
mov byte [r10 + 1], cl ; value tag from entry
61656175
INCREF_VAL r14, rcx
61666176

61676177
; Delete key from dict
61686178
mov rdi, rbx
61696179
mov rsi, r13
6170-
mov edx, TAG_PTR
6180+
movzx edx, byte [r10] ; key tag
61716181
call dict_del
61726182

61736183
mov rax, r12
@@ -6919,6 +6929,57 @@ DEF_FUNC set_method_union
69196929
call raise_exception
69206930
END_FUNC set_method_union
69216931

6932+
;; ============================================================================
6933+
;; set_method_update(args, nargs) -> None
6934+
;; args[0]=self, args[1]=other (set). Adds all elements of other to self.
6935+
;; ============================================================================
6936+
DEF_FUNC set_method_update
6937+
push rbx
6938+
push r12
6939+
push r13
6940+
6941+
mov rbx, [rdi] ; self
6942+
; If no other arg, no-op
6943+
cmp rsi, 2
6944+
jl .supd_done
6945+
6946+
mov r12, [rdi + 16] ; other set
6947+
mov r13, [r12 + PyDictObject.capacity]
6948+
xor ecx, ecx
6949+
6950+
.supd_loop:
6951+
cmp rcx, r13
6952+
jge .supd_done
6953+
6954+
imul rax, rcx, SET_ENTRY_SIZE
6955+
add rax, [r12 + PyDictObject.entries]
6956+
push rcx
6957+
6958+
cmp qword [rax + SET_ENTRY_KEY_TAG], 0
6959+
je .supd_next
6960+
6961+
mov rdi, rbx
6962+
mov rsi, [rax + SET_ENTRY_KEY]
6963+
mov rdx, [rax + SET_ENTRY_KEY_TAG]
6964+
call set_add
6965+
6966+
.supd_next:
6967+
pop rcx
6968+
inc ecx
6969+
jmp .supd_loop
6970+
6971+
.supd_done:
6972+
extern none_singleton
6973+
lea rax, [rel none_singleton]
6974+
inc qword [rax + PyObject.ob_refcnt]
6975+
mov edx, TAG_PTR
6976+
pop r13
6977+
pop r12
6978+
pop rbx
6979+
leave
6980+
ret
6981+
END_FUNC set_method_update
6982+
69226983
;; ============================================================================
69236984
;; set_method_intersection(args, nargs) -> new set = self & other
69246985
;; args[0]=self, args[1]=other
@@ -9781,6 +9842,11 @@ DEF_FUNC methods_init
97819842
lea rdx, [rel set_method_isdisjoint]
97829843
call add_method_to_dict
97839844

9845+
mov rdi, rbx
9846+
lea rsi, [rel mn_update]
9847+
lea rdx, [rel set_method_update]
9848+
call add_method_to_dict
9849+
97849850
; Store in set_type.tp_dict
97859851
lea rax, [rel set_type]
97869852
mov [rax + PyTypeObject.tp_dict], rbx

tests/cpython/test_dict.py

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ def test_update(self):
7878
d.update({1: 1, 2: 2, 3: 3})
7979
self.assertEqual(d, {1: 1, 2: 2, 3: 3})
8080

81-
# Skip: d.update() with no args crashes (known bug)
81+
d.update()
8282
self.assertEqual(d, {1: 1, 2: 2, 3: 3})
8383

8484
def test_fromkeys(self):
@@ -135,6 +135,19 @@ def test_pop(self):
135135
self.assertEqual(d.pop('abc', 'ghi'), 'ghi')
136136
self.assertEqual(len(d), 0)
137137

138+
def test_popitem(self):
139+
d = {1: 'a', 2: 'b', 3: 'c'}
140+
items = []
141+
while d:
142+
items.append(d.popitem())
143+
self.assertEqual(len(items), 3)
144+
self.assertEqual(sorted(items), [(1, 'a'), (2, 'b'), (3, 'c')])
145+
try:
146+
d.popitem()
147+
self.fail("Expected KeyError")
148+
except KeyError:
149+
pass
150+
138151
def test_repr(self):
139152
d = {}
140153
self.assertEqual(repr(d), '{}')

tests/cpython/test_set.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,11 @@ def test_large_set(self):
157157
self.assertIn(i, s)
158158

159159
def test_update(self):
160-
# set.update via |= (update method not available)
161160
s = {1, 2}
162-
s |= {3, 4}
161+
s.update({3, 4})
162+
self.assertEqual(sorted(s), [1, 2, 3, 4])
163+
# No-arg update is a no-op
164+
s.update()
163165
self.assertEqual(sorted(s), [1, 2, 3, 4])
164166

165167
def test_set_from_list(self):

0 commit comments

Comments
 (0)