Skip to content

Commit ee6e71f

Browse files
thiremaniclaude
andcommitted
refactor(cond): name the || store-emission pieces instead of nesting closures
extractFallbackOrSlots carried a 40-line storeSide closure with a second closure inside it, and its per-slot emission shape (run under the slot condition, free the skipped mask on the else arm) duplicated commitSpineSlot. Extract that shape as underSlotCond — both call sites now share it — and the store body as storeFallbackValue, shrinking storeSide to a few lines that only compute each slot's take-condition. The closures that remain are the ones earning their keep: storeSide is emitted twice (left side inline, right side inside the lazy or_rhs branch), and the store callback lets one body be emitted at whichever insertion point is current. No behavior change; IR block labels for spine slots become slot_take/slot_skip/slot_cont. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent bffec35 commit ee6e71f

1 file changed

Lines changed: 40 additions & 45 deletions

File tree

compiler/cond.go

Lines changed: 40 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -642,50 +642,18 @@ func (c *Compiler) extractFallbackOrSlots(or *ast.InfixExpression, info *ExprInf
642642
c.createStore(llvm.ConstInt(i1Ty, 0, false), yields[i], i1)
643643
}
644644

645+
// Left and right stores differ only in needFlags: the right side must also
646+
// check that the slot was not already filled by the left.
645647
storeSide := func(side ast.Expression, conds []llvm.Value, needFlags bool) {
646648
for i, outType := range info.OutTypes {
647-
store := func() {
648-
val, owned := c.spineSlotValue(side, i, outType)
649-
if owned {
650-
// Ownership moves into the result slot; mark the source so
651-
// mask cleanup skips it (a fresh combine is simply discarded).
652-
val.Borrowed = true
653-
} else {
654-
// Deref before deciding to copy: a leaf's slot value may be
655-
// Ptr-wrapped, and the copy must clone the pointee — the
656-
// source is freed while the result slot lives on.
657-
val = c.derefIfPointer(val, fmt.Sprintf("or_take_val_%d", i))
658-
if !IsStrG(val.Type) {
659-
val = c.deepCopyIfNeeded(val)
660-
}
661-
}
662-
coerced := c.coerceSymbolForType(val, outType, fmt.Sprintf("or_val_%d", i))
663-
c.createStore(coerced.Val, slots[i], coerced.Type)
664-
c.createStore(llvm.ConstInt(i1Ty, 1, false), yields[i], i1)
665-
}
666-
667649
cond := slotCondAt(conds, i)
668650
if needFlags {
669651
need := c.builder.CreateNot(c.createLoad(yields[i], i1, fmt.Sprintf("or_need_%d", i)), fmt.Sprintf("or_miss_%d", i))
670652
cond = c.andConds(need, cond, fmt.Sprintf("or_take_%d", i))
671653
}
672-
if cond.IsNil() {
673-
store()
674-
continue
675-
}
676-
677-
ifBlock, elseBlock, contBlock := c.createIfElseCont(cond, fmt.Sprintf("or_store_%d", i), fmt.Sprintf("or_skip_%d", i), fmt.Sprintf("or_store_cont_%d", i))
678-
679-
c.builder.SetInsertPointAtEnd(ifBlock)
680-
store()
681-
c.builder.CreateBr(contBlock)
682-
683-
// A directly stored mask skipped at runtime was never moved; free it.
684-
c.builder.SetInsertPointAtEnd(elseBlock)
685-
c.freeSkippedSlotMask(side, i)
686-
c.builder.CreateBr(contBlock)
687-
688-
c.builder.SetInsertPointAtEnd(contBlock)
654+
c.underSlotCond(side, i, cond, fmt.Sprintf("or_store_%d", i), func() {
655+
c.storeFallbackValue(side, i, outType, slots[i], yields[i])
656+
})
689657
}
690658
}
691659

