From 8a575927616e66eb64611cf7c6b3225721bd116d Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Thu, 15 Jun 2023 11:48:59 -0400 Subject: [PATCH] syntax: parameterize API over FileOptions, avoid globals This change eliminates the need for client applications to set global variables to control dialect options, as globals are error-prone and concurrency hostile. All relevant API functions Foo now have a variant FooOptions that takes an explicit FileOptions; the original function accesses the legacy options by reading the global variables. TODO: needs more doc comments Fixes #435 --- internal/compile/codegen_test.go | 2 +- internal/compile/compile.go | 15 ++-- internal/compile/serial.go | 4 ++ lib/proto/cmd/star2proto/star2proto.go | 4 +- repl/repl.go | 31 ++++---- resolve/resolve.go | 33 +++++---- resolve/resolve_test.go | 28 +++++--- starlark/bench_test.go | 12 ++-- starlark/eval.go | 40 +++++++---- starlark/eval_test.go | 26 ++++--- starlark/interp.go | 7 +- syntax/parse.go | 98 +++++++++++++++----------- syntax/syntax.go | 6 +- 13 files changed, 188 insertions(+), 118 deletions(-) diff --git a/internal/compile/codegen_test.go b/internal/compile/codegen_test.go index f67204ff..c5954c45 100644 --- a/internal/compile/codegen_test.go +++ b/internal/compile/codegen_test.go @@ -64,7 +64,7 @@ func TestPlusFolding(t *testing.T) { t.Errorf("#%d: %v", i, err) continue } - got := disassemble(Expr(expr, "", locals).Toplevel) + got := disassemble(Expr(syntax.LegacyFileOptions(), expr, "", locals).Toplevel) if test.want != got { t.Errorf("expression <<%s>> generated <<%s>>, want <<%s>>", test.src, got, test.want) diff --git a/internal/compile/compile.go b/internal/compile/compile.go index 888d95c5..ecf689f0 100644 --- a/internal/compile/compile.go +++ b/internal/compile/compile.go @@ -23,7 +23,6 @@ // // Operands, logically uint32s, are encoded using little-endian 7-bit // varints, the top bit indicating that more bytes follow. -// package compile // import "go.starlark.net/internal/compile" import ( @@ -47,7 +46,7 @@ var Disassemble = false const debug = false // make code generation verbose, for debugging the compiler // Increment this to force recompilation of saved bytecode files. -const Version = 13 +const Version = 14 type Opcode uint8 @@ -317,6 +316,7 @@ type Program struct { Functions []*Funcode Globals []Binding // for error messages and tracing Toplevel *Funcode // module initialization function + Recursion bool // disable recursion check for functions in this file } // The type of a bytes literal value, to distinguish from text string. @@ -486,17 +486,20 @@ func bindings(bindings []*resolve.Binding) []Binding { } // Expr compiles an expression to a program whose toplevel function evaluates it. -func Expr(expr syntax.Expr, name string, locals []*resolve.Binding) *Program { +// The options must be consistent with those used when parsing expr. +func Expr(opts *syntax.FileOptions, expr syntax.Expr, name string, locals []*resolve.Binding) *Program { pos := syntax.Start(expr) stmts := []syntax.Stmt{&syntax.ReturnStmt{Result: expr}} - return File(stmts, pos, name, locals, nil) + return File(opts, stmts, pos, name, locals, nil) } // File compiles the statements of a file into a program. -func File(stmts []syntax.Stmt, pos syntax.Position, name string, locals, globals []*resolve.Binding) *Program { +// The options must be consistent with those used when parsing stmts. +func File(opts *syntax.FileOptions, stmts []syntax.Stmt, pos syntax.Position, name string, locals, globals []*resolve.Binding) *Program { pcomp := &pcomp{ prog: &Program{ - Globals: bindings(globals), + Globals: bindings(globals), + Recursion: opts.Recursion, }, names: make(map[string]uint32), constants: make(map[interface{}]uint32), diff --git a/internal/compile/serial.go b/internal/compile/serial.go index adadabfc..4d71738c 100644 --- a/internal/compile/serial.go +++ b/internal/compile/serial.go @@ -25,6 +25,7 @@ package compile // toplevel Funcode // numfuncs varint // funcs []Funcode +// recursion varint (0 or 1) // []byte # concatenation of all referenced strings // EOF // @@ -130,6 +131,7 @@ func (prog *Program) Encode() []byte { for _, fn := range prog.Functions { e.function(fn) } + e.int(b2i(prog.Recursion)) // Patch in the offset of the string data section. binary.LittleEndian.PutUint32(e.p[4:8], uint32(len(e.p))) @@ -270,6 +272,7 @@ func DecodeProgram(data []byte) (_ *Program, err error) { for i := range funcs { funcs[i] = d.function() } + recursion := d.int() != 0 prog := &Program{ Loads: loads, @@ -278,6 +281,7 @@ func DecodeProgram(data []byte) (_ *Program, err error) { Globals: globals, Functions: funcs, Toplevel: toplevel, + Recursion: recursion, } toplevel.Prog = prog for _, f := range funcs { diff --git a/lib/proto/cmd/star2proto/star2proto.go b/lib/proto/cmd/star2proto/star2proto.go index 24b5a0e4..791f2072 100644 --- a/lib/proto/cmd/star2proto/star2proto.go +++ b/lib/proto/cmd/star2proto/star2proto.go @@ -40,8 +40,10 @@ var ( // Starlark dialect flags func init() { - flag.BoolVar(&resolve.AllowFloat, "fp", true, "allow floating-point numbers") flag.BoolVar(&resolve.AllowSet, "set", resolve.AllowSet, "allow set data type") + + // obsolete, no effect: + flag.BoolVar(&resolve.AllowFloat, "fp", true, "allow floating-point numbers") flag.BoolVar(&resolve.AllowLambda, "lambda", resolve.AllowLambda, "allow lambda expressions") flag.BoolVar(&resolve.AllowNestedDef, "nesteddef", resolve.AllowNestedDef, "allow nested def statements") } diff --git a/repl/repl.go b/repl/repl.go index 94f8947e..96aadb3b 100644 --- a/repl/repl.go +++ b/repl/repl.go @@ -20,7 +20,6 @@ import ( "os/signal" "github.com/chzyer/readline" - "go.starlark.net/resolve" "go.starlark.net/starlark" "go.starlark.net/syntax" ) @@ -33,8 +32,10 @@ var interrupted = make(chan os.Signal, 1) // variable named "context" to a context.Context that is cancelled by a // SIGINT (Control-C). Client-supplied global functions may use this // context to make long-running operations interruptable. -// func REPL(thread *starlark.Thread, globals starlark.StringDict) { + REPLOptions(syntax.LegacyFileOptions(), thread, globals) +} +func REPLOptions(opts *syntax.FileOptions, thread *starlark.Thread, globals starlark.StringDict) { signal.Notify(interrupted, os.Interrupt) defer signal.Stop(interrupted) @@ -45,7 +46,7 @@ func REPL(thread *starlark.Thread, globals starlark.StringDict) { } defer rl.Close() for { - if err := rep(rl, thread, globals); err != nil { + if err := rep(opts, rl, thread, globals); err != nil { if err == readline.ErrInterrupt { fmt.Println(err) continue @@ -59,7 +60,7 @@ func REPL(thread *starlark.Thread, globals starlark.StringDict) { // // It returns an error (possibly readline.ErrInterrupt) // only if readline failed. Starlark errors are printed. -func rep(rl *readline.Instance, thread *starlark.Thread, globals starlark.StringDict) error { +func rep(opts *syntax.FileOptions, rl *readline.Instance, thread *starlark.Thread, globals starlark.StringDict) error { // Each item gets its own context, // which is cancelled by a SIGINT. // @@ -93,8 +94,14 @@ func rep(rl *readline.Instance, thread *starlark.Thread, globals starlark.String return []byte(line + "\n"), nil } + // Treat load bindings as global (like they used to be) in the REPL. + // Fixes github.com/google/starlark-go/issues/224. + opts2 := *opts + opts2.LoadBindsGlobally = true + opts = &opts2 + // parse - f, err := syntax.ParseCompoundStmt("", readline) + f, err := opts.ParseCompoundStmt("", readline) if err != nil { if eof { return io.EOF @@ -103,16 +110,9 @@ func rep(rl *readline.Instance, thread *starlark.Thread, globals starlark.String return nil } - // Treat load bindings as global (like they used to be) in the REPL. - // This is a workaround for github.com/google/starlark-go/issues/224. - // TODO(adonovan): not safe wrt concurrent interpreters. - // Come up with a more principled solution (or plumb options everywhere). - defer func(prev bool) { resolve.LoadBindsGlobally = prev }(resolve.LoadBindsGlobally) - resolve.LoadBindsGlobally = true - if expr := soleExpr(f); expr != nil { // eval - v, err := starlark.EvalExpr(thread, expr, globals) + v, err := starlark.EvalExprOptions(f.Options, thread, expr, globals) if err != nil { PrintError(err) return nil @@ -153,6 +153,9 @@ func PrintError(err error) { // suitable for use in the REPL. // Each function returned by MakeLoad accesses a distinct private cache. func MakeLoad() func(thread *starlark.Thread, module string) (starlark.StringDict, error) { + return MakeLoadOptions(syntax.LegacyFileOptions()) +} +func MakeLoadOptions(opts *syntax.FileOptions) func(thread *starlark.Thread, module string) (starlark.StringDict, error) { type entry struct { globals starlark.StringDict err error @@ -173,7 +176,7 @@ func MakeLoad() func(thread *starlark.Thread, module string) (starlark.StringDic // Load it. thread := &starlark.Thread{Name: "exec " + module, Load: thread.Load} - globals, err := starlark.ExecFile(thread, module, nil, nil) + globals, err := starlark.ExecFileOptions(opts, thread, module, nil, nil) e = &entry{globals, err} // Update the cache. diff --git a/resolve/resolve.go b/resolve/resolve.go index 09b9acde..f8e4d224 100644 --- a/resolve/resolve.go +++ b/resolve/resolve.go @@ -97,6 +97,9 @@ const doesnt = "this Starlark dialect does not " // global options // These features are either not standard Starlark (yet), or deprecated // features of the BUILD language, so we put them behind flags. +// +// Deprecated: use an explicit [syntax.FileOptions] argument instead, +// as it avoids all the usual problems of global variables. var ( AllowSet = false // allow the 'set' built-in AllowGlobalReassign = false // allow reassignment to top-level names; also, allow if/for/while at top-level @@ -130,7 +133,7 @@ func File(file *syntax.File, isPredeclared, isUniversal func(name string) bool) // REPLChunk is a generalization of the File function that supports a // non-empty initial global block, as occurs in a REPL. func REPLChunk(file *syntax.File, isGlobal, isPredeclared, isUniversal func(name string) bool) error { - r := newResolver(isGlobal, isPredeclared, isUniversal) + r := newResolver(file.Options, isGlobal, isPredeclared, isUniversal) r.stmts(file.Stmts) r.env.resolveLocalUses() @@ -156,7 +159,10 @@ func REPLChunk(file *syntax.File, isGlobal, isPredeclared, isUniversal func(name // // The isPredeclared and isUniversal predicates behave as for the File function. func Expr(expr syntax.Expr, isPredeclared, isUniversal func(name string) bool) ([]*Binding, error) { - r := newResolver(nil, isPredeclared, isUniversal) + return ExprOptions(syntax.LegacyFileOptions(), expr, isPredeclared, isUniversal) +} +func ExprOptions(opts *syntax.FileOptions, expr syntax.Expr, isPredeclared, isUniversal func(name string) bool) ([]*Binding, error) { + r := newResolver(opts, nil, isPredeclared, isUniversal) r.expr(expr) r.env.resolveLocalUses() r.resolveNonLocalUses(r.env) // globals & universals @@ -179,9 +185,10 @@ type Error struct { func (e Error) Error() string { return e.Pos.String() + ": " + e.Msg } -func newResolver(isGlobal, isPredeclared, isUniversal func(name string) bool) *resolver { +func newResolver(options *syntax.FileOptions, isGlobal, isPredeclared, isUniversal func(name string) bool) *resolver { file := new(block) return &resolver{ + options: options, file: file, env: file, isGlobal: isGlobal, @@ -193,6 +200,8 @@ func newResolver(isGlobal, isPredeclared, isUniversal func(name string) bool) *r } type resolver struct { + options *syntax.FileOptions + // env is the current local environment: // a linked list of blocks, innermost first. // The tail of the list is the file block. @@ -314,7 +323,7 @@ func (r *resolver) bind(id *syntax.Ident) bool { r.moduleGlobals = append(r.moduleGlobals, bind) } } - if ok && !AllowGlobalReassign { + if ok && !r.options.GlobalReassign { r.errorf(id.NamePos, "cannot reassign %s %s declared at %s", bind.Scope, id.Name, bind.First.NamePos) } @@ -382,7 +391,7 @@ func (r *resolver) use(id *syntax.Ident) { // We will piggyback support for the legacy semantics on the // AllowGlobalReassign flag, which is loosely related and also // required for Bazel. - if AllowGlobalReassign && r.env == r.file { + if r.options.GlobalReassign && r.env == r.file { r.useToplevel(use) return } @@ -420,7 +429,7 @@ func (r *resolver) useToplevel(use use) (bind *Binding) { r.predeclared[id.Name] = bind // save it } else if r.isUniversal(id.Name) { // use of universal name - if !AllowSet && id.Name == "set" { + if !r.options.Set && id.Name == "set" { r.errorf(id.NamePos, doesnt+"support sets") } bind = &Binding{Scope: Universal} @@ -493,7 +502,7 @@ func (r *resolver) stmt(stmt syntax.Stmt) { } case *syntax.IfStmt: - if !AllowGlobalReassign && r.container().function == nil { + if !r.options.GlobalReassign && r.container().function == nil { r.errorf(stmt.If, "if statement not within a function") } r.expr(stmt.Cond) @@ -519,7 +528,7 @@ func (r *resolver) stmt(stmt syntax.Stmt) { r.function(fn, stmt.Def) case *syntax.ForStmt: - if !AllowGlobalReassign && r.container().function == nil { + if !r.options.GlobalReassign && r.container().function == nil { r.errorf(stmt.For, "for loop not within a function") } r.expr(stmt.X) @@ -530,10 +539,10 @@ func (r *resolver) stmt(stmt syntax.Stmt) { r.loops-- case *syntax.WhileStmt: - if !AllowRecursion { + if !r.options.While { r.errorf(stmt.While, doesnt+"support while loops") } - if !AllowGlobalReassign && r.container().function == nil { + if !r.options.GlobalReassign && r.container().function == nil { r.errorf(stmt.While, "while loop not within a function") } r.expr(stmt.Cond) @@ -569,9 +578,9 @@ func (r *resolver) stmt(stmt syntax.Stmt) { } id := stmt.To[i] - if LoadBindsGlobally { + if r.options.LoadBindsGlobally { r.bind(id) - } else if r.bindLocal(id) && !AllowGlobalReassign { + } else if r.bindLocal(id) && !r.options.GlobalReassign { // "Global" in AllowGlobalReassign is a misnomer for "toplevel". // Sadly we can't report the previous declaration // as id.Binding may not be set yet. diff --git a/resolve/resolve_test.go b/resolve/resolve_test.go index 23bee215..95a9445a 100644 --- a/resolve/resolve_test.go +++ b/resolve/resolve_test.go @@ -14,11 +14,20 @@ import ( "go.starlark.net/syntax" ) -func setOptions(src string) { - resolve.AllowGlobalReassign = option(src, "globalreassign") - resolve.AllowRecursion = option(src, "recursion") - resolve.AllowSet = option(src, "set") - resolve.LoadBindsGlobally = option(src, "loadbindsglobally") +// A test may enable non-standard options by containing (e.g.) "option:recursion". +func getOptions(src string) *syntax.FileOptions { + // TODO(adonovan): use new fine-grained names. + // And share with eval_test.go + allowGlobalReassign := option(src, "globalreassign") + recursion := option(src, "recursion") + return &syntax.FileOptions{ + Set: option(src, "set"), + While: allowGlobalReassign || recursion, + TopLevelControl: allowGlobalReassign, + GlobalReassign: allowGlobalReassign, + LoadBindsGlobally: option(src, "loadbindsglobally"), + Recursion: recursion, + } } func option(chunk, name string) bool { @@ -26,18 +35,17 @@ func option(chunk, name string) bool { } func TestResolve(t *testing.T) { - defer setOptions("") filename := starlarktest.DataFile("resolve", "testdata/resolve.star") for _, chunk := range chunkedfile.Read(filename, t) { - f, err := syntax.Parse(filename, chunk.Source, 0) + // A chunk may set options by containing e.g. "option:recursion". + opts := getOptions(chunk.Source) + + f, err := opts.Parse(filename, chunk.Source, 0) if err != nil { t.Error(err) continue } - // A chunk may set options by containing e.g. "option:recursion". - setOptions(chunk.Source) - if err := resolve.File(f, isPredeclared, isUniversal); err != nil { for _, err := range err.(resolve.ErrorList) { chunk.GotError(int(err.Pos.Line), err.Msg) diff --git a/starlark/bench_test.go b/starlark/bench_test.go index e860df7a..46be8900 100644 --- a/starlark/bench_test.go +++ b/starlark/bench_test.go @@ -18,8 +18,6 @@ import ( ) func BenchmarkStarlark(b *testing.B) { - defer setOptions("") - starlark.Universe["json"] = json.Module testdata := starlarktest.DataFile("starlark", ".") @@ -36,10 +34,10 @@ func BenchmarkStarlark(b *testing.B) { b.Error(err) continue } - setOptions(string(src)) + opts := getOptions(string(src)) // Evaluate the file once. - globals, err := starlark.ExecFile(thread, filename, src, nil) + globals, err := starlark.ExecFileOptions(opts, thread, filename, src, nil) if err != nil { reportEvalError(b, err) } @@ -63,9 +61,9 @@ func BenchmarkStarlark(b *testing.B) { // It provides b.n, the number of iterations that must be executed by the function, // which is typically of the form: // -// def bench_foo(b): -// for _ in range(b.n): -// ...work... +// def bench_foo(b): +// for _ in range(b.n): +// ...work... // // It also provides stop, start, and restart methods to stop the clock in case // there is significant set-up work that should not count against the measured diff --git a/starlark/eval.go b/starlark/eval.go index 949cb934..0541b0cc 100644 --- a/starlark/eval.go +++ b/starlark/eval.go @@ -343,8 +343,11 @@ func (prog *Program) Write(out io.Writer) error { // If ExecFile fails during evaluation, it returns an *EvalError // containing a backtrace. func ExecFile(thread *Thread, filename string, src interface{}, predeclared StringDict) (StringDict, error) { + return ExecFileOptions(syntax.LegacyFileOptions(), thread, filename, src, predeclared) +} +func ExecFileOptions(opts *syntax.FileOptions, thread *Thread, filename string, src interface{}, predeclared StringDict) (StringDict, error) { // Parse, resolve, and compile a Starlark source file. - _, mod, err := SourceProgram(filename, src, predeclared.Has) + _, mod, err := SourceProgramOptions(opts, filename, src, predeclared.Has) if err != nil { return nil, err } @@ -364,7 +367,10 @@ func ExecFile(thread *Thread, filename string, src interface{}, predeclared Stri // Its typical value is predeclared.Has, // where predeclared is a StringDict of pre-declared values. func SourceProgram(filename string, src interface{}, isPredeclared func(string) bool) (*syntax.File, *Program, error) { - f, err := syntax.Parse(filename, src, 0) + return SourceProgramOptions(syntax.LegacyFileOptions(), filename, src, isPredeclared) +} +func SourceProgramOptions(opts *syntax.FileOptions, filename string, src interface{}, isPredeclared func(string) bool) (*syntax.File, *Program, error) { + f, err := opts.Parse(filename, src, 0) if err != nil { return nil, nil, err } @@ -396,7 +402,7 @@ func FileProgram(f *syntax.File, isPredeclared func(string) bool) (*Program, err } module := f.Module.(*resolve.Module) - compiled := compile.File(f.Stmts, pos, "", module.Locals, module.Globals) + compiled := compile.File(f.Options, f.Stmts, pos, "", module.Locals, module.Globals) return &Program{compiled}, nil } @@ -453,7 +459,7 @@ func ExecREPLChunk(f *syntax.File, thread *Thread, globals StringDict) error { } module := f.Module.(*resolve.Module) - compiled := compile.File(f.Stmts, pos, "", module.Locals, module.Globals) + compiled := compile.File(f.Options, f.Stmts, pos, "", module.Locals, module.Globals) prog := &Program{compiled} // -- variant of Program.Init -- @@ -523,11 +529,14 @@ func makeToplevelFunction(prog *compile.Program, predeclared StringDict) *Functi // If Eval fails during evaluation, it returns an *EvalError // containing a backtrace. func Eval(thread *Thread, filename string, src interface{}, env StringDict) (Value, error) { - expr, err := syntax.ParseExpr(filename, src, 0) + return EvalOptions(syntax.LegacyFileOptions(), thread, filename, src, env) +} +func EvalOptions(opts *syntax.FileOptions, thread *Thread, filename string, src interface{}, env StringDict) (Value, error) { + expr, err := opts.ParseExpr(filename, src, 0) if err != nil { return nil, err } - f, err := makeExprFunc(expr, env) + f, err := makeExprFunc(opts, expr, env) if err != nil { return nil, err } @@ -547,7 +556,10 @@ func Eval(thread *Thread, filename string, src interface{}, env StringDict) (Val // If Eval fails during evaluation, it returns an *EvalError // containing a backtrace. func EvalExpr(thread *Thread, expr syntax.Expr, env StringDict) (Value, error) { - fn, err := makeExprFunc(expr, env) + return EvalExprOptions(syntax.LegacyFileOptions(), thread, expr, env) +} +func EvalExprOptions(opts *syntax.FileOptions, thread *Thread, expr syntax.Expr, env StringDict) (Value, error) { + fn, err := makeExprFunc(opts, expr, env) if err != nil { return nil, err } @@ -557,21 +569,25 @@ func EvalExpr(thread *Thread, expr syntax.Expr, env StringDict) (Value, error) { // ExprFunc returns a no-argument function // that evaluates the expression whose source is src. func ExprFunc(filename string, src interface{}, env StringDict) (*Function, error) { - expr, err := syntax.ParseExpr(filename, src, 0) + return ExprFuncOptions(syntax.LegacyFileOptions(), filename, src, env) +} +func ExprFuncOptions(options *syntax.FileOptions, filename string, src interface{}, env StringDict) (*Function, error) { + expr, err := options.ParseExpr(filename, src, 0) if err != nil { return nil, err } - return makeExprFunc(expr, env) + return makeExprFunc(options, expr, env) } // makeExprFunc returns a no-argument function whose body is expr. -func makeExprFunc(expr syntax.Expr, env StringDict) (*Function, error) { - locals, err := resolve.Expr(expr, env.Has, Universe.Has) +// The options must be consistent with those used when parsing expr. +func makeExprFunc(opts *syntax.FileOptions, expr syntax.Expr, env StringDict) (*Function, error) { + locals, err := resolve.ExprOptions(opts, expr, env.Has, Universe.Has) if err != nil { return nil, err } - return makeToplevelFunction(compile.Expr(expr, "", locals), env), nil + return makeToplevelFunction(compile.Expr(opts, expr, "", locals), env), nil } // The following functions are primitive operations of the byte code interpreter. diff --git a/starlark/eval_test.go b/starlark/eval_test.go index 9ffd1794..aea24b8f 100644 --- a/starlark/eval_test.go +++ b/starlark/eval_test.go @@ -7,6 +7,7 @@ package starlark_test import ( "bytes" "fmt" + "log" "math" "os/exec" "path/filepath" @@ -20,7 +21,6 @@ import ( starlarkmath "go.starlark.net/lib/math" "go.starlark.net/lib/proto" "go.starlark.net/lib/time" - "go.starlark.net/resolve" "go.starlark.net/starlark" "go.starlark.net/starlarkstruct" "go.starlark.net/starlarktest" @@ -31,11 +31,19 @@ import ( ) // A test may enable non-standard options by containing (e.g.) "option:recursion". -func setOptions(src string) { - resolve.AllowGlobalReassign = option(src, "globalreassign") - resolve.LoadBindsGlobally = option(src, "loadbindsglobally") - resolve.AllowRecursion = option(src, "recursion") - resolve.AllowSet = option(src, "set") +func getOptions(src string) *syntax.FileOptions { + // TODO(adonovan): use new fine-grained names. + // And share with resolve_test.go + allowGlobalReassign := option(src, "globalreassign") + recursion := option(src, "recursion") + return &syntax.FileOptions{ + Set: option(src, "set"), + While: allowGlobalReassign || recursion, + TopLevelControl: allowGlobalReassign, + GlobalReassign: allowGlobalReassign, + LoadBindsGlobally: option(src, "loadbindsglobally"), + Recursion: recursion, + } } func option(chunk, name string) bool { @@ -113,7 +121,6 @@ func TestEvalExpr(t *testing.T) { } func TestExecFile(t *testing.T) { - defer setOptions("") testdata := starlarktest.DataFile("starlark", ".") thread := &starlark.Thread{Load: load} starlarktest.SetReporter(thread, t) @@ -148,9 +155,10 @@ func TestExecFile(t *testing.T) { "struct": starlark.NewBuiltin("struct", starlarkstruct.Make), } - setOptions(chunk.Source) + opts := getOptions(chunk.Source) + log.Printf("%s=%+v", filename, opts) - _, err := starlark.ExecFile(thread, filename, chunk.Source, predeclared) + _, err := starlark.ExecFileOptions(opts, thread, filename, chunk.Source, predeclared) switch err := err.(type) { case *starlark.EvalError: found := false diff --git a/starlark/interp.go b/starlark/interp.go index b41905a0..d29e5253 100644 --- a/starlark/interp.go +++ b/starlark/interp.go @@ -10,7 +10,6 @@ import ( "go.starlark.net/internal/compile" "go.starlark.net/internal/spell" - "go.starlark.net/resolve" "go.starlark.net/syntax" ) @@ -24,19 +23,19 @@ func (fn *Function) CallInternal(thread *Thread, args Tuple, kwargs []Tuple) (Va // Postcondition: args is not mutated. This is stricter than required by Callable, // but allows CALL to avoid a copy. - if !resolve.AllowRecursion { + f := fn.funcode + if !f.Prog.Recursion { // detect recursion for _, fr := range thread.stack[:len(thread.stack)-1] { // We look for the same function code, // not function value, otherwise the user could // defeat the check by writing the Y combinator. - if frfn, ok := fr.Callable().(*Function); ok && frfn.funcode == fn.funcode { + if frfn, ok := fr.Callable().(*Function); ok && frfn.funcode == f { return nil, fmt.Errorf("function %s called recursively", fn.Name()) } } } - f := fn.funcode fr := thread.frameAt(0) // Allocate space for stack and locals. diff --git a/syntax/parse.go b/syntax/parse.go index f4c8fff4..e70776a5 100644 --- a/syntax/parse.go +++ b/syntax/parse.go @@ -25,17 +25,20 @@ const ( // Parse parses the input data and returns the corresponding parse tree. // -// If src != nil, ParseFile parses the source from src and the filename +// If src != nil, Parse parses the source from src and the filename // is only used when recording position information. // The type of the argument for the src parameter must be string, // []byte, io.Reader, or FilePortion. -// If src == nil, ParseFile parses the file specified by filename. +// If src == nil, Parse parses the file specified by filename. func Parse(filename string, src interface{}, mode Mode) (f *File, err error) { + return LegacyFileOptions().Parse(filename, src, mode) +} +func (opts *FileOptions) Parse(filename string, src interface{}, mode Mode) (f *File, err error) { in, err := newScanner(filename, src, mode&RetainComments != 0) if err != nil { return nil, err } - p := parser{in: in} + p := parser{options: opts, in: in} defer p.in.recover(&err) p.nextToken() // read first lookahead token @@ -55,12 +58,15 @@ func Parse(filename string, src interface{}, mode Mode) (f *File, err error) { // The parser calls the readline function each // time it needs a new line of input. func ParseCompoundStmt(filename string, readline func() ([]byte, error)) (f *File, err error) { + return LegacyFileOptions().ParseCompoundStmt(filename, readline) +} +func (opts *FileOptions) ParseCompoundStmt(filename string, readline func() ([]byte, error)) (f *File, err error) { in, err := newScanner(filename, readline, false) if err != nil { return nil, err } - p := parser{in: in} + p := parser{options: opts, in: in} defer p.in.recover(&err) p.nextToken() // read first lookahead token @@ -79,18 +85,21 @@ func ParseCompoundStmt(filename string, readline func() ([]byte, error)) (f *Fil } } - return &File{Path: filename, Stmts: stmts}, nil + return &File{Options: opts, Path: filename, Stmts: stmts}, nil } // ParseExpr parses a Starlark expression. // A comma-separated list of expressions is parsed as a tuple. // See Parse for explanation of parameters. func ParseExpr(filename string, src interface{}, mode Mode) (expr Expr, err error) { + return LegacyFileOptions().ParseExpr(filename, src, mode) +} +func (opts *FileOptions) ParseExpr(filename string, src interface{}, mode Mode) (expr Expr, err error) { in, err := newScanner(filename, src, mode&RetainComments != 0) if err != nil { return nil, err } - p := parser{in: in} + p := parser{options: opts, in: in} defer p.in.recover(&err) p.nextToken() // read first lookahead token @@ -112,9 +121,10 @@ func ParseExpr(filename string, src interface{}, mode Mode) (expr Expr, err erro } type parser struct { - in *scanner - tok Token - tokval tokenValue + options *FileOptions + in *scanner + tok Token + tokval tokenValue } // nextToken advances the scanner and returns the position of the @@ -139,7 +149,7 @@ func (p *parser) parseFile() *File { } stmts = p.parseStmt(stmts) } - return &File{Stmts: stmts} + return &File{Options: p.options, Stmts: stmts} } func (p *parser) parseStmt(stmts []Stmt) []Stmt { @@ -275,10 +285,11 @@ func (p *parser) parseSimpleStmt(stmts []Stmt, consumeNL bool) []Stmt { } // small_stmt = RETURN expr? -// | PASS | BREAK | CONTINUE -// | LOAD ... -// | expr ('=' | '+=' | '-=' | '*=' | '/=' | '%=' | '&=' | '|=' | '^=' | '<<=' | '>>=') expr // assign -// | expr +// +// | PASS | BREAK | CONTINUE +// | LOAD ... +// | expr ('=' | '+=' | '-=' | '*=' | '/=' | '%=' | '&=' | '|=' | '^=' | '<<=' | '>>=') expr // assign +// | expr func (p *parser) parseSmallStmt() Stmt { switch p.tok { case RETURN: @@ -415,21 +426,23 @@ func (p *parser) consume(t Token) Position { } // params = (param COMMA)* param COMMA? -// | +// +// | // // param = IDENT -// | IDENT EQ test -// | STAR -// | STAR IDENT -// | STARSTAR IDENT +// +// | IDENT EQ test +// | STAR +// | STAR IDENT +// | STARSTAR IDENT // // parseParams parses a parameter list. The resulting expressions are of the form: // -// *Ident x -// *Binary{Op: EQ, X: *Ident, Y: Expr} x=y -// *Unary{Op: STAR} * -// *Unary{Op: STAR, X: *Ident} *args -// *Unary{Op: STARSTAR, X: *Ident} **kwargs +// *Ident x +// *Binary{Op: EQ, X: *Ident, Y: Expr} x=y +// *Unary{Op: STAR} * +// *Unary{Op: STAR, X: *Ident} *args +// *Unary{Op: STARSTAR, X: *Ident} **kwargs func (p *parser) parseParams() []Expr { var params []Expr for p.tok != RPAREN && p.tok != COLON && p.tok != EOF { @@ -651,9 +664,10 @@ func init() { } // primary_with_suffix = primary -// | primary '.' IDENT -// | primary slice_suffix -// | primary call_suffix +// +// | primary '.' IDENT +// | primary slice_suffix +// | primary call_suffix func (p *parser) parsePrimaryWithSuffix() Expr { x := p.parsePrimary() for { @@ -770,12 +784,13 @@ func (p *parser) parseArgs() []Expr { return args } -// primary = IDENT -// | INT | FLOAT | STRING | BYTES -// | '[' ... // list literal or comprehension -// | '{' ... // dict literal or comprehension -// | '(' ... // tuple or parenthesized expression -// | ('-'|'+'|'~') primary_with_suffix +// primary = IDENT +// +// | INT | FLOAT | STRING | BYTES +// | '[' ... // list literal or comprehension +// | '{' ... // dict literal or comprehension +// | '(' ... // tuple or parenthesized expression +// | ('-'|'+'|'~') primary_with_suffix func (p *parser) parsePrimary() Expr { switch p.tok { case IDENT: @@ -836,9 +851,10 @@ func (p *parser) parsePrimary() Expr { } // list = '[' ']' -// | '[' expr ']' -// | '[' expr expr_list ']' -// | '[' expr (FOR loop_variables IN expr)+ ']' +// +// | '[' expr ']' +// | '[' expr expr_list ']' +// | '[' expr (FOR loop_variables IN expr)+ ']' func (p *parser) parseList() Expr { lbrack := p.nextToken() if p.tok == RBRACK { @@ -865,8 +881,9 @@ func (p *parser) parseList() Expr { } // dict = '{' '}' -// | '{' dict_entry_list '}' -// | '{' dict_entry FOR loop_variables IN expr '}' +// +// | '{' dict_entry_list '}' +// | '{' dict_entry FOR loop_variables IN expr '}' func (p *parser) parseDict() Expr { lbrace := p.nextToken() if p.tok == RBRACE { @@ -904,8 +921,9 @@ func (p *parser) parseDictEntry() *DictEntry { } // comp_suffix = FOR loopvars IN expr comp_suffix -// | IF expr comp_suffix -// | ']' or ')' (end) +// +// | IF expr comp_suffix +// | ']' or ')' (end) // // There can be multiple FOR/IF clauses; the first is always a FOR. func (p *parser) parseComprehensionSuffix(lbrace Position, body Expr, endBrace Token) Expr { diff --git a/syntax/syntax.go b/syntax/syntax.go index 37566375..0911f542 100644 --- a/syntax/syntax.go +++ b/syntax/syntax.go @@ -70,7 +70,8 @@ type File struct { Path string Stmts []Stmt - Module interface{} // a *resolve.Module, set by resolver + Module interface{} // a *resolve.Module, set by resolver + Options *FileOptions } func (x *File) Span() (start, end Position) { @@ -99,9 +100,10 @@ func (*LoadStmt) stmt() {} func (*ReturnStmt) stmt() {} // An AssignStmt represents an assignment: +// // x = 0 // x, y = y, x -// x += 1 +// x += 1 type AssignStmt struct { commentsRef OpPos Position