Repository: d5/tengo
Pull request #302 (2020-06-07 13:27:53)

spread last call argument v2
This PR is created in favor of feedback in #299 
- No new OpCode is introduced.
- OpCall has two operands now. Existing compiled scripts do not work, recompilation is required.
- OpCall second operand is used to denote whether last stack object is expanded/spread or not, which must be valid tengo Object `*Array, *ImmutableArray` or for custom user types `Spreader`.


d5: It looks good to me. But I'm not sure if we need a Spreader interface given that it will always be array type, which is not difficult to compose. Do you have any good use cases for that?
ozanh: I don't have a solid use case for Spreader, it can be removed. User can expose any method to return an array.
d5: Also, I'd appreciate it if you could update documentation as well.
ozanh: I will remove Spreader interface and add documentation tomorrow :+1: 
ozanh: I hope docs are OK because I am not good at English. I added some missing notes about CLI and keyword selectors as well.
d5: @geseq please review this PR and feel free to merge/release. This should be `v2.6.0`.
geseq: @ozanh can you post the before and after fib(35) benchmarks in cmd/bench/main.go?
ozanh: @geseq  cmd/bench/main.go results
**OLD**
```
-------------------------------------
fibonacci(35)
-------------------------------------
Result:  9227465
Go:      72.629763ms
Parser:  14.569µs
Compile: 75.709µs
VM:      3.015807538s
-------------------------------------
fibonacci(35) (tail-call #1)
-------------------------------------
Result:  9227465
Go:      65.526133ms
Parser:  16.425µs
Compile: 66.364µs
VM:      3.17805909s
-------------------------------------
fibonacci(35) (tail-call #2)
-------------------------------------
Result:  9227465
Go:      230ns
Parser:  16.279µs
Compile: 60.879µs
VM:      25.062µs
```

**NEW**
```
-------------------------------------
fibonacci(35)
-------------------------------------
Result:  9227465
Go:      62.885487ms
Parser:  15.205µs
Compile: 79.682µs
VM:      2.998246515s
-------------------------------------
fibonacci(35) (tail-call #1)
-------------------------------------
Result:  9227465
Go:      64.565828ms
Parser:  16.822µs
Compile: 68.22µs
VM:      3.084532036s
-------------------------------------
fibonacci(35) (tail-call #2)
-------------------------------------
Result:  9227465
Go:      245ns
Parser:  16.211µs
Compile: 57.697µs
VM:      11.635µs
```
These are my benchstat comparisons using similar fibonacci functions
```
name      old time/op    new time/op    delta
Fib-8        2.98s ± 1%     2.98s ± 1%    ~     (p=0.436 n=10+10)
FibTC1-8     3.17s ± 1%     3.11s ± 1%  -1.72%  (p=0.000 n=10+10)
FibTC2-8    53.9µs ± 1%    54.0µs ± 1%    ~     (p=0.143 n=10+10)

name      old alloc/op   new alloc/op   delta
Fib-8        313MB ± 0%     313MB ± 0%    ~     (p=0.881 n=9+9)
FibTC1-8     358MB ± 0%     358MB ± 0%  +0.00%  (p=0.000 n=9+10)
FibTC2-8     126kB ± 0%     126kB ± 0%  +0.06%  (p=0.000 n=10+9)

name      old allocs/op  new allocs/op  delta
Fib-8        39.1M ± 0%     39.1M ± 0%  +0.00%  (p=0.003 n=10+9)
FibTC1-8     44.8M ± 0%     44.8M ± 0%  +0.00%  (p=0.000 n=9+10)
FibTC2-8       462 ± 0%       467 ± 0%  +1.08%  (p=0.000 n=10+10)
```

Patch:
diff --git a/compiler.go b/compiler.go
index cb1c8f3..1937a1d 100644
--- a/compiler.go
+++ b/compiler.go
@@ -506,7 +506,11 @@ func (c *Compiler) Compile(node parser.Node) error {
 				return err
 			}
 		}