@@ -1348,28 +1316,32 @@ func (c *Compiler) prepareSpineLeaf(expr ast.Expression, info *ExprInfo, temps [
13481316
}
13491317

13501318
// commitSpineSlot commits output slot i of a per-slot spine into its temp
1351-
// slot, keeping the seeded prior value when the slot condition fails. When the
1352-
// slot's value is a directly committed mask, the failing branch frees it — the
1353-
// mask was built during extraction, so a runtime-skipped store must release it.
1319+
// slot, keeping the seeded prior value when the slot condition fails.
13541320
func (c *Compiler) commitSpineSlot(expr ast.Expression, i int, cond llvm.Value, tempName *ast.Identifier, outType Type) {
1355-
commit := func() {
1321+
c.underSlotCond(expr, i, cond, "slot", func() {
13561322
val, owned := c.spineSlotValue(expr, i, outType)
13571323
c.commitSlotValue(tempName, val, !owned)
13581324
if owned {
13591325
// Ownership moved into the slot; mark the source so mask cleanup
13601326
// skips it (a fresh combine is simply discarded).
13611327
val.Borrowed = true
13621328
}
1363-
}
1329+
})
1330+
}
1331+
1332+
// underSlotCond emits store for slot i of expr under cond (unconditionally
1333+
// when nil). The skipped arm frees the mask that slot would have moved: masks
1334+
// are built during extraction, so a runtime-skipped store must release one.
1335+
func (c *Compiler) underSlotCond(expr ast.Expression, i int, cond llvm.Value, name string, store func()) {
13641336
if cond.IsNil() {
1365-
commit()
1337+
store()
13661338
return
13671339
}
13681340

1369-
ifBlock, elseBlock, contBlock := c.createIfElseCont(cond, "slot_if", "slot_else", "slot_cont")
1341+
ifBlock, elseBlock, contBlock := c.createIfElseCont(cond, name+"_take", name+"_skip", name+"_cont")
13701342

13711343
c.builder.SetInsertPointAtEnd(ifBlock)
1372-
commit()
1344+
store()
13731345
c.builder.CreateBr(contBlock)
13741346

13751347
c.builder.SetInsertPointAtEnd(elseBlock)
@@ -1379,6 +1351,29 @@ func (c *Compiler) commitSpineSlot(expr ast.Expression, i int, cond llvm.Value,
13791351
c.builder.SetInsertPointAtEnd(contBlock)
13801352
}
13811353

1354+
// storeFallbackValue writes slot i of side into a || result slot and marks it
1355+
// yielded. An owned value (mask, fresh combine) moves in — its source is
1356+
// marked so mask cleanup skips it. A borrowed view is dereferenced first (a
1357+
// leaf's slot value may be Ptr-wrapped, and the copy must clone the pointee)
1358+
// and deep-copied, since the source is freed while the result slot lives on;
1359+
// a static string is left to the store's coercion, which copies it only when
1360+
// the slot type is heap-owned.
1361+
func (c *Compiler) storeFallbackValue(side ast.Expression, i int, outType Type, slot, yield llvm.Value) {
1362+
val, owned := c.spineSlotValue(side, i, outType)
1363+
if owned {
1364+
val.Borrowed = true
1365+
} else {
1366+
val = c.derefIfPointer(val, fmt.Sprintf("or_take_val_%d", i))
1367+
if !IsStrG(val.Type) {
1368+
val = c.deepCopyIfNeeded(val)
1369+
}
1370+
}
1371+
coerced := c.coerceSymbolForType(val, outType, fmt.Sprintf("or_val_%d", i))
1372+
c.createStore(coerced.Val, slot, coerced.Type)
1373+
i1 := Int{Width: 1}
1374+
c.createStore(llvm.ConstInt(c.mapToLLVMType(i1), 1, false), yield, i1)
1375+
}
1376+
13821377
// freeSkippedSlotMask frees the frame mask that slot i of expr would have
13831378
// moved, on the runtime path where the slot's store was skipped. The Borrowed
13841379
// mark set while building the store arm is ignored — these are exclusive

0 commit comments

Comments
 (0)