cue: delay evaluation of args in bottom

Change-Id: I72ec254756e075f37e95b8db5529b9870a7594e6
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/2212
Reviewed-by: Marcel van Lohuizen <mpvl@google.com>
diff --git a/cue/ast.go b/cue/ast.go
index 15e6212..c0e93f8 100644
--- a/cue/ast.go
+++ b/cue/ast.go
@@ -126,12 +126,9 @@
 
 func (v *astVisitor) errf(n ast.Node, format string, args ...interface{}) evaluated {
 	v.astState.errors.Add(&nodeError{
-		path: v.appendPath(nil),
-		n:    n,
-		Message: errors.Message{
-			Format: format,
-			Args:   args,
-		},
+		path:    v.appendPath(nil),
+		n:       n,
+		Message: errors.NewMessage(format, args),
 	})
 	arguments := append([]interface{}{format}, args...)
 	return v.mkErr(newNode(n), arguments...)
@@ -460,7 +457,7 @@
 
 	case *ast.BottomLit:
 		// TODO: record inline comment.
-		value = &bottom{baseValue: newExpr(n), msg: "from source"}
+		value = &bottom{baseValue: newExpr(n), format: "from source"}
 
 	case *ast.BadDecl:
 		// nothing to do
diff --git a/cue/debug.go b/cue/debug.go
index 252bdc8..9fa4bf4 100644
--- a/cue/debug.go
+++ b/cue/debug.go
@@ -403,12 +403,12 @@
 
 	case *bottom:
 		write("_|_")
-		if x.value != nil || x.msg != "" {
+		if x.value != nil || x.format != "" {
 			write("(")
 			if x.value != nil {
 				writef("%s:", debugStr(p.ctx, x.value))
 			}
-			write(x.msg)
+			write(x.msg())
 			write(")")
 		}
 	case *top:
diff --git a/cue/errors.go b/cue/errors.go
index 0d7d84f..0d415e8 100644
--- a/cue/errors.go
+++ b/cue/errors.go
@@ -16,6 +16,7 @@
 
 import (
 	"fmt"
+	"reflect"
 
 	"cuelang.org/go/cue/ast"
 	"cuelang.org/go/cue/errors"
@@ -49,8 +50,8 @@
 
 func (v Value) toErr(b *bottom) errors.Error {
 	return &valueError{
-		v:       v,
-		err:     b,
+		v:   v,
+		err: b,
 	}
 }
 
@@ -74,6 +75,10 @@
 	return e.err.Positions()
 }
 
+func (e *valueError) Msg() (string, []interface{}) {
+	return e.err.Msg()
+}
+
 func (e *valueError) Path() (a []string) {
 	a, _ = e.v.path.appendPath(a, e.v.idx)
 	return a
@@ -97,7 +102,7 @@
 	return false
 }
 
-var errNotExists = &bottom{code: codeNotExist, msg: "undefined value"}
+var errNotExists = &bottom{code: codeNotExist, format: "undefined value"}
 
 func exists(v value) bool {
 	if err, ok := v.(*bottom); ok {
@@ -110,14 +115,15 @@
 type bottom struct {
 	baseValue
 
-	index          *index
-	code           errCode
-	exprDepth      int
-	value          value
-	offendingValue value
-	pos            source
-	msg            string
+	index     *index
+	code      errCode
+	exprDepth int
+	pos       source
+	format    string
+	args      []interface{}
 
+	// Debugging info
+	value   value
 	wrapped *bottom
 
 	// TODO: file at which the error was generated in the code.
@@ -127,7 +133,7 @@
 func (x *bottom) kind() kind { return bottomKind }
 
 func (x *bottom) Positions() []token.Pos {
-	if x.index != nil {
+	if x.index != nil { // TODO: remove check?
 		return appendPositions(nil, x.pos)
 	}
 	return nil
@@ -152,11 +158,22 @@
 	return pos
 }
 
+func (x *bottom) Msg() (format string, args []interface{}) {
+	ctx := x.index.newContext()
+	// We need to copy to avoid races.
+	args = make([]interface{}, len(x.args))
+	copy(args, x.args)
+	preEvalArgs(ctx, args)
+	return x.format, x.args
+}
+
+func (x *bottom) msg() string {
+	return fmt.Sprint(x)
+}
+
 func (x *bottom) Format(s fmt.State, verb rune) {
-	fmt.Fprint(s, x.msg)
-	if x.wrapped != nil {
-		fmt.Fprint(s, ": ", x.wrapped)
-	}
+	msg, args := x.Msg()
+	fmt.Fprintf(s, msg, args...)
 }
 
 func cycleError(v evaluated) *bottom {
@@ -186,13 +203,14 @@
 			e.code = x
 		case *bottom:
 			e.wrapped = x
-			e.offendingValue = x
 		case value:
-			e.offendingValue = x
-		case op:
-			panic("no longer using offending value and op")
 		case string:
-			e.msg += fmt.Sprintf(x, args[i+1:]...)
+			e.format = x
+			e.args = args[i+1:]
+			// Do not expand message so that errors can be localized.
+			for i, a := range e.args {
+				e.args[i] = fixArg(idx, a)
+			}
 			return e
 		}
 	}
@@ -202,6 +220,35 @@
 	return e
 }
 
+func fixArg(idx *index, x interface{}) interface{} {
+	switch x.(type) {
+	case uint, int, string:
+		return x
+	case value:
+		return x
+	}
+	t := reflect.TypeOf(x)
+	// Store all non-ptr types as is, as they cannot change.
+	if k := t.Kind(); k == reflect.String || k <= reflect.Complex128 {
+		return x
+	}
+	return fmt.Sprint(x)
+}
+
+// preEvalArgs is used to expand value arguments just before printing.
+func preEvalArgs(ctx *context, args []interface{}) {
+	for i, a := range args {
+		switch v := a.(type) {
+		case *bottom:
+			args[i] = v.msg()
+		case value:
+			// TODO: convert to Go values so that localization frameworks
+			// can format values accordingly.
+			args[i] = debugStr(ctx, v)
+		}
+	}
+}
+
 func isBottom(n value) bool {
 	return n.kind() == bottomKind
 }
diff --git a/cue/errors/errors.go b/cue/errors/errors.go
index 32b3770..e60e8c4 100644
--- a/cue/errors/errors.go
+++ b/cue/errors/errors.go
@@ -46,7 +46,7 @@
 
 // NewMessage creates an error message for human consumption. The arguments
 // are for later consumption, allowing the message to be localized at a later
-// time.
+// time. The passed argument list should not be modified.
 func NewMessage(format string, args []interface{}) Message {
 	return Message{format: format, args: args}
 }
@@ -71,6 +71,10 @@
 	// Path returns the path into the data tree where the error occurred.
 	// This path may be nil if the error is not associated with such a location.
 	Path() []string
+
+	// Msg returns the unformatted error message and its arguments for human
+	// consumption.
+	Msg() (format string, args []interface{})
 }
 
 // Path returns the path of an Error if err is of that type.
diff --git a/cue/export.go b/cue/export.go
index 45ed181..dbb4d41 100644
--- a/cue/export.go
+++ b/cue/export.go
@@ -567,8 +567,8 @@
 
 	case *bottom:
 		err := &ast.BottomLit{}
-		if x.msg != "" {
-			comment := &ast.Comment{Text: "/* " + x.msg + " */"}
+		if x.format != "" {
+			comment := &ast.Comment{Text: "/* " + x.msg() + " */"}
 			err.AddComment(&ast.CommentGroup{
 				Line:     true,
 				Position: 1,
diff --git a/cue/types.go b/cue/types.go
index 759dbe5..8ab4d11 100644
--- a/cue/types.go
+++ b/cue/types.go
@@ -166,8 +166,9 @@
 	return fmt.Sprintf("cue: marshal error at path %s: %v", p, e.err)
 }
 
-func (e *marshalError) Path() []string      { return e.err.Path() }
-func (e *marshalError) Position() token.Pos { return e.err.Position() }
+func (e *marshalError) Path() []string               { return e.err.Path() }
+func (e *marshalError) Position() token.Pos          { return e.err.Position() }
+func (e *marshalError) Msg() (string, []interface{}) { return e.err.Msg() }
 
 func unwrapJSONError(err error) errors.Error {
 	switch x := err.(type) {
diff --git a/cue/value.go b/cue/value.go
index dbcf507..9447251 100644
--- a/cue/value.go
+++ b/cue/value.go
@@ -680,7 +680,7 @@
 			index:     ctx.index,
 			code:      codeCycle,
 			value:     x.arcs[i].v,
-			msg:       "cycle detected",
+			format:    "cycle detected",
 		})
 		x.arcs[i].cache = &(ctx.evalStack[len(ctx.evalStack)-1])