Revert "starlark: add debug API for Locals and FreeVars"

This reverts commit b533e7f.

internal/compile/compile.go

@@ -335,7 +335,7 @@ type Funcode struct {
 	pclinetab             []uint16        // mapping from pc to linenum
 	Locals                []Binding       // locals, parameters first
 	Cells                 []int           // indices of Locals that require cells
-	FreeVars              []Binding       // for tracing
+	Freevars              []Binding       // for tracing
 	MaxStack              int
 	NumParams             int
 	NumKwonlyParams       int
@@ -520,7 +520,7 @@ func (pcomp *pcomp) function(name string, pos syntax.Position, stmts []syntax.St
 			Name:     name,
 			Doc:      docStringFromBody(stmts),
 			Locals:   bindings(locals),
-			FreeVars: bindings(freevars),
+			Freevars: bindings(freevars),
 		},
 	}
 
@@ -887,7 +887,7 @@ func PrintOp(fn *Funcode, pc uint32, op Opcode, arg uint32) {
 	case ATTR, SETFIELD, PREDECLARED, UNIVERSAL:
 		comment = fn.Prog.Names[arg]
 	case FREE:
-		comment = fn.FreeVars[arg].Name
+		comment = fn.Freevars[arg].Name
 	case CALL, CALL_VAR, CALL_KW, CALL_VAR_KW:
 		comment = fmt.Sprintf("%d pos, %d named", arg>>8, arg&0xff)
 	default:

internal/compile/serial.go

@@ -195,7 +195,7 @@ func (e *encoder) function(fn *Funcode) {
 	for _, index := range fn.Cells {
 		e.int(index)
 	}
-	e.bindings(fn.FreeVars)
+	e.bindings(fn.Freevars)
 	e.int(fn.MaxStack)
 	e.int(fn.NumParams)
 	e.int(fn.NumKwonlyParams)
@@ -389,7 +389,7 @@ func (d *decoder) function() *Funcode {
 		pclinetab:       pclinetab,
 		Locals:          locals,
 		Cells:           cells,
-		FreeVars:        freevars,
+		Freevars:        freevars,
 		MaxStack:        maxStack,
 		NumParams:       numParams,
 		NumKwonlyParams: numKwonlyParams,

starlark/debug.go

@@ -1,59 +1,41 @@
 package starlark
 
-import (
-	"go.starlark.net/syntax"
-)
+import "go.starlark.net/syntax"
 
 // This file defines an experimental API for the debugging tools.
 // Some of these declarations expose details of internal packages.
 // (The debugger makes liberal use of exported fields of unexported types.)
 // Breaking changes may occur without notice.
 
-// A Binding is the name and position of a binding identifier.
-type Binding struct {
-	Name string
-	Pos  syntax.Position
-}
-
-// NumLocals returns the number of local variables of this frame.
-// It is zero unless fr.Callable() is a *Function.
-func (fr *frame) NumLocals() int { return len(fr.locals) }
-
-// Local returns the binding (name and binding position) and value of
-// the i'th local variable of the frame's function.
-// Beware: the value may be nil if it has not yet been assigned!
+// Local returns the value of the i'th local variable.
+// It may be nil if not yet assigned.
 //
-// The index i must be less than [NumLocals].
-// Local may be called only while the frame is active.
+// Local may be called only for frames whose Callable is a *Function (a
+// function defined by Starlark source code), and only while the frame
+// is active; it will panic otherwise.
 //
 // This function is provided only for debugging tools.
-func (fr *frame) Local(i int) (Binding, Value) {
-	return Binding(fr.callable.(*Function).funcode.Locals[i]), fr.locals[i]
-}
+//
+// THIS API IS EXPERIMENTAL AND MAY CHANGE WITHOUT NOTICE.
+func (fr *frame) Local(i int) Value { return fr.locals[i] }
 
 // DebugFrame is the debugger API for a frame of the interpreter's call stack.
 //
 // Most applications have no need for this API; use CallFrame instead.
 //
-// It may be tempting to use this interface when implementing built-in
-// functions. Beware that reflection over the call stack is easily
-// abused, leading to built-in functions whose behavior is mysterious
-// and unpredictable.
-//
 // Clients must not retain a DebugFrame nor call any of its methods once
 // the current built-in call has returned or execution has resumed
 // after a breakpoint as this may have unpredictable effects, including
 // but not limited to retention of object that would otherwise be garbage.
 type DebugFrame interface {
-	Callable() Callable           // returns the frame's function
-	NumLocals() int               // returns the number of local variables in this frame
-	Local(i int) (Binding, Value) // returns the binding and value of the (Starlark) frame's ith local variable
-	Position() syntax.Position    // returns the current position of execution in this frame
+	Callable() Callable        // returns the frame's function
+	Local(i int) Value         // returns the value of the (Starlark) frame's ith local variable
+	Position() syntax.Position // returns the current position of execution in this frame
 }
 
 // DebugFrame returns the debugger interface for
 // the specified frame of the interpreter's call stack.
-// Frame numbering is as for Thread.CallFrame: 0 <= depth < thread.CallStackDepth().
+// Frame numbering is as for Thread.CallFrame.
 //
 // This function is intended for use in debugging tools.
 // Most applications should have no need for it; use CallFrame instead.

starlark/eval_test.go

@@ -824,8 +824,7 @@ func TestFrameLocals(t *testing.T) {
 						buf.WriteString(", ")
 					}
 					name, _ := fn.Param(i)
-					_, v := fr.Local(i)
-					fmt.Fprintf(buf, "%s=%s", name, v)
+					fmt.Fprintf(buf, "%s=%s", name, fr.Local(i))
 				}
 			} else {
 				buf.WriteString("...") // a built-in function
@@ -1057,55 +1056,3 @@ main()
 		}()
 	}
 }
