Skip to content

Commit

Permalink
Stack frame refactor (#33)
Browse files Browse the repository at this point in the history
This fixes a bug where mutations to stack frames weren't being captured
correctly. When pushing a frame to the stack, a reference is taken to
the frame slice's backing array. If frames are added and a new backing
array is allocated, the references are invalidated and any further
mutations are lost.

The three alternatives for fixing the issue were:
1. avoid a dynamic array and instead use a linked list for the frames,
so that adding new frames doesn't invalidate previous references
2. update the stack's `Push()` method to return a different reference to
a frame (e.g. `FrameRef{stack, index}`), and intercept mutating calls so
that they're routed to the correct place
3. update the stack's `Push()` method to instead return a `Frame` by
value, and require that callers store the frame after mutations have
been made when unwinding the stack

I went with option 3 given that:
* there a lot of mutations (we need to update the IP often)
* we don't always need to persist the frame; only when unwinding; so no
need to pay the cost of indirection at all times
* there are potentially many frames so we want to avoid many small
allocations/fragmentation
  • Loading branch information
chriso authored Sep 19, 2023
2 parents ea412cf + 7c5c762 commit f5ad6ab
Show file tree
Hide file tree
Showing 7 changed files with 138 additions and 103 deletions.
14 changes: 11 additions & 3 deletions compiler/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ func (c *compiler) compileFunction(p *packages.Package, fn *ast.FuncDecl, color

ctx := ast.NewIdent("_c")
frame := ast.NewIdent("_f")
fp := ast.NewIdent("_fp")

yieldTypeExpr := make([]ast.Expr, 2)
yieldTypeExpr[0] = typeExpr(color.Params().At(0).Type())
Expand All @@ -277,9 +278,9 @@ func (c *compiler) compileFunction(p *packages.Package, fn *ast.FuncDecl, color
},
})

// _f := _c.Push()
// _f, _fp := _c.Push()
gen.Body.List = append(gen.Body.List, &ast.AssignStmt{
Lhs: []ast.Expr{frame},
Lhs: []ast.Expr{frame, fp},
Tok: token.DEFINE,
Rhs: []ast.Expr{
&ast.CallExpr{
Expand Down Expand Up @@ -379,7 +380,14 @@ func (c *compiler) compileFunction(p *packages.Package, fn *ast.FuncDecl, color
Cond: &ast.CallExpr{
Fun: &ast.SelectorExpr{X: ctx, Sel: ast.NewIdent("Unwinding")},
},
Body: &ast.BlockStmt{List: saveStmts},
Body: &ast.BlockStmt{
List: append(saveStmts, &ast.ExprStmt{
X: &ast.CallExpr{
Fun: &ast.SelectorExpr{X: ctx, Sel: ast.NewIdent("Store")},
Args: []ast.Expr{fp, frame},
},
}),
},
Else: &ast.BlockStmt{List: []ast.Stmt{
&ast.ExprStmt{X: &ast.CallExpr{Fun: &ast.SelectorExpr{X: ctx, Sel: ast.NewIdent("Pop")}}}},
},
Expand Down
6 changes: 6 additions & 0 deletions compiler/coroutine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ func TestCoroutineYield(t *testing.T) {
yields: []int{1, 4, 9, 16, 1, 4, 9, 16},
},

{
name: "square generator twice loop",
coro: func() { SquareGeneratorTwiceLoop(4) },
yields: []int{1, 4, 9, 16, 1, 4, 9, 16},
},

{
name: "even square generator",
coro: func() { EvenSquareGenerator(6) },
Expand Down
6 changes: 6 additions & 0 deletions compiler/testdata/coroutine.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ func SquareGeneratorTwice(n int) {
SquareGenerator(n)
}

func SquareGeneratorTwiceLoop(n int) {
for i := 0; i < 2; i++ {
SquareGenerator(n)
}
}

func EvenSquareGenerator(n int) {
for i := 1; i <= n; i++ {
if mod2 := i % 2; mod2 == 0 {
Expand Down
Loading

0 comments on commit f5ad6ab

Please sign in to comment.