aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--resolve/resolve.go52
-rw-r--r--resolve/testdata/resolve.star18
-rw-r--r--starlark/eval.go153
-rw-r--r--starlark/eval_test.go14
-rw-r--r--starlark/testdata/function.star2
-rw-r--r--syntax/parse.go2
-rw-r--r--syntax/testdata/errors.star18
7 files changed, 138 insertions, 121 deletions
diff --git a/resolve/resolve.go b/resolve/resolve.go
index 4b104c3..4933e3d 100644
--- a/resolve/resolve.go
+++ b/resolve/resolve.go
@@ -735,56 +735,60 @@ func (r *resolver) function(pos syntax.Position, name string, function *syntax.F
r.push(b)
const allowRebind = false
- var seenVarargs, seenKwargs, seenOptional bool
+ var seenOptional bool
+ var star, starStar *syntax.Ident // *args, **kwargs
for _, param := range function.Params {
switch param := param.(type) {
case *syntax.Ident:
// e.g. x
- if seenKwargs {
- r.errorf(pos, "parameter may not follow **kwargs")
- } else if seenVarargs {
- r.errorf(pos, "parameter may not follow *args")
+ if starStar != nil {
+ r.errorf(param.NamePos, "required parameter may not follow **%s", starStar.Name)
+ } else if star != nil {
+ r.errorf(param.NamePos, "required parameter may not follow * parameter")
} else if seenOptional {
- r.errorf(pos, "required parameter may not follow optional")
+ r.errorf(param.NamePos, "required parameter may not follow optional")
}
if r.bind(param, allowRebind) {
- r.errorf(pos, "duplicate parameter: %s", param.Name)
+ r.errorf(param.NamePos, "duplicate parameter: %s", param.Name)
}
case *syntax.BinaryExpr:
// e.g. y=dflt
- if seenKwargs {
- r.errorf(pos, "parameter may not follow **kwargs")
- } else if seenVarargs {
- r.errorf(pos, "parameter may not follow *args")
+ if starStar != nil {
+ r.errorf(param.OpPos, "optional parameter may not follow **%s", starStar.Name)
+ } else if star != nil {
+ r.errorf(param.OpPos, "optional parameter may not follow *%s", star.Name)
}
if id := param.X.(*syntax.Ident); r.bind(id, allowRebind) {
- r.errorf(pos, "duplicate parameter: %s", id.Name)
+ r.errorf(param.OpPos, "duplicate parameter: %s", id.Name)
}
seenOptional = true
case *syntax.UnaryExpr:
// *args or **kwargs
+ id := param.X.(*syntax.Ident)
if param.Op == syntax.STAR {
- if seenKwargs {
- r.errorf(pos, "*args may not follow **kwargs")
- } else if seenVarargs {
- r.errorf(pos, "multiple *args not allowed")
+ if starStar != nil {
+ r.errorf(param.OpPos, "*%s parameter may not follow **%s", id.Name, starStar.Name)
+ } else if star != nil {
+ r.errorf(param.OpPos, "multiple * parameters not allowed")
+ } else {
+ star = id
}
- seenVarargs = true
} else {
- if seenKwargs {
- r.errorf(pos, "multiple **kwargs not allowed")
+ if starStar != nil {
+ r.errorf(param.OpPos, "multiple ** parameters not allowed")
+ } else {
+ starStar = id
}
- seenKwargs = true
}
- if id := param.X.(*syntax.Ident); r.bind(id, allowRebind) {
- r.errorf(pos, "duplicate parameter: %s", id.Name)
+ if r.bind(id, allowRebind) {
+ r.errorf(id.NamePos, "duplicate parameter: %s", id.Name)
}
}
}
- function.HasVarargs = seenVarargs
- function.HasKwargs = seenKwargs
+ function.HasVarargs = star != nil
+ function.HasKwargs = starStar != nil
r.stmts(function.Body)
// Resolve all uses of this function's local vars,
diff --git a/resolve/testdata/resolve.star b/resolve/testdata/resolve.star
index 1a83115..9d19869 100644
--- a/resolve/testdata/resolve.star
+++ b/resolve/testdata/resolve.star
@@ -184,31 +184,31 @@ M(x=1, 2) ### `positional argument may not follow named`
def f(x=1, y): pass ### `required parameter may not follow optional`
---
-# No parameters may follow **kwargs
+# No parameters may follow **kwargs in a declaration.
def f(**kwargs, x): ### `parameter may not follow \*\*kwargs`
pass
-def g(**kwargs, *args): ### `\*args may not follow \*\*kwargs`
+def g(**kwargs, *args): ### `\*args parameter may not follow \*\*kwargs`
pass
-def h(**kwargs1, **kwargs2): ### `multiple \*\*kwargs not allowed`
+def h(**kwargs1, **kwargs2): ### `multiple \*\* parameters not allowed`
pass
---
-# Only **kwargs may follow *args
+# Only keyword-only params and **kwargs may follow *args in a declaration.
-def f(*args, x): ### `parameter may not follow \*args`
+def f(*args, x): ### `required parameter may not follow \* parameter`
pass
-def g(*args1, *args2): ### `multiple \*args not allowed`
+def g(*args1, *args2): ### `multiple \* parameters not allowed`
pass
-def h(*args, **kwargs): # ok
+def h(*args, a=1, **kwargs): ### `optional parameter may not follow \*args`
pass
---
-# No arguments may follow **kwargs
+# No arguments may follow **kwargs in a call.
def f(*args, **kwargs):
pass
@@ -218,7 +218,7 @@ f(**{}, *[]) ### `\*args may not follow \*\*kwargs`
f(**{}, **{}) ### `multiple \*\*kwargs not allowed`
---
-# Only keyword arguments may follow *args
+# Only keyword arguments may follow *args in a call.
def f(*args, **kwargs):
pass
diff --git a/starlark/eval.go b/starlark/eval.go
index 70d4414..9ed9e27 100644
--- a/starlark/eval.go
+++ b/starlark/eval.go
@@ -1011,6 +1011,16 @@ func asIndex(v Value, len int, result *int) error {
// setArgs sets the values of the formal parameters of function fn in
// based on the actual parameter values in args and kwargs.
func setArgs(locals []Value, fn *Function, args Tuple, kwargs []Tuple) error {
+ // This is adapted from the algorithm from PyEval_EvalCodeEx.
+
+ // Nullary function?
+ if fn.NumParams() == 0 {
+ if nactual := len(args) + len(kwargs); nactual > 0 {
+ return fmt.Errorf("function %s takes no arguments (%d given)", fn.Name(), nactual)
+ }
+ return nil
+ }
+
cond := func(x bool, y, z interface{}) interface{} {
if x {
return y
@@ -1020,101 +1030,94 @@ func setArgs(locals []Value, fn *Function, args Tuple, kwargs []Tuple) error {
// nparams is the number of ordinary parameters (sans * or **).
nparams := fn.NumParams()
- if fn.HasVarargs() {
+ var kwdict *Dict
+ if fn.HasKwargs() {
nparams--
+ kwdict = new(Dict)
+ locals[nparams] = kwdict
}
- if fn.HasKwargs() {
+ if fn.HasVarargs() {
nparams--
}
- // This is the algorithm from PyEval_EvalCodeEx.
- var kwdict *Dict
+ // Too many positional args?
n := len(args)
- if nparams > 0 || fn.HasVarargs() || fn.HasKwargs() {
- if fn.HasKwargs() {
- kwdict = new(Dict)
- locals[fn.NumParams()-1] = kwdict
- }
-
- // too many args?
- if len(args) > nparams {
- if !fn.HasVarargs() {
- return fmt.Errorf("function %s takes %s %d argument%s (%d given)",
- fn.Name(),
- cond(len(fn.defaults) > 0, "at most", "exactly"),
- nparams,
- cond(nparams == 1, "", "s"),
- len(args)+len(kwargs))
- }
- n = nparams
+ maxpos := nparams
+ if len(args) > maxpos {
+ if !fn.HasVarargs() {
+ return fmt.Errorf("function %s takes %s %d positional argument%s (%d given)",
+ fn.Name(),
+ cond(len(fn.defaults) > 0, "at most", "exactly"),
+ maxpos,
+ cond(nparams == 1, "", "s"),
+ len(args)+len(kwargs))
}
+ n = maxpos
+ }
- // set of defined (regular) parameters
- var defined intset
- defined.init(nparams)
+ // set of defined (regular) parameters
+ var defined intset
+ defined.init(nparams)
- // ordinary parameters
- for i := 0; i < n; i++ {
- locals[i] = args[i]
- defined.set(i)
- }
+ // ordinary parameters
+ for i := 0; i < n; i++ {
+ locals[i] = args[i]
+ defined.set(i)
+ }
- // variadic arguments
- if fn.HasVarargs() {
- tuple := make(Tuple, len(args)-n)
- for i := n; i < len(args); i++ {
- tuple[i-n] = args[i]
- }
- locals[nparams] = tuple
+ // variadic arguments
+ if fn.HasVarargs() {
+ tuple := make(Tuple, len(args)-n)
+ for i := n; i < len(args); i++ {
+ tuple[i-n] = args[i]
}
+ locals[nparams] = tuple
+ }
- // keyword arguments
- paramIdents := fn.funcode.Locals[:nparams]
- for _, pair := range kwargs {
- k, v := pair[0].(String), pair[1]
- if i := findParam(paramIdents, string(k)); i >= 0 {
- if defined.set(i) {
- return fmt.Errorf("function %s got multiple values for keyword argument %s", fn.Name(), k)
- }
- locals[i] = v
- continue
- }
- if kwdict == nil {
- return fmt.Errorf("function %s got an unexpected keyword argument %s", fn.Name(), k)
- }
- n := kwdict.Len()
- kwdict.SetKey(k, v)
- if kwdict.Len() == n {
+ // keyword arguments
+ paramIdents := fn.funcode.Locals[:nparams]
+ for _, pair := range kwargs {
+ k, v := pair[0].(String), pair[1]
+ if i := findParam(paramIdents, string(k)); i >= 0 {
+ if defined.set(i) {
return fmt.Errorf("function %s got multiple values for keyword argument %s", fn.Name(), k)
}
+ locals[i] = v
+ continue
}
+ if kwdict == nil {
+ return fmt.Errorf("function %s got an unexpected keyword argument %s", fn.Name(), k)
+ }
+ n := kwdict.Len()
+ kwdict.SetKey(k, v)
+ if kwdict.Len() == n {
+ return fmt.Errorf("function %s got multiple values for keyword argument %s", fn.Name(), k)
+ }
+ }
- // default values
- if len(args) < nparams {
- m := nparams - len(fn.defaults) // first default
-
- // report errors for missing non-optional arguments
- i := len(args)
- for ; i < m; i++ {
- if !defined.get(i) {
- return fmt.Errorf("function %s takes %s %d argument%s (%d given)",
- fn.Name(),
- cond(fn.HasVarargs() || len(fn.defaults) > 0, "at least", "exactly"),
- m,
- cond(m == 1, "", "s"),
- defined.len())
- }
+ // default values
+ if n < nparams {
+ m := nparams - len(fn.defaults) // first default
+
+ // report errors for missing non-optional arguments
+ i := n
+ for ; i < m; i++ {
+ if !defined.get(i) {
+ return fmt.Errorf("function %s takes %s %d positional argument%s (%d given)",
+ fn.Name(),
+ cond(fn.HasVarargs() || len(fn.defaults) > 0, "at least", "exactly"),
+ m,
+ cond(m == 1, "", "s"),
+ defined.len())
}
+ }
- // set default values
- for ; i < nparams; i++ {
- if !defined.get(i) {
- locals[i] = fn.defaults[i-m]
- }
+ // set default values
+ for ; i < nparams; i++ {
+ if !defined.get(i) {
+ locals[i] = fn.defaults[i-m]
}
}
- } else if nactual := len(args) + len(kwargs); nactual > 0 {
- return fmt.Errorf("function %s takes no arguments (%d given)", fn.Name(), nactual)
}
return nil
}
diff --git a/starlark/eval_test.go b/starlark/eval_test.go
index dde92f5..09c6bfa 100644
--- a/starlark/eval_test.go
+++ b/starlark/eval_test.go
@@ -258,11 +258,11 @@ def f(a, b=42, *args, **kwargs):
for _, test := range []struct{ src, want string }{
{`a()`, `None`},
{`a(1)`, `function a takes no arguments (1 given)`},
- {`b()`, `function b takes exactly 2 arguments (0 given)`},
- {`b(1)`, `function b takes exactly 2 arguments (1 given)`},
+ {`b()`, `function b takes exactly 2 positional arguments (0 given)`},
+ {`b(1)`, `function b takes exactly 2 positional arguments (1 given)`},
{`b(1, 2)`, `(1, 2)`},
{`b`, `<function b>`}, // asserts that b's parameter b was treated as a local variable
- {`b(1, 2, 3)`, `function b takes exactly 2 arguments (3 given)`},
+ {`b(1, 2, 3)`, `function b takes exactly 2 positional arguments (3 given)`},
{`b(1, b=2)`, `(1, 2)`},
{`b(1, a=2)`, `function b got multiple values for keyword argument "a"`},
{`b(1, x=2)`, `function b got an unexpected keyword argument "x"`},
@@ -270,10 +270,10 @@ def f(a, b=42, *args, **kwargs):
{`b(b=1, a=2)`, `(2, 1)`},
{`b(b=1, a=2, x=1)`, `function b got an unexpected keyword argument "x"`},
{`b(x=1, b=1, a=2)`, `function b got an unexpected keyword argument "x"`},
- {`c()`, `function c takes at least 1 argument (0 given)`},
+ {`c()`, `function c takes at least 1 positional argument (0 given)`},
{`c(1)`, `(1, 42)`},
{`c(1, 2)`, `(1, 2)`},
- {`c(1, 2, 3)`, `function c takes at most 2 arguments (3 given)`},
+ {`c(1, 2, 3)`, `function c takes at most 2 positional arguments (3 given)`},
{`c(1, b=2)`, `(1, 2)`},
{`c(1, a=2)`, `function c got multiple values for keyword argument "a"`},
{`c(a=1, b=2)`, `(1, 2)`},
@@ -284,10 +284,10 @@ def f(a, b=42, *args, **kwargs):
{`d(1, 2, k=3)`, `function d got an unexpected keyword argument "k"`},
{`d(args=[])`, `function d got an unexpected keyword argument "args"`},
{`e()`, `{}`},
- {`e(1)`, `function e takes exactly 0 arguments (1 given)`},
+ {`e(1)`, `function e takes exactly 0 positional arguments (1 given)`},
{`e(k=1)`, `{"k": 1}`},
{`e(kwargs={})`, `{"kwargs": {}}`},
- {`f()`, `function f takes at least 1 argument (0 given)`},
+ {`f()`, `function f takes at least 1 positional argument (0 given)`},
{`f(0)`, `(0, 42, (), {})`},
{`f(0)`, `(0, 42, (), {})`},
{`f(0, 1)`, `(0, 1, (), {})`},
diff --git a/starlark/testdata/function.star b/starlark/testdata/function.star
index 5d5fe37..daa87e4 100644
--- a/starlark/testdata/function.star
+++ b/starlark/testdata/function.star
@@ -161,7 +161,7 @@ assert.fails(lambda: f(
33, 34, 35, 36, 37, 38, 39, 40,
41, 42, 43, 44, 45, 46, 47, 48,
49, 50, 51, 52, 53, 54, 55, 56,
- 57, 58, 59, 60, 61, 62, 63, 64), "takes exactly 65 arguments .64 given.")
+ 57, 58, 59, 60, 61, 62, 63, 64), "takes exactly 65 positional arguments .64 given.")
assert.fails(lambda: f(
1, 2, 3, 4, 5, 6, 7, 8,
diff --git a/syntax/parse.go b/syntax/parse.go
index 82c6ab0..af79cc1 100644
--- a/syntax/parse.go
+++ b/syntax/parse.go
@@ -11,7 +11,7 @@ package syntax
// package. Verify that error positions are correct using the
// chunkedfile mechanism.
-import log "log"
+import "log"
// Enable this flag to print the token stream and log.Fatal on the first error.
const debug = false
diff --git a/syntax/testdata/errors.star b/syntax/testdata/errors.star
index 558845c..6ff3ea6 100644
--- a/syntax/testdata/errors.star
+++ b/syntax/testdata/errors.star
@@ -27,6 +27,12 @@ def f(**kwargs, ): ### `got '\)', want parameter`
---
+# Parameters are validated later.
+def f(**kwargs, *args, b=1, a, **kwargs, *args, b=1, a):
+ pass
+
+---
+
def pass(): ### "not an identifier"
pass
@@ -48,6 +54,10 @@ f(**kwargs, ) ### `got '\)', want argument`
---
+f(a=1, *, b=2) ### `got ',', want primary`
+
+---
+
_ = {x:y for y in z} # ok
_ = {x for y in z} ### `got for, want ':'`
@@ -109,7 +119,7 @@ _ = 0 == 1 == 2 ### "== does not associate with =="
---
_ = (0 <= i) < n # ok
-_ = 0 <= (i < n) # ok
+_ = 0 <= (i < n) # ok
_ = 0 <= i < n ### "<= does not associate with <"
---
@@ -152,14 +162,14 @@ load("a", x2="x", "y") # => positional-before-named arg check happens later (!)
# 'load' is not an identifier
load = 1 ### `got '=', want '\('`
---
-# 'load' is not an identifier
+# 'load' is not an identifier
f(load()) ### `got load, want primary`
---
-# 'load' is not an identifier
+# 'load' is not an identifier
def load(): ### `not an identifier`
pass
---
-# 'load' is not an identifier
+# 'load' is not an identifier
def f(load): ### `not an identifier`
pass
---
generated by cgit v1.2.3 (git 2.25.1) at 2024-07-04 20:54:30 +0000