-		c.emit(node, parser.OpCall, len(node.Args))
+		ellipsis := 0
+		if node.Ellipsis.IsValid() {
+			ellipsis = 1
+		}
+		c.emit(node, parser.OpCall, len(node.Args), ellipsis)
 	case *parser.ImportExpr:
 		if node.ModuleName == "" {
 			return c.errorf(node, "empty module name")
@@ -526,7 +530,7 @@ func (c *Compiler) Compile(node parser.Node) error {
 					return err
 				}
 				c.emit(node, parser.OpConstant, c.addConstant(compiled))
-				c.emit(node, parser.OpCall, 0)
+				c.emit(node, parser.OpCall, 0, 0)
 			case Object: // builtin module
 				c.emit(node, parser.OpConstant, c.addConstant(v))
 			default:
@@ -556,7 +560,7 @@ func (c *Compiler) Compile(node parser.Node) error {
 				return err
 			}
 			c.emit(node, parser.OpConstant, c.addConstant(compiled))
-			c.emit(node, parser.OpCall, 0)
+			c.emit(node, parser.OpCall, 0, 0)
 		} else {
 			return c.errorf(node, "module '%s' not found", node.ModuleName)
 		}
diff --git a/docs/tengo-cli.md b/docs/tengo-cli.md
index 2e7179e..ec45b9b 100644
--- a/docs/tengo-cli.md
+++ b/docs/tengo-cli.md
@@ -52,6 +52,12 @@ chmod +x myapp.tengo
 
 **Note: Your source file must have `.tengo` extension.**
 
