Repository: google/starlark-go

Issue:
Add (*starlark.Function).Predeclared()
👋 I'm writing an interactive debugger for Starlark. I've found some of the previous PRs in the area pretty helpful, including #539. However, it doesn't make sense to me why `(*starlark.Function).Globals()` [exists](https://pkg.go.dev/go.starlark.net/starlark#Function.Globals) but not  `(*starlark.Function).Predeclared()`.

While it's true that the embedder of Starlark provides the predeclared dict in the first place, Starlark allows using a different predeclared dict for every module. It would be a bit onerous to thread the predeclared dict for every module through to the debugger, when the debugger technically already has access to the dict through `thread.DebugFrame(i).Callable().(*starlark.Function).module.predeclared`.

In terms of what this `Predeclared` method should return, I considered returning `fn.module.predeclared`, but that dictionary is the actual dictionary used for execution, and modification would be ill-advised. Since StringDict has no shallow-freeze mechanism, it's probably better for `Predeclared` to return a clone of `fn.module.predeclared`.

Related to #304.

Hints from issue comments:
@timothyg-stripe: See #595

Solution patch:
diff --git a/starlark/eval.go b/starlark/eval.go
index be3eb61..83e1834 100644
--- a/starlark/eval.go
+++ b/starlark/eval.go
@@ -435,7 +435,7 @@ func CompiledProgram(in io.Reader) (*Program, error) {
 // executes the toplevel code of the specified program,
 // and returns a new, unfrozen dictionary of the globals.
 func (prog *Program) Init(thread *Thread, predeclared StringDict) (StringDict, error) {
-	toplevel := makeToplevelFunction(prog.compiled, predeclared)
+	toplevel := makeToplevelFunction(prog, predeclared)
 
 	_, err := Call(thread, toplevel, nil, nil)
 
@@ -474,7 +474,7 @@ func ExecREPLChunk(f *syntax.File, thread *Thread, globals StringDict) error {
 
 	// -- variant of Program.Init --
 
-	toplevel := makeToplevelFunction(prog.compiled, predeclared)
+	toplevel := makeToplevelFunction(prog, predeclared)
 
 	// Initialize module globals from parameter.
 	for i, id := range prog.compiled.Globals {
@@ -495,10 +495,10 @@ func ExecREPLChunk(f *syntax.File, thread *Thread, globals StringDict) error {
 	return err
 }
 
-func makeToplevelFunction(prog *compile.Program, predeclared StringDict) *Function {
+func makeToplevelFunction(prog *Program, predeclared StringDict) *Function {
 	// Create the Starlark value denoted by each program constant c.
-	constants := make([]Value, len(prog.Constants))
-	for i, c := range prog.Constants {
+	constants := make([]Value, len(prog.compiled.Constants))
+	for i, c := range prog.compiled.Constants {
 		var v Value
 		switch c := c.(type) {
 		case int64:
@@ -518,11 +518,11 @@ func makeToplevelFunction(prog *compile.Program, predeclared StringDict) *Functi
 	}
 
 	return &Function{
-		funcode: prog.Toplevel,
-		module: &module{
+		funcode: prog.compiled.Toplevel,
+		module: &Module{
 			program:     prog,
 			predeclared: predeclared,
-			globals:     make([]Value, len(prog.Globals)),
+			globals:     make([]Value, len(prog.compiled.Globals)),
 			constants:   constants,
 		},
 	}
@@ -612,7 +612,8 @@ func makeExprFunc(opts *syntax.FileOptions, expr syntax.Expr, env StringDict) (*
 		return nil, err
 	}
 
-	return makeToplevelFunction(compile.Expr(opts, expr, "<expr>", locals), env), nil
+	prog := compile.Expr(opts, expr, "<expr>", locals)
+	return makeToplevelFunction(&Program{prog}, env), nil
 }
 
 // The following functions are primitive operations of the byte code interpreter.
diff --git a/starlark/value.go b/starlark/value.go
index d89a9f5..7a3ce8c 100644
--- a/starlark/value.go
+++ b/starlark/value.go
@@ -696,25 +696,31 @@ func (*stringCodepointsIterator) Done() {}
 // The initialization behavior of a Starlark module is also represented by a Function.
 type Function struct {
 	funcode  *compile.Funcode
-	module   *module
+	module   *Module
 	defaults Tuple
 	freevars Tuple
 }
 
-// A module is the dynamic counterpart to a Program.
-// All functions in the same program share a module.
-type module struct {
-	program     *compile.Program
+// A Module represents an evaluated Starlark module.
+// It is the dynamic counterpart to a [Program].
+// All functions in the same program share a Module.
+type Module struct {
+	program     *Program
 	predeclared StringDict
 	globals     []Value
 	constants   []Value
 }
 
-// makeGlobalDict returns a new, unfrozen StringDict containing all global
+// Program returns the program from which this module was constructed.
+func (m *Module) Program() *Program {
+	return m.program
+}
+
+// Globals returns a new StringDict containing all global
 // variables so far defined in the module.
-func (m *module) makeGlobalDict() StringDict {
-	r := make(StringDict, len(m.program.Globals))
-	for i, id := range m.program.Globals {
+func (m *Module) Globals() StringDict {
+	r := make(StringDict, len(m.program.compiled.Globals))
+	for i, id := range m.program.compiled.Globals {
 		if v := m.globals[i]; v != nil {
 			r[id.Name] = v
 		}
@@ -722,6 +728,12 @@ func (m *module) makeGlobalDict() StringDict {
 	return r
 }
 
+// Predeclared returns the predeclared environment used
+// to construct this module.
+func (m *Module) Predeclared() StringDict {
+	return m.predeclared
+}
+
 func (fn *Function) Name() string          { return fn.funcode.Name } // "lambda" for anonymous functions
 func (fn *Function) Doc() string           { return fn.funcode.Doc }
 func (fn *Function) Hash() (uint32, error) { return hashString(fn.funcode.Name), nil }
@@ -729,10 +741,13 @@ func (fn *Function) Freeze()               { fn.defaults.Freeze(); fn.freevars.F
 func (fn *Function) String() string        { return toString(fn) }
 func (fn *Function) Type() string          { return "function" }
 func (fn *Function) Truth() Bool           { return true }
+func (fn *Function) Module() *Module       { return fn.module }
 
-// Globals returns a new, unfrozen StringDict containing all global
+// Globals returns a new StringDict containing all global
 // variables so far defined in the function's module.
-func (fn *Function) Globals() StringDict { return fn.module.makeGlobalDict() }
+//
+// fn.Globals() is equivalent to fn.Module().Globals().
+func (fn *Function) Globals() StringDict { return fn.module.Globals() }
 
 func (fn *Function) Position() syntax.Position { return fn.funcode.Pos }
 func (fn *Function) NumParams() int            { return fn.funcode.NumParams }



Test patch:
diff --git a/starlark/eval_test.go b/starlark/eval_test.go
index 70ac241..637c30d 100644
--- a/starlark/eval_test.go
+++ b/starlark/eval_test.go
@@ -828,6 +828,61 @@ def somefunc():
 	}
 }
 
+func TestFunction_Globals(t *testing.T) {
+	predeclared := starlark.StringDict{
+		"predecl": starlark.String("foo"),
+	}
+	globals, _ := starlark.ExecFile(&starlark.Thread{}, "func.star", `
+def somefunc():
+	return predecl
+`, predeclared)
+
+	fn := globals["somefunc"].(*starlark.Function)
+	fnGlobals := fn.Globals()
+	if !reflect.DeepEqual(fnGlobals, globals) {
+		t.Errorf("somefunc.Globals(): got %q, want %q", fnGlobals, globals)
+	}
+	fnGlobals["shouldnt_appear"] = starlark.String("something_new")
+	if globals.Has("shouldnt_appear") {
+		t.Errorf("somefunc.Globals(): Globals() reflected onto ExecFile's returned globals")
+	}
+}
+
+func TestFunction_Module(t *testing.T) {
+	predeclared := starlark.StringDict{
+		"predecl": starlark.String("foo"),
+	}
+	globals, _ := starlark.ExecFile(&starlark.Thread{}, "func.star", `
+def somefunc1():
+	return predecl
+
+def somefunc2():
+	return 42
+`, predeclared)
+
+	fn1 := globals["somefunc1"].(*starlark.Function)
+	fn2 := globals["somefunc2"].(*starlark.Function)
+	if fn1.Module() != fn2.Module() {
+		t.Errorf("somefunc1 and somefunc2 have different modules: %p vs. %p", fn1.Module(), fn2.Module())
+	}
+
+	module := fn1.Module()
+
+	moduleGlobals := module.Globals()
+	if !reflect.DeepEqual(moduleGlobals, globals) {
+		t.Errorf("module.Globals(): got %q, want %q", moduleGlobals, globals)
+	}
+	moduleGlobals["shouldnt_appear"] = starlark.String("something_new")
+	if globals.Has("shouldnt_appear") {
+		t.Errorf("module.Globals(): Globals() reflected onto ExecFile's returned globals")
+	}
+
+	modulePredeclared := module.Predeclared()
+	if reflect.ValueOf(modulePredeclared).UnsafePointer() != reflect.ValueOf(predeclared).UnsafePointer() {
+		t.Errorf("module.Predeclared(): got %p=%q, want %p=%q", modulePredeclared, modulePredeclared, predeclared, predeclared)
+	}
+}
+
 func TestFrameLocals(t *testing.T) {
 	// trace prints a nice stack trace including argument
 	// values of calls to Starlark functions.