Skip to content

Commit 0ab33f4

Browse files
committed
done!
1 parent fda3202 commit 0ab33f4

File tree

1 file changed

+139
-35
lines changed

1 file changed

+139
-35
lines changed

teaching-todo.md

Lines changed: 139 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ r == r2 # comparable
148148
- [x] Find `PySequenceMethods` in headers
149149
- [x] Study `sq_length` - returns `Py_ssize_t`
150150
- [x] Study `sq_item` - takes index, returns item (with INCREF!)
151-
- [~] **For Record:** Implement these so `r[0]` and `len(r)` work *(design understood, implementation pending)*
151+
- [x] **For Record:** Implement these so `r[0]` and `len(r)` work
152152
153153
**Notes from session:**
154154
- `PySequenceMethods` contains: `sq_length`, `sq_concat`, `sq_repeat`, `sq_item`, `sq_ass_item`, `sq_contains`, etc.
@@ -243,38 +243,132 @@ r == r2 # comparable
243243
## Phase 5: Implementation
244244
245245
### 5.1 Create the Header File
246-
- [ ] Create `Include/recordobject.h`
247-
- [ ] Define `RecordObject` struct
248-
- [ ] Declare `PyRecord_Type`
249-
- [ ] Declare `PyRecord_New()` constructor function
246+
- [x] Create `Include/recordobject.h`
247+
- [x] Define `RecordObject` struct
248+
- [x] Declare `PyRecord_Type`
249+
- [x] Declare `PyRecord_New()` constructor function
250+
251+
**Notes from session:**
252+
- Two-level header structure: `Include/recordobject.h` (public) includes `Include/cpython/recordobject.h` (internal)
253+
- Must add `#include "recordobject.h"` to `Include/Python.h` for it to be visible
254+
- `PyRecord_Check` macro simplified to `Py_IS_TYPE(op, &PyRecord_Type)` (no subclass flag)
250255
251256
### 5.2 Implement the Type
252-
- [ ] Create `Objects/recordobject.c`
253-
- [ ] Implement `record_dealloc`
254-
- [ ] Implement `record_repr`
255-
- [ ] Implement `record_hash`
256-
- [ ] Implement `record_richcompare`
257-
- [ ] Implement `record_length` (sq_length)
258-
- [ ] Implement `record_item` (sq_item)
259-
- [ ] Implement `record_getattro` (attribute access by name)
260-
- [ ] Define `PyRecord_Type` with all slots filled
261-
- [ ] Implement `PyRecord_New()` - the C API constructor
257+
- [x] Create `Objects/recordobject.c`
258+
- [x] Implement `record_dealloc`
259+
- [x] Implement `record_repr`
260+
- [x] Implement `record_hash`
261+
- [x] Implement `record_richcompare`
262+
- [x] Implement `record_length` (sq_length)
263+
- [x] Implement `record_item` (sq_item)
264+
- [x] Implement `record_getattro` (attribute access by name)
265+
- [x] Define `PyRecord_Type` with all slots filled
266+
- [x] Implement `PyRecord_New()` - the C API constructor
267+
268+
**Notes from session:**
269+
- Started by copying tupleobject.c, then stripped out complexity (clinic, free lists, etc.)
270+
- **GC gotcha:** Originally had GC alloc/track/untrack but no `Py_TPFLAGS_HAVE_GC` or `tp_traverse` - caused segfault. Fixed by removing GC entirely (simpler for learning)
271+
- **record_dealloc:** `Py_XDECREF(names)` + XDECREF each value, use XDECREF (handles NULL safely)
272+
- **record_repr:** Use `_PyUnicodeWriter`, format as `record(x=1, y=2)`, don't repr the field names (just write them directly)
273+
- **record_hash:** Copied xxHash pattern from tuple, hash names tuple + all values, `-1` → `-2` for actual -1 hash
274+
- **record_getattro:** Loop through names with `PyUnicode_Compare`, return value with INCREF, fallback to `PyObject_GenericGetAttr`
275+
- **record_richcompare:** Compare names first (if different, not equal), then values; return `Py_NotImplemented` for ordering
276+
- **record_new (tp_new):** Parse kwargs with `PyDict_Keys`/`PyDict_Values`, convert keys to tuple for names
262277
263278
### 5.3 Add the Opcode
264-
- [ ] Add `BUILD_RECORD` to `Lib/opcode.py` (pick unused number, needs argument)
265-
- [ ] Run `make regen-opcode` and `make regen-opcode-targets`
266-
- [ ] Implement `BUILD_RECORD` handler in `Python/ceval.c`
279+
- [x] Add `BUILD_RECORD` to `Lib/opcode.py` (pick unused number, needs argument)
280+
- [x] Run `make regen-opcode` and `make regen-opcode-targets`
281+
- [x] Implement `BUILD_RECORD` handler in `Python/ceval.c`
282+
283+
**Notes from session:**
284+
- Used opcode 166 (was available)
285+
- Stack layout: `[names_tuple, val0, val1, ...]` - pop values in reverse, then pop names
286+
- Must add stack effect to compile.c: `case BUILD_RECORD: return -oparg;`
287+
- Regenerate with `./python.exe ./Tools/scripts/generate_opcode_h.py` and `./python.exe ./Python/makeopcodetargets.py`
267288
268289
### 5.4 Build System Integration
269-
- [ ] Add `recordobject.c` to the build (Makefile.pre.in or setup.py)
270-
- [ ] Add header to appropriate include lists
271-
- [ ] Register type in Python initialization
290+
- [x] Add `recordobject.c` to the build (Makefile.pre.in or setup.py)
291+
- [x] Add header to appropriate include lists
292+
- [x] Register type in Python initialization
293+
294+
**Notes from session:**
295+
- Add `Objects/recordobject.o` to `OBJECT_OBJS` in Makefile.pre.in
296+
- Add both headers to `PYTHON_HEADERS` section
297+
- Register with `SETBUILTIN("record", &PyRecord_Type)` in `Python/bltinmodule.c`
272298
273299
### 5.5 Build and Test
274-
- [ ] Run `make` - fix any compilation errors
275-
- [ ] Test basic creation via C API
276-
- [ ] Test via manual bytecode or compiler modification
277-
- [ ] Verify all operations: indexing, len, hash, repr, equality, attribute access
300+
- [x] Run `make` - fix any compilation errors
301+
- [x] Test basic creation via C API
302+
- [x] Test via manual bytecode or compiler modification
303+
- [x] Verify all operations: indexing, len, hash, repr, equality, attribute access
304+
305+
**Notes from session:**
306+
- All tests pass:
307+
```python
308+
r = record(x=1, y=2)
309+
print(r) # record(x=1, y=2)
310+
print(r.x) # 1
311+
print(r[0]) # 1
312+
print(len(r)) # 2
313+
print(hash(r)) # works
314+
r2 = record(x=1, y=2)
315+
print(r == r2) # True
316+
```
317+
318+
---
319+
320+
## Phase 6: Literal Syntax (Bonus)
321+
322+
### 6.1 Grammar Extension
323+
- [x] Add `record` rule to `Grammar/python.gram`
324+
- [x] Choose syntax: `{| x=1, y=2 |}`
325+
- [x] Add to `atom` rule with lookahead `&'{'`
326+
327+
**Notes from session:**
328+
- Reused `kwargs` from function call parsing for `x=1, y=2` syntax
329+
- Had to use separate tokens `'{' '|'` not `'{|'` (multi-char tokens don't exist)
330+
- Grammar rule: `| '{' '|' a=[kwargs] '|' '}' { _PyAST_Record(...) }`
331+
332+
### 6.2 AST Node
333+
- [x] Add `Record(expr* keys, expr* values)` to `Parser/Python.asdl`
334+
- [x] Run `make regen-ast` to generate `_PyAST_Record`
335+
336+
**Notes from session:**
337+
- Modeled after `Dict(expr* keys, expr* values)`
338+
- Regeneration creates `_PyAST_Record` function and `Record_kind` enum
339+
340+
### 6.3 Parser Helpers
341+
- [x] Create `_PyPegen_get_record_keys` in `Parser/pegen.c`
342+
- [x] Create `_PyPegen_get_record_values` in `Parser/pegen.c`
343+
344+
**Notes from session:**
345+
- `kwargs` returns `KeywordOrStarred*` items, not `KeyValuePair*` like dict
346+
- Had to unwrap each to get `keyword_ty`, then extract `.arg` (identifier) and `.value` (expression)
347+
- For keys, create `Name` expression from the identifier
348+
349+
### 6.4 Compiler Integration
350+
- [x] Add `compiler_record` function in `Python/compile.c`
351+
- [x] Add `case Record_kind:` to `compiler_visit_expr`
352+
- [x] Add `case Record_kind:` to ALL switches that handle `Dict_kind`:
353+
- `Python/ast.c` (2 switches!)
354+
- `Python/ast_opt.c`
355+
- `Python/ast_unparse.c`
356+
- `Python/symtable.c`
357+
- `Python/compile.c` (3 switches!)
358+
359+
**Notes from session:**
360+
- **Key lesson:** Every switch on expression kinds needs the new case - easy to miss some!
361+
- `compiler_record`: Build names tuple as constant, VISIT each value, emit BUILD_RECORD
362+
- Extracting keys: `e->v.Record.keys` contains `expr_ty` Name nodes, access with `key_expr->v.Name.id`
363+
364+
### 6.5 Final Result
365+
- [x] Literal syntax `{| x=1, y=2 |}` works
366+
- [x] Emits BUILD_RECORD opcode
367+
- [x] All features work with literal syntax
368+
369+
**Notes from session:**
370+
- Full pipeline working: Grammar → Parser → AST → Compiler → Bytecode → Execution
371+
- Test: `r = {|x=1, y=2|}; print(r.x)``1`
278372

279373
---
280374

@@ -313,16 +407,26 @@ assert r == r2
313407

314408
---
315409

316-
## Files We'll Create/Modify
317-
318-
| File | Action | ~Lines |
319-
|------|--------|--------|
320-
| `Include/recordobject.h` | Create | 25 |
321-
| `Objects/recordobject.c` | Create | 200 |
322-
| `Lib/opcode.py` | Modify | 2 |
323-
| `Python/ceval.c` | Modify | 30 |
324-
| `Makefile.pre.in` | Modify | 5 |
325-
| `Python/bltinmodule.c` | Modify | 10 |
410+
## Files Created/Modified
411+
412+
| File | Action | Notes |
413+
|------|--------|-------|
414+
| `Include/recordobject.h` | Create | Public API header |
415+
| `Include/cpython/recordobject.h` | Create | Internal struct definition |
416+
| `Include/Python.h` | Modify | Add include for recordobject.h |
417+
| `Objects/recordobject.c` | Create | ~300 lines - type implementation |
418+
| `Lib/opcode.py` | Modify | Add BUILD_RECORD = 166 |
419+
| `Python/ceval.c` | Modify | BUILD_RECORD handler |
420+
| `Python/compile.c` | Modify | compiler_record, stack effect, expr switches |
421+
| `Makefile.pre.in` | Modify | Add recordobject.o and headers |
422+
| `Python/bltinmodule.c` | Modify | SETBUILTIN for record |
423+
| `Grammar/python.gram` | Modify | record rule, atom lookahead |
424+
| `Parser/Python.asdl` | Modify | Record AST node |
425+
| `Parser/pegen.c` | Modify | Helper functions for record keys/values |
426+
| `Python/ast.c` | Modify | Record_kind in validation switches |
427+
| `Python/ast_opt.c` | Modify | Record_kind case |
428+
| `Python/ast_unparse.c` | Modify | append_ast_record |
429+
| `Python/symtable.c` | Modify | Record_kind case |
326430

327431
---
328432

0 commit comments

Comments
 (0)