fmt: catch panics from calls to String etc.

This change causes Print et al. to catch panics generated by
calls to String, GoString, and Format.  The panic is formatted
into the output stream as an error, but the program continues.
As a special case, if the argument was a nil pointer, the
result is just "<nil>", because that's almost certainly enough
information and handles the very common case of String
methods that don't guard against nil.

Scan does not want this change. Input must work; output can
be for debugging and it's nice to get output even when you
make a mistake.

R=dsymonds, r, adg, gri, rsc, gri
CC=golang-dev
https://golang.org/cl/4640043

diff --git a/src/pkg/fmt/fmt_test.go b/src/pkg/fmt/fmt_test.go
index 3d255c3d1f29552597d7158adc600c4df4dc4eb0..9a802452842e76e9b3776ab9e9e95699a48e90d7 100644 (file)
--- a/src/pkg/fmt/fmt_test.go
+++ b/src/pkg/fmt/fmt_test.go
@@ -683,3 +683,56 @@ func TestWidthAndPrecision(t *testing.T) {
                }
        }
 }
+
+// A type that panics in String.
+type Panic struct {
+       message interface{}
+}
+
+// Value receiver.
+func (p Panic) GoString() string {
+       panic(p.message)
+}
+
+// Value receiver.
+func (p Panic) String() string {
+       panic(p.message)
+}
+
+// A type that panics in Format.
+type PanicF struct {
+       message interface{}
+}
+
+// Value receiver.
+func (p PanicF) Format(f State, c int) {
+       panic(p.message)
+}
+
+var panictests = []struct {
+       fmt string
+       in  interface{}
+       out string
+}{
+       // String
+       {"%d", (*Panic)(nil), "<nil>"}, // nil pointer special case
+       {"%d", Panic{io.ErrUnexpectedEOF}, "%d(PANIC=unexpected EOF)"},
+       {"%d", Panic{3}, "%d(PANIC=3)"},
+       // GoString
+       {"%#v", (*Panic)(nil), "<nil>"}, // nil pointer special case
+       {"%#v", Panic{io.ErrUnexpectedEOF}, "%v(PANIC=unexpected EOF)"},
+       {"%#v", Panic{3}, "%v(PANIC=3)"},
+       // Format
+       {"%s", (*PanicF)(nil), "<nil>"}, // nil pointer special case
+       {"%s", PanicF{io.ErrUnexpectedEOF}, "%s(PANIC=unexpected EOF)"},
+       {"%s", PanicF{3}, "%s(PANIC=3)"},
+}
+
+func TestPanics(t *testing.T) {
+       for _, tt := range panictests {
+               s := Sprintf(tt.fmt, tt.in)
+               if s != tt.out {
+                       t.Errorf("%q: got %q expected %q", tt.fmt, s, tt.out)
+               }
+       }
+}

diff --git a/src/pkg/fmt/print.go b/src/pkg/fmt/print.go
index 2b2a7192703706004bcd75109ad38d1945302522..20a507a3a933a0fb3e5950d04ea38427912a38da 100644 (file)
--- a/src/pkg/fmt/print.go
+++ b/src/pkg/fmt/print.go
@@ -22,6 +22,7 @@ var (
        nilBytes        = []byte("nil")
        mapBytes        = []byte("map[")
        missingBytes    = []byte("(MISSING)")
+       panicBytes      = []byte("(PANIC=")
        extraBytes      = []byte("%!(EXTRA ")
        irparenBytes    = []byte("i)")
        bytesBytes      = []byte("[]byte{")
@@ -69,10 +70,11 @@ type GoStringer interface {
 }
 
 type pp struct {
-       n       int
-       buf     bytes.Buffer
-       runeBuf [utf8.UTFMax]byte
-       fmt     fmt
+       n         int
+       panicking bool
+       buf       bytes.Buffer
+       runeBuf   [utf8.UTFMax]byte
+       fmt       fmt
 }
 
 // A cache holds a set of reusable objects.
@@ -111,6 +113,7 @@ var ppFree = newCache(func() interface{} { return new(pp) })
 // Allocate a new pp struct or grab a cached one.
 func newPrinter() *pp {
        p := ppFree.get().(*pp)
+       p.panicking = false
        p.fmt.init(&p.buf)
        return p
 }
@@ -566,6 +569,31 @@ var (
        uintptrBits = reflect.TypeOf(uintptr(0)).Bits()
 )
 
+func (p *pp) catchPanic(val interface{}, verb int) {
+       if err := recover(); err != nil {
+               // If it's a nil pointer, just say "<nil>". The likeliest causes are a
+               // Stringer that fails to guard against nil or a nil pointer for a
+               // value receiver, and in either case, "<nil>" is a nice result.
+               if v := reflect.ValueOf(val); v.Kind() == reflect.Ptr && v.IsNil() {
+                       p.buf.Write(nilAngleBytes)
+                       return
+               }
+               // Otherwise print a concise panic message. Most of the time the panic
+               // value will print itself nicely.
+               if p.panicking {
+                       // Nested panics; the recursion in printField cannot succeed.
+                       panic(err)
+               }
+               p.buf.WriteByte('%')
+               p.add(verb)
+               p.buf.Write(panicBytes)
+               p.panicking = true
+               p.printField(err, 'v', false, false, 0)
+               p.panicking = false
+               p.buf.WriteByte(')')
+       }
+}
+
 func (p *pp) printField(field interface{}, verb int, plus, goSyntax bool, depth int) (wasString bool) {
        if field == nil {
                if verb == 'T' || verb == 'v' {
@@ -588,6 +616,7 @@ func (p *pp) printField(field interface{}, verb int, plus, goSyntax bool, depth
        }
        // Is it a Formatter?
        if formatter, ok := field.(Formatter); ok {
+               defer p.catchPanic(field, verb)
                formatter.Format(p, verb)
                return false // this value is not a string
 
@@ -600,6 +629,7 @@ func (p *pp) printField(field interface{}, verb int, plus, goSyntax bool, depth
        if goSyntax {
                p.fmt.sharp = false
                if stringer, ok := field.(GoStringer); ok {
+                       defer p.catchPanic(field, verb)
                        // Print the result of GoString unadorned.
                        p.fmtString(stringer.GoString(), 's', false, field)
                        return false // this value is not a string
@@ -607,6 +637,7 @@ func (p *pp) printField(field interface{}, verb int, plus, goSyntax bool, depth
        } else {
                // Is it a Stringer?
                if stringer, ok := field.(Stringer); ok {
+                       defer p.catchPanic(field, verb)
                        p.printField(stringer.String(), verb, plus, false, depth)
                        return false // this value is not a string
                }