Skip to content

Commit e8ab118

Browse files
r41k0uclaude
andcommitted
Tests: Record the lower-do-not-desugar house style in the skill
Captures the trade-off behind reworking augmented assignment: desugaring into synthetic AST is a legitimate compiler technique with real benefits, and it is still the wrong shape for this compiler, whose passes communicate through the source tree. Records both sides so the next person tempted by it declines for the right reasons, and states the idiom to reach for instead -- shared value-level helpers, not shared AST. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01L1PX8EuP9C3o3veWGA84RF
1 parent 1b9adc7 commit e8ab118

1 file changed

Lines changed: 27 additions & 0 deletions

File tree

.claude/skills/ir-first-feature/SKILL.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,33 @@ Emit IR through the existing passes (`globals_pass`, `expr_pass`, `assign_pass`,
5454
clang reference for the same shapes** — "it compiles and llc accepts it" is not the
5555
bar; llc accepts plenty of subtly wrong IR.
5656

57+
### House style: lower, don't desugar
58+
59+
Handlers walk the AST the user wrote and emit IR directly. **Do not synthesize new AST
60+
nodes mid-compilation and feed them back through other handlers** — no
61+
`ast.Assign(ast.BinOp(...))` conjured to make `x += v` reuse the assignment path.
62+
63+
The temptation is legitimate, so know the argument you are declining. Desugaring is a
64+
standard compiler move (CPython itself lowers `x += v` this way), it guarantees semantic
65+
agreement with the composed form, it is the smallest diff, and any later fix to the
66+
composed path applies automatically. Those are real benefits.
67+
68+
They lose in this codebase for structural reasons: the passes communicate through the
69+
source tree. Allocation runs before codegen and walks `Assign` — a synthetic `Assign`
70+
created during codegen is invisible to it, so the two passes silently disagree about
71+
what the function contains (it happened to be harmless for augmented assignment only
72+
because that statement never needs a fresh slot; that is luck, not design). Synthetic
73+
nodes carry no source location, so diagnostics point nowhere. And `ast.dump` in the logs
74+
shows statements the user never typed, which turns every debugging session into an
75+
archaeology exercise.
76+
77+
The resolution is to move sharing down a level: **equivalence should come from shared
78+
value-level helpers, not shared AST.** When two constructs must agree, extract the common
79+
logic into a helper both call — the way binary-op evaluation and augmented assignment
80+
both use `apply_binop` for the operator table and `get_operand_value` for operands —
81+
and let each handler resolve its own target and emit its own store. Two handlers calling
82+
one helper is the idiom; one handler manufacturing input for another is not.
83+
5784
## 6. Test at the right tier
5885

5986
- Works now → `tests/passing_tests/<category>/`.

0 commit comments

Comments
 (0)