+## Resolving Relative Import Paths
+
+If there are tengo source module files which are imported with relative import
+paths, CLI has `-resolve` flag. Flag enables to import a module relative to
+importing file. This behavior will be default at version 3.
+
 ## Tengo REPL
 
 You can run Tengo [REPL](https://en.wikipedia.org/wiki/Read–eval–print_loop)
diff --git a/docs/tutorial.md b/docs/tutorial.md
index 75b7594..9f98358 100644
--- a/docs/tutorial.md
+++ b/docs/tutorial.md
@@ -194,6 +194,30 @@ Only the last parameter can be variadic. The following code is also illegal:
 illegal := func(a..., b) { /*... */ }
 ```
 
+When calling a function, the number of passing arguments must match that of
+function definition.
+
+```golang
+f := func(a, b) {}
+f(1, 2, 3) // Runtime Error: wrong number of arguments: want=2, got=3
+```
+
+Like Go, you can use ellipsis `...` to pass array-type value as its last parameter:
+
+```golang
+f1 := func(a, b, c) { return a + b + c }
+f1([1, 2, 3]...)    // => 6
+f1(1, [2, 3]...)    // => 6
+f1(1, 2, [3]...)    // => 6
+f1([1, 2]...)       // Runtime Error: wrong number of arguments: want=3, got=2
+
+f2 := func(a, ...b) {}
+f2(1)               // valid; a = 1, b = []
+f2(1, 2)            // valid; a = 1, b = [2]
+f2(1, 2, 3)         // valid; a = 1, b = [2, 3]
+f2([1, 2, 3]...)    // valid; a = 1, b = [2, 3]
+```
+
 ## Variables and Scopes
 
 A value can be assigned to a variable using assignment operator `:=` and `=`.
@@ -382,6 +406,20 @@ d := "hello world"[2:10]     // == "llo worl"
 c := [1, 2, 3, 4, 5][-1:10]  // == [1, 2, 3, 4, 5]
 ```
 
+**Note: Keywords cannot be used as selectors.**
+
+```golang
+a := {in: true} // Parse Error: expected map key, found 'in'
+a.func = ""     // Parse Error: expected selector, found 'func'
+```
+
+Use double quotes and indexer to use keywords with maps.
+
+```golang
+a := {"in": true}
+a["func"] = ""
+```
+
 ## Statements
 
 ### If Statement
diff --git a/parser/expr.go b/parser/expr.go
index 71e5155..b6b6c62 100644
--- a/parser/expr.go
+++ b/parser/expr.go
@@ -111,10 +111,11 @@ func (e *BoolLit) String() string {
 
 // CallExpr represents a function call expression.
 type CallExpr struct {
-	Func   Expr
-	LParen Pos
-	Args   []Expr
-	RParen Pos
+	Func     Expr
+	LParen   Pos
+	Args     []Expr
+	Ellipsis Pos
+	RParen   Pos
 }
 
 func (e *CallExpr) exprNode() {}
@@ -134,6 +135,9 @@ func (e *CallExpr) String() string {
 	for _, e := range e.Args {
 		args = append(args, e.String())
 	}
+	if len(args) > 0 && e.Ellipsis.IsValid() {
+		args[len(args)-1] = args[len(args)-1] + "..."
+	}
 	return e.Func.String() + "(" + strings.Join(args, ", ") + ")"
 }
 
diff --git a/parser/opcodes.go b/parser/opcodes.go
index a4fbfba..d97f489 100644
--- a/parser/opcodes.go
+++ b/parser/opcodes.go
@@ -120,7 +120,7 @@ var OpcodeOperands = [...][]int{
 	OpImmutable:     {},
 	OpIndex:         {},
 	OpSliceIndex:    {},
-	OpCall:          {1},
+	OpCall:          {1, 1},
 	OpReturn:        {1},
 	OpGetLocal:      {1},
 	OpSetLocal:      {1},
diff --git a/parser/parser.go b/parser/parser.go
index 501a910..fd20423 100644
--- a/parser/parser.go
+++ b/parser/parser.go
@@ -270,9 +270,13 @@ func (p *Parser) parseCall(x Expr) *CallExpr {
 	p.exprLevel++
 
 	var list []Expr
-	for p.token != token.RParen && p.token != token.EOF {
+	var ellipsis Pos
+	for p.token != token.RParen && p.token != token.EOF && !ellipsis.IsValid() {
 		list = append(list, p.parseExpr())
-
+		if p.token == token.Ellipsis {
+			ellipsis = p.pos
+			p.next()
+		}
 		if !p.expectComma(token.RParen, "call argument") {
 			break
 		}
@@ -281,10 +285,11 @@ func (p *Parser) parseCall(x Expr) *CallExpr {
 	p.exprLevel--
 	rparen := p.expect(token.RParen)
 	return &CallExpr{
-		Func:   x,
-		LParen: lparen,
-		RParen: rparen,
-		Args:   list,
+		Func:     x,
+		LParen:   lparen,
+		RParen:   rparen,
+		Ellipsis: ellipsis,
+		Args:     list,
 	}
 }
 
diff --git a/vm.go b/vm.go
index af8783f..811ecef 100644
--- a/vm.go
+++ b/vm.go
@@ -537,12 +537,36 @@ func (v *VM) run() {
 			}
 		case parser.OpCall:
 			numArgs := int(v.curInsts[v.ip+1])
-			v.ip++
+			spread := int(v.curInsts[v.ip+2])
+			v.ip += 2
+
 			value := v.stack[v.sp-1-numArgs]
 			if !value.CanCall() {
 				v.err = fmt.Errorf("not callable: %s", value.TypeName())
 				return
 			}
+
+			if spread == 1 {
+				v.sp--
+				switch arr := v.stack[v.sp].(type) {
+				case *Array:
+					for _, item := range arr.Value {
+						v.stack[v.sp] = item
+						v.sp++
+					}
+					numArgs += len(arr.Value) - 1
+				case *ImmutableArray:
+					for _, item := range arr.Value {
+						v.stack[v.sp] = item
+						v.sp++
+					}
+					numArgs += len(arr.Value) - 1
+				default:
+					v.err = fmt.Errorf("not an array: %s", arr.TypeName())
+					return
+				}
+			}
+
 			if callee, ok := value.(*CompiledFunction); ok {
 				if callee.VarArgs {
 					// if the closure is variadic,


Test patch:
diff --git a/compiler_test.go b/compiler_test.go
index 09f609f..905fa15 100644
--- a/compiler_test.go
+++ b/compiler_test.go
@@ -483,6 +483,25 @@ func TestCompiler_Compile(t *testing.T) {
 				intObject(3),
 				intObject(0))))
 
+	expectCompile(t, `f1 := func(a) { return a }; f1([1, 2]...);`,
+		bytecode(
+			concatInsts(
+				tengo.MakeInstruction(parser.OpConstant, 0),
+				tengo.MakeInstruction(parser.OpSetGlobal, 0),
+				tengo.MakeInstruction(parser.OpGetGlobal, 0),
+				tengo.MakeInstruction(parser.OpConstant, 1),
+				tengo.MakeInstruction(parser.OpConstant, 2),
+				tengo.MakeInstruction(parser.OpArray, 2),
+				tengo.MakeInstruction(parser.OpCall, 1, 1),
+				tengo.MakeInstruction(parser.OpPop),
+				tengo.MakeInstruction(parser.OpSuspend)),
+			objectsArray(
+				compiledFunction(1, 1,
+					tengo.MakeInstruction(parser.OpGetLocal, 0),
+					tengo.MakeInstruction(parser.OpReturn, 1)),
+				intObject(1),
+				intObject(2))))
+
 	expectCompile(t, `func() { return 5 + 10 }`,
 		bytecode(
 			concatInsts(
@@ -601,7 +620,7 @@ func TestCompiler_Compile(t *testing.T) {
 		bytecode(
 			concatInsts(
 				tengo.MakeInstruction(parser.OpConstant, 1),
-				tengo.MakeInstruction(parser.OpCall, 0),
+				tengo.MakeInstruction(parser.OpCall, 0, 0),
 				tengo.MakeInstruction(parser.OpPop),
 				tengo.MakeInstruction(parser.OpSuspend)),
 			objectsArray(
@@ -615,7 +634,7 @@ func TestCompiler_Compile(t *testing.T) {
 		bytecode(
 			concatInsts(
 				tengo.MakeInstruction(parser.OpConstant, 1),
-				tengo.MakeInstruction(parser.OpCall, 0),
+				tengo.MakeInstruction(parser.OpCall, 0, 0),
 				tengo.MakeInstruction(parser.OpPop),
 				tengo.MakeInstruction(parser.OpSuspend)),
 			objectsArray(
@@ -630,7 +649,7 @@ func TestCompiler_Compile(t *testing.T) {
 				tengo.MakeInstruction(parser.OpConstant, 1),
 				tengo.MakeInstruction(parser.OpSetGlobal, 0),
 				tengo.MakeInstruction(parser.OpGetGlobal, 0),
-				tengo.MakeInstruction(parser.OpCall, 0),
+				tengo.MakeInstruction(parser.OpCall, 0, 0),
 				tengo.MakeInstruction(parser.OpPop),
 				tengo.MakeInstruction(parser.OpSuspend)),
 			objectsArray(
@@ -646,7 +665,7 @@ func TestCompiler_Compile(t *testing.T) {
 				tengo.MakeInstruction(parser.OpConstant, 1),
 				tengo.MakeInstruction(parser.OpSetGlobal, 0),
 				tengo.MakeInstruction(parser.OpGetGlobal, 0),
-				tengo.MakeInstruction(parser.OpCall, 0),
+				tengo.MakeInstruction(parser.OpCall, 0, 0),
 				tengo.MakeInstruction(parser.OpPop),
 				tengo.MakeInstruction(parser.OpSuspend)),
 			objectsArray(
@@ -710,7 +729,7 @@ func TestCompiler_Compile(t *testing.T) {
 				tengo.MakeInstruction(parser.OpSetGlobal, 0),
 				tengo.MakeInstruction(parser.OpGetGlobal, 0),
 				tengo.MakeInstruction(parser.OpConstant, 1),
-				tengo.MakeInstruction(parser.OpCall, 1),
+				tengo.MakeInstruction(parser.OpCall, 1, 0),
 				tengo.MakeInstruction(parser.OpPop),
 				tengo.MakeInstruction(parser.OpSuspend)),
 			objectsArray(
@@ -728,7 +747,7 @@ func TestCompiler_Compile(t *testing.T) {
 				tengo.MakeInstruction(parser.OpConstant, 1),
 				tengo.MakeInstruction(parser.OpConstant, 2),
 				tengo.MakeInstruction(parser.OpConstant, 3),
-				tengo.MakeInstruction(parser.OpCall, 3),
+				tengo.MakeInstruction(parser.OpCall, 3, 0),
 				tengo.MakeInstruction(parser.OpPop),
 				tengo.MakeInstruction(parser.OpSuspend)),
 			objectsArray(
@@ -746,7 +765,7 @@ func TestCompiler_Compile(t *testing.T) {
 				tengo.MakeInstruction(parser.OpConstant, 1),
 				tengo.MakeInstruction(parser.OpConstant, 2),
 				tengo.MakeInstruction(parser.OpConstant, 3),
-				tengo.MakeInstruction(parser.OpCall, 3),
+				tengo.MakeInstruction(parser.OpCall, 3, 0),
 				tengo.MakeInstruction(parser.OpPop),
 				tengo.MakeInstruction(parser.OpSuspend)),
 			objectsArray(
@@ -782,7 +801,7 @@ func TestCompiler_Compile(t *testing.T) {
 			concatInsts(
 				tengo.MakeInstruction(parser.OpGetBuiltin, 0),
 				tengo.MakeInstruction(parser.OpArray, 0),
-				tengo.MakeInstruction(parser.OpCall, 1),
+				tengo.MakeInstruction(parser.OpCall, 1, 0),
 				tengo.MakeInstruction(parser.OpPop),
 				tengo.MakeInstruction(parser.OpSuspend)),
 			objectsArray()))
@@ -797,7 +816,7 @@ func TestCompiler_Compile(t *testing.T) {
 				compiledFunction(0, 0,
 					tengo.MakeInstruction(parser.OpGetBuiltin, 0),
 					tengo.MakeInstruction(parser.OpArray, 0),
-					tengo.MakeInstruction(parser.OpCall, 1),
+					tengo.MakeInstruction(parser.OpCall, 1, 0),
 					tengo.MakeInstruction(parser.OpReturn, 1)))))
 
 	expectCompile(t, `func(a) { func(b) { return a + b } }`,
diff --git a/parser/parser_test.go b/parser/parser_test.go
index 476d4e3..4ee6ad7 100644
--- a/parser/parser_test.go
+++ b/parser/parser_test.go
@@ -290,12 +290,23 @@ func TestParseCall(t *testing.T) {
 			exprStmt(
 				callExpr(
 					ident("add", p(1, 1)),
-					p(1, 4), p(1, 12),
+					p(1, 4), p(1, 12), NoPos,
 					intLit(1, p(1, 5)),
 					intLit(2, p(1, 8)),
 					intLit(3, p(1, 11)))))
 	})
 
+	expectParse(t, "add(1, 2, v...)", func(p pfn) []Stmt {
+		return stmts(
+			exprStmt(
+				callExpr(
+					ident("add", p(1, 1)),
+					p(1, 4), p(1, 15), p(1, 12),
+					intLit(1, p(1, 5)),
+					intLit(2, p(1, 8)),
+					ident("v", p(1, 11)))))
+	})
+
 	expectParse(t, "a = add(1, 2, 3)", func(p pfn) []Stmt {
 		return stmts(
 			assignStmt(
@@ -304,7 +315,7 @@ func TestParseCall(t *testing.T) {
 				exprs(
 					callExpr(
 						ident("add", p(1, 5)),
-						p(1, 8), p(1, 16),
+						p(1, 8), p(1, 16), NoPos,
 						intLit(1, p(1, 9)),
 						intLit(2, p(1, 12)),
 						intLit(3, p(1, 15)))),
@@ -321,7 +332,7 @@ func TestParseCall(t *testing.T) {
 				exprs(
 					callExpr(
 						ident("add", p(1, 8)),
-						p(1, 11), p(1, 19),
+						p(1, 11), p(1, 19), NoPos,
 						intLit(1, p(1, 12)),
 						intLit(2, p(1, 15)),
 						intLit(3, p(1, 18)))),
@@ -334,7 +345,7 @@ func TestParseCall(t *testing.T) {
 			exprStmt(
 				callExpr(
 					ident("add", p(1, 1)),
-					p(1, 4), p(1, 26),
+					p(1, 4), p(1, 26), NoPos,
 					binaryExpr(
 						ident("a", p(1, 5)),
 						intLit(1, p(1, 9)),
@@ -381,7 +392,7 @@ func TestParseCall(t *testing.T) {
 									ident("b", p(1, 18)),
 									token.Add,
 									p(1, 16))))),
-					p(1, 21), p(1, 26),
+					p(1, 21), p(1, 26), NoPos,
 					intLit(1, p(1, 22)),
 					intLit(2, p(1, 25)))))
 	})
@@ -393,7 +404,7 @@ func TestParseCall(t *testing.T) {
 					selectorExpr(
 						ident("a", p(1, 1)),
 						stringLit("b", p(1, 3))),
-					p(1, 4), p(1, 5))))
+					p(1, 4), p(1, 5), NoPos)))
 	})
 
 	expectParse(t, `a.b.c()`, func(p pfn) []Stmt {
@@ -405,7 +416,7 @@ func TestParseCall(t *testing.T) {
 							ident("a", p(1, 1)),
 							stringLit("b", p(1, 3))),
 						stringLit("c", p(1, 5))),
-					p(1, 6), p(1, 7))))
+					p(1, 6), p(1, 7), NoPos)))
 	})
 
 	expectParse(t, `a["b"].c()`, func(p pfn) []Stmt {
@@ -418,8 +429,17 @@ func TestParseCall(t *testing.T) {
 							stringLit("b", p(1, 3)),
 							p(1, 2), p(1, 6)),
 						stringLit("c", p(1, 8))),
-					p(1, 9), p(1, 10))))
+					p(1, 9), p(1, 10), NoPos)))
 	})
