Builtin: push a Frame on the Thread's stack even when calling Builtins

eval.go

@@ -62,11 +62,12 @@ func (thread *Thread) Local(key string) interface{} {
 	return thread.locals[key]
 }
 
-// Caller returns the frame of the innermost enclosing Skylark function.
+// Caller returns the frame of the caller of the current function.
 // It should only be used in built-ins called from Skylark code.
-func (thread *Thread) Caller() *Frame {
-	return thread.frame
-}
+func (thread *Thread) Caller() *Frame { return thread.frame.parent }
+
+// TopFrame returns the topmost stack frame.
+func (thread *Thread) TopFrame() *Frame { return thread.frame }
 
 // A StringDict is a mapping from names to values, and represents
 // an environment such as the global variables of a module.
@@ -107,10 +108,10 @@ func (d StringDict) Has(key string) bool { _, ok := d[key]; return ok }
 // A Frame holds the execution state of a single Skylark function call
 // or module toplevel.
 type Frame struct {
-	parent *Frame          // caller's frame (or nil)
-	posn   syntax.Position // source position of PC, set during error
-	callpc uint32          // PC of position of active call, set during call
-	fn     *Function       // current function (or toplevel)
+	parent   *Frame          // caller's frame (or nil)
+	posn     syntax.Position // source position of PC, set during error
+	callpc   uint32          // PC of position of active call, set during call
+	callable Callable        // current function (or toplevel) or built-in
 }
 
 func (fr *Frame) errorf(posn syntax.Position, format string, args ...interface{}) *EvalError {
@@ -124,11 +125,16 @@ func (fr *Frame) Position() syntax.Position {
 	if fr.posn.IsValid() {
 		return fr.posn // leaf frame only (the error)
 	}
-	return fr.fn.funcode.Position(fr.callpc) // position of active call
+	if fn, ok := fr.callable.(*Function); ok {
+		return fn.funcode.Position(fr.callpc) // position of active call
+	}
+	return syntax.MakePosition(&builtinFilename, 1, 0)
 }
 
-// Function returns the frame's function, or nil for the top-level of a module.
-func (fr *Frame) Function() *Function { return fr.fn }
+var builtinFilename = "<builtin>"
+
+// Function returns the frame's function or built-in.
+func (fr *Frame) Callable() Callable { return fr.callable }
 
 // Parent returns the frame of the enclosing function call, if any.
 func (fr *Frame) Parent() *Frame { return fr.parent }
@@ -157,7 +163,7 @@ func (fr *Frame) WriteBacktrace(out *bytes.Buffer) {
 	print = func(fr *Frame) {
 		if fr != nil {
 			print(fr.parent)
-			fmt.Fprintf(out, "  %s: in %s\n", fr.Position(), fr.fn.Name())
+			fmt.Fprintf(out, "  %s: in %s\n", fr.Position(), fr.Callable().Name())
 		}
 	}
 	print(fr)

eval_test.go

@@ -167,7 +167,8 @@ func load(thread *skylark.Thread, module string) (skylark.StringDict, error) {
 	}
 
 	// TODO(adonovan): test load() using this execution path.
-	filename := filepath.Join(filepath.Dir(thread.Caller().Position().Filename()), module)
+	filename := filepath.Join(filepath.Dir(thread.
+		Caller().Position().Filename()), module)
 	return skylark.ExecFile(thread, filename, nil, nil)
 }
 
@@ -322,7 +323,7 @@ f()
 	print := func(thread *skylark.Thread, msg string) {
 		caller := thread.Caller()
 		fmt.Fprintf(buf, "%s: %s: %s\n",
-			caller.Position(), caller.Function().Name(), msg)
+			caller.Position(), caller.Callable().Name(), msg)
 	}
 	thread := &skylark.Thread{Print: print}
 	if _, err := skylark.ExecFile(thread, "foo.go", src, nil); err != nil {
@@ -420,17 +421,18 @@ def i(): return h()
 i()
 `
 	thread := new(skylark.Thread)
-	_, err := skylark.ExecFile(thread, "crash.go", src, nil)
+	_, err := skylark.ExecFile(thread, "crash.sky", src, nil)
 	switch err := err.(type) {
 	case *skylark.EvalError:
 		got := err.Backtrace()
 		// Compiled code currently has no column information.
 		const want = `Traceback (most recent call last):
-  crash.go:6: in <toplevel>
-  crash.go:5: in i
-  crash.go:4: in h
-  crash.go:3: in g
-  crash.go:2: in f
+  crash.sky:6: in <toplevel>
+  crash.sky:5: in i
+  crash.sky:4: in h
+  <builtin>:1: in min
+  crash.sky:3: in g
+  crash.sky:2: in f
 Error: floored division by zero`
 		if got != want {
 			t.Errorf("error was %s, want %s", got, want)

interp.go

@@ -28,28 +28,29 @@ func (fn *Function) Call(thread *Thread, args Tuple, kwargs []Tuple) (Value, err
 		// We look for the same function code,
 		// not function value, otherwise the user could
 		// defeat the check by writing the Y combinator.
-		if fr.fn != nil && fr.fn.funcode == fn.funcode {
+		if frfn, ok := fr.Callable().(*Function); ok && frfn.funcode == fn.funcode {
 			return nil, fmt.Errorf("function %s called recursively", fn.Name())
 		}
 	}
 
-	thread.frame = &Frame{parent: thread.frame, fn: fn}
+	thread.frame = &Frame{parent: thread.frame, callable: fn}
 	result, err := call(thread, args, kwargs)
 	thread.frame = thread.frame.parent
 	return result, err
 }
 
 func call(thread *Thread, args Tuple, kwargs []Tuple) (Value, error) {
 	fr := thread.frame
-	f := fr.fn.funcode
+	fn := fr.callable.(*Function)
+	f := fn.funcode
 	nlocals := len(f.Locals)
 	stack := make([]Value, nlocals+f.MaxStack)
 	locals := stack[:nlocals:nlocals] // local variables, starting with parameters
 	stack = stack[nlocals:]
 
-	err := setArgs(locals, fr.fn, args, kwargs)
+	err := setArgs(locals, fn, args, kwargs)
 	if err != nil {
-		return nil, fr.errorf(fr.fn.Position(), "%v", err)
+		return nil, fr.errorf(fr.Position(), "%v", err)
 	}
 
 	if vmdebug {
@@ -432,7 +433,7 @@ loop:
 			sp--
 
 		case compile.CONSTANT:
-			stack[sp] = fr.fn.constants[arg]
+			stack[sp] = fn.constants[arg]
 			sp++
 
 		case compile.MAKETUPLE:
@@ -458,9 +459,9 @@ loop:
 			sp -= 2
 			stack[sp] = &Function{
 				funcode:     funcode,
-				predeclared: fr.fn.predeclared,
-				globals:     fr.fn.globals,
-				constants:   fr.fn.constants,
+				predeclared: fn.predeclared,
+				globals:     fn.globals,
+				constants:   fn.constants,
 				defaults:    defaults,
 				freevars:    freevars,
 			}
@@ -497,7 +498,7 @@ loop:
 			sp--
 
 		case compile.SETGLOBAL:
-			fr.fn.globals[arg] = stack[sp-1]
+			fn.globals[arg] = stack[sp-1]
 			sp--
 
 		case compile.LOCAL:
@@ -510,11 +511,11 @@ loop:
 			sp++
 
 		case compile.FREE:
-			stack[sp] = fr.fn.freevars[arg]
+			stack[sp] = fn.freevars[arg]
 			sp++
 
 		case compile.GLOBAL:
-			x := fr.fn.globals[arg]
+			x := fn.globals[arg]
 			if x == nil {
 				err = fmt.Errorf("global variable %s referenced before assignment", f.Prog.Globals[arg].Name)
 				break loop
@@ -524,7 +525,7 @@ loop:
 
 		case compile.PREDECLARED:
 			name := f.Prog.Names[arg]
-			x := fr.fn.predeclared[name]
+			x := fn.predeclared[name]
 			if x == nil {
 				err = fmt.Errorf("internal error: predeclared variable %s is uninitialized", name)
 				break loop

value.go

@@ -559,7 +559,12 @@ func (b *Builtin) Receiver() Value { return b.recv }
 func (b *Builtin) String() string  { return toString(b) }
 func (b *Builtin) Type() string    { return "builtin_function_or_method" }
 func (b *Builtin) Call(thread *Thread, args Tuple, kwargs []Tuple) (Value, error) {
-	return b.fn(thread, b, args, kwargs)
+	// TODO(adonovan): opt: we can avoid allocating a Frame unnecessarily if we
+	// do it only when a built-in calls back into Skylark, or when Call fails.
+	thread.frame = &Frame{parent: thread.frame, callable: b}
+	result, err := b.fn(thread, b, args, kwargs)
+	thread.frame = thread.frame.parent
+	return result, err
 }
 func (b *Builtin) Truth() Bool { return true }