-
-func TestDebugFrame(t *testing.T) {
-	predeclared := starlark.StringDict{
-		"env": starlark.NewBuiltin("env", func(thread *starlark.Thread, b *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
-			if thread.CallStackDepth() < 2 {
-				return nil, fmt.Errorf("env must not be called directly")
-			}
-			fr := thread.DebugFrame(1) // parent
-			fn, ok := fr.Callable().(*starlark.Function)
-			if !ok {
-				return nil, fmt.Errorf("env must be called from a Starlark function")
-			}
-			dict := starlark.NewDict(0)
-			for i := 0; i < fr.NumLocals(); i++ {
-				bind, val := fr.Local(i)
-				if val == nil {
-					continue
-				}
-				dict.SetKey(starlark.String(bind.Name), val) // ignore error
-			}
-			for i := 0; i < fn.NumFreeVars(); i++ {
-				bind, val := fn.FreeVar(i)
-				dict.SetKey(starlark.String(bind.Name), val) // ignore error
-			}
-			dict.Freeze()
-			return dict, nil
-		}),
-	}
-	const src = `
-e = [None]
-
-def f(p):
-    outer = 3
-    def g(q):
-        inner = outer + 1
-        e[0] = env() # {"q": 2, "inner": 4, "outer": 3}
-	inner2 = None # not defined at call to env()
-    g(2)
-
-f(1)
-`
-	thread := new(starlark.Thread)
-	m, err := starlark.ExecFile(thread, "env.star", src, predeclared)
-	if err != nil {
-		t.Fatalf("ExecFile returned error %q, expected panic", err)
-	}
-	got := m["e"].(*starlark.List).Index(0).String()
-	want := `{"q": 2, "inner": 4, "outer": 3}`
-	if got != want {
-		t.Errorf("env() returned %s, want %s", got, want)
-	}
-}

starlark/interp.go

@@ -541,7 +541,7 @@ loop:
 		case compile.MAKEFUNC:
 			funcode := f.Prog.Functions[arg]
 			tuple := stack[sp-1].(Tuple)
-			n := len(tuple) - len(funcode.FreeVars)
+			n := len(tuple) - len(funcode.Freevars)
 			defaults := tuple[:n:n]
 			freevars := tuple[n:]
 			stack[sp-1] = &Function{
@@ -622,7 +622,7 @@ loop:
 		case compile.FREECELL:
 			v := fn.freevars[arg].(*cell).v
 			if v == nil {
-				err = fmt.Errorf("local variable %s referenced before assignment", f.FreeVars[arg].Name)
+				err = fmt.Errorf("local variable %s referenced before assignment", f.Freevars[arg].Name)
 				break loop
 			}
 			stack[sp] = v

starlark/value.go

@@ -775,15 +775,6 @@ func (fn *Function) ParamDefault(i int) Value {
 func (fn *Function) HasVarargs() bool { return fn.funcode.HasVarargs }
 func (fn *Function) HasKwargs() bool  { return fn.funcode.HasKwargs }
 
-// NumFreeVars returns the number of free variables of this function.
-func (fn *Function) NumFreeVars() int { return len(fn.funcode.FreeVars) }
-
-// FreeVar returns the binding (name and binding position) and value
-// of the i'th free variable of function fn.
-func (fn *Function) FreeVar(i int) (Binding, Value) {
-	return Binding(fn.funcode.FreeVars[i]), fn.freevars[i].(*cell).v
-}
-
 // A Builtin is a function implemented in Go.
 type Builtin struct {
 	name string