Skip to content

Commit

Permalink
bug fix for function return and if statement (#160)
Browse files Browse the repository at this point in the history
* fix a bug where function return instruction is missing with if statement

* remove script cancel context test
  • Loading branch information
d5 authored Mar 23, 2019
1 parent 2b21c29 commit 01fe30f
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 18 deletions.
39 changes: 36 additions & 3 deletions compiler/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,9 +405,7 @@ func (c *Compiler) Compile(node ast.Node) error {
}

// add OpReturn if function returns nothing
if !c.lastInstructionIs(OpReturn) {
c.emit(node, OpReturn, 0)
}
c.fixReturn(node)

freeSymbols := c.symbolTable.FreeSymbols()
numLocals := c.symbolTable.MaxSymbols()
Expand Down Expand Up @@ -733,6 +731,41 @@ func (c *Compiler) changeOperand(opPos int, operand ...int) {
c.replaceInstruction(opPos, inst)
}

// fixReturn appends "return" statement at the end of the function if
// 1) the function does not have a "return" statement at the end.
// 2) or, there are jump instructions that jump to the end of the function.
func (c *Compiler) fixReturn(node ast.Node) {
var appendReturn bool

if !c.lastInstructionIs(OpReturn) {
appendReturn = true
} else {
var lastOp Opcode
insts := c.scopes[c.scopeIndex].instructions
endPos := len(insts)
iterateInstructions(insts, func(pos int, opcode Opcode, operands []int) bool {
defer func() { lastOp = opcode }()

switch opcode {
case OpJump, OpJumpFalsy, OpAndJump, OpOrJump:
dst := operands[0]
if dst == endPos && lastOp != OpReturn {
appendReturn = true
return false
} else if dst > endPos {
panic(fmt.Errorf("wrong jump position: %d (end: %d)", dst, endPos))
}
}

return true
})
}

if appendReturn {
c.emit(node, OpReturn, 0)
}
}

func (c *Compiler) emit(node ast.Node, opcode Opcode, operands ...int) int {
filePos := source.NoPos
if node != nil {
Expand Down
5 changes: 1 addition & 4 deletions compiler/compiler_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,7 @@ func (c *Compiler) compileModule(node ast.Node, moduleName, modulePath string, s
return nil, err
}

// add OpReturn (== export undefined) if export is missing
if !moduleCompiler.lastInstructionIs(OpReturn) {
moduleCompiler.emit(nil, OpReturn)
}
moduleCompiler.fixReturn(node)

compiledFunc := moduleCompiler.Bytecode().MainFunction
compiledFunc.NumLocals = symbolTable.MaxSymbols()
Expand Down
13 changes: 13 additions & 0 deletions compiler/instructions.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,16 @@ func FormatInstructions(b []byte, posOffset int) []string {

return out
}

func iterateInstructions(b []byte, fn func(pos int, opcode Opcode, operands []int) bool) {
for i := 0; i < len(b); i++ {
numOperands := OpcodeOperands[Opcode(b[i])]
operands, read := ReadOperands(numOperands, b[i+1:])

if !fn(i, b[i], operands) {
break
}

i += read
}
}
11 changes: 11 additions & 0 deletions runtime/vm_function_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,4 +253,15 @@ func() {
}()
}()
}()`, nil, 15)

// function skipping return
expect(t, `out = func() {}()`, nil, objects.UndefinedValue)
expect(t, `out = func(v) { if v { return true } }(1)`, nil, true)
expect(t, `out = func(v) { if v { return true } }(0)`, nil, objects.UndefinedValue)
expect(t, `out = func(v) { if v { } else { return true } }(1)`, nil, objects.UndefinedValue)
expect(t, `out = func(v) { if v { return } }(1)`, nil, objects.UndefinedValue)
expect(t, `out = func(v) { if v { return } }(0)`, nil, objects.UndefinedValue)
expect(t, `out = func(v) { if v { } else { return } }(1)`, nil, objects.UndefinedValue)
expect(t, `out = func(v) { for ;;v++ { if v == 3 { return true } } }(1)`, nil, true)
expect(t, `out = func(v) { for ;;v++ { if v == 3 { break } } }(1)`, nil, objects.UndefinedValue)
}
4 changes: 4 additions & 0 deletions runtime/vm_if_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,8 @@ func() {
out = a
}()
`, nil, 3)

// expression statement in init (should not leave objects on stack)
expect(t, `a := 1; if a; a { out = a }`, nil, 1)
expect(t, `a := 1; if a + 4; a { out = a }`, nil, 1)
}
8 changes: 8 additions & 0 deletions runtime/vm_module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,14 @@ export func(a) {
a()
}
`), "Runtime Error: not callable: int\n\tat mod1:3:4\n\tat test:4:1")

// module skipping export
expect(t, `out = import("mod0")`, Opts().Module("mod0", ``), objects.UndefinedValue)
expect(t, `out = import("mod0")`, Opts().Module("mod0", `if 1 { export true }`), true)
expect(t, `out = import("mod0")`, Opts().Module("mod0", `if 0 { export true }`), objects.UndefinedValue)
expect(t, `out = import("mod0")`, Opts().Module("mod0", `if 1 { } else { export true }`), objects.UndefinedValue)
expect(t, `out = import("mod0")`, Opts().Module("mod0", `for v:=0;;v++ { if v == 3 { export true } } }`), true)
expect(t, `out = import("mod0")`, Opts().Module("mod0", `for v:=0;;v++ { if v == 3 { break } } }`), objects.UndefinedValue)
}

func TestModuleBlockScopes(t *testing.T) {
Expand Down
12 changes: 1 addition & 11 deletions script/compiled_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,19 +84,9 @@ func TestCompiled_RunContext(t *testing.T) {
assert.NoError(t, err)
compiledGet(t, c, "a", int64(5))

// cancelled
c = compile(t, `for true {}`, nil)
ctx, cancel := context.WithCancel(context.Background())
go func() {
time.Sleep(1 * time.Millisecond)
cancel()
}()
err = c.RunContext(ctx)
assert.Equal(t, context.Canceled, err)

// timeout
c = compile(t, `for true {}`, nil)
ctx, cancel = context.WithTimeout(context.Background(), 1*time.Millisecond)
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond)
defer cancel()
err = c.RunContext(ctx)
assert.Equal(t, context.DeadlineExceeded, err)
Expand Down

0 comments on commit 01fe30f

Please sign in to comment.