+
+	expectParseError(t, `add(...a, 1)`)
+	expectParseError(t, `add(a..., 1)`)
+	expectParseError(t, `add(a..., b...)`)
+	expectParseError(t, `add(1, a..., b...)`)
+	expectParseError(t, `add(...)`)
+	expectParseError(t, `add(1, ...)`)
+	expectParseError(t, `add(1, ..., )`)
+	expectParseError(t, `add(...a)`)
 }
 
 func TestParseChar(t *testing.T) {
@@ -1001,7 +1021,7 @@ func TestParseImport(t *testing.T) {
 					selectorExpr(
 						importExpr("mod1", p(1, 1)),
 						stringLit("func1", p(1, 16))),
-					p(1, 21), p(1, 22))))
+					p(1, 21), p(1, 22), NoPos)))
 	})
 
 	expectParse(t, `for x, y in import("mod1") {}`, func(p pfn) []Stmt {
@@ -1753,10 +1773,11 @@ func parenExpr(x Expr, lparen, rparen Pos) *ParenExpr {
 
 func callExpr(
 	f Expr,
-	lparen, rparen Pos,
+	lparen, rparen, ellipsis Pos,
 	args ...Expr,
 ) *CallExpr {
-	return &CallExpr{Func: f, LParen: lparen, RParen: rparen, Args: args}
+	return &CallExpr{Func: f, LParen: lparen, RParen: rparen,
+		Ellipsis: ellipsis, Args: args}
 }
 
 func indexExpr(
diff --git a/vm_test.go b/vm_test.go
index b996280..c20fcd1 100644
--- a/vm_test.go
+++ b/vm_test.go
@@ -3472,6 +3472,63 @@ func() {
 }()`, nil, 25)
 }
 
+func TestSpread(t *testing.T) {
+	expectRun(t, `
+	f := func(...a) {
+		return append(a, 3)
+	}
+	out = f([1, 2]...)
+	`, nil, ARR{1, 2, 3})
+
+	expectRun(t, `
+	f := func(a, ...b) {
+		return append([a], append(b, 3)...)
+	}
+	out = f([1, 2]...)
+	`, nil, ARR{1, 2, 3})
+
+	expectRun(t, `
+	f := func(a, ...b) {
+		return append(append([a], b), 3)
+	}
+	out = f(1, [2]...)
+	`, nil, ARR{1, ARR{2}, 3})
+
+	expectRun(t, `
+	f1 := func(...a){
+		return append([3], a...)
+	}
+	f2 := func(a, ...b) {
+		return f1(append([a], b...)...)
+	}
+	out = f2([1, 2]...)
+	`, nil, ARR{3, 1, 2})
+
+	expectRun(t, `
+	f := func(a, ...b) {
+		return func(...a) {
+			return append([3], append(a, 4)...)
+		}(a, b...)
+	}
+	out = f([1, 2]...)
+	`, nil, ARR{3, 1, 2, 4})
+
+	expectRun(t, `
+	f := func(a, ...b) {
+		c := append(b, 4)
+		return func(){
+			return append(append([a], b...), c...)
+		}()
+	}
+	out = f(1, immutable([2, 3])...)
+	`, nil, ARR{1, 2, 3, 2, 3, 4})
+
+	expectError(t, `func(a) {}([1, 2]...)`, nil,
+		"Runtime Error: wrong number of arguments: want=1, got=2")
+	expectError(t, `func(a, b, c) {}([1, 2]...)`, nil,
+		"Runtime Error: wrong number of arguments: want=3, got=2")
+}
+
 func expectRun(
 	t *testing.T,
 	input string,