fmt: part 2 of the great flag rebuild: make %+v work in formatters

Apply a similar transformation to %+v that we did to %#v, making it
a top-level setting separate from the + flag itself. This fixes the
appearance of flags in Formatters and cleans up the code too,
probably making it a little faster.

Fixes #8835.

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://golang.org/cl/154820043

diff --git a/src/fmt/fmt_test.go b/src/fmt/fmt_test.go
index f3b527d1ff09314a52116d01759444f2d848dccf..4c3ba8fad11855b0f54702b1d19daadc5a479882 100644 (file)
--- a/src/fmt/fmt_test.go
+++ b/src/fmt/fmt_test.go
@@ -919,7 +919,7 @@ func TestCountMallocs(t *testing.T) {
 
 type flagPrinter struct{}
 
-func (*flagPrinter) Format(f State, c rune) {
+func (flagPrinter) Format(f State, c rune) {
        s := "%"
        for i := 0; i < 128; i++ {
                if f.Flag(i) {
@@ -1208,86 +1208,66 @@ func TestNilDoesNotBecomeTyped(t *testing.T) {
        }
 }
 
-// Formatters did not get delivered flags correctly in all cases. Issue 8835.
-type fp struct{}
-
-func (fp) Format(f State, c rune) {
-       s := "%"
-       for i := 0; i < 128; i++ {
-               if f.Flag(i) {
-                       s += string(i)
-               }
-       }
-       if w, ok := f.Width(); ok {
-               s += Sprintf("%d", w)
-       }
-       if p, ok := f.Precision(); ok {
-               s += Sprintf(".%d", p)
-       }
-       s += string(c)
-       io.WriteString(f, "["+s+"]")
-}
-
 var formatterFlagTests = []struct {
        in  string
        val interface{}
        out string
 }{
        // scalar values with the (unused by fmt) 'a' verb.
-       {"%a", fp{}, "[%a]"},
-       {"%-a", fp{}, "[%-a]"},
-       {"%+a", fp{}, "[%+a]"},
-       {"%#a", fp{}, "[%#a]"},
-       {"% a", fp{}, "[% a]"},
-       {"%0a", fp{}, "[%0a]"},
-       {"%1.2a", fp{}, "[%1.2a]"},
-       {"%-1.2a", fp{}, "[%-1.2a]"},
-       {"%+1.2a", fp{}, "[%+1.2a]"},
-       {"%-+1.2a", fp{}, "[%+-1.2a]"},
-       {"%-+1.2abc", fp{}, "[%+-1.2a]bc"},
-       {"%-1.2abc", fp{}, "[%-1.2a]bc"},
+       {"%a", flagPrinter{}, "[%a]"},
+       {"%-a", flagPrinter{}, "[%-a]"},
+       {"%+a", flagPrinter{}, "[%+a]"},
+       {"%#a", flagPrinter{}, "[%#a]"},
+       {"% a", flagPrinter{}, "[% a]"},
+       {"%0a", flagPrinter{}, "[%0a]"},
+       {"%1.2a", flagPrinter{}, "[%1.2a]"},
+       {"%-1.2a", flagPrinter{}, "[%-1.2a]"},
+       {"%+1.2a", flagPrinter{}, "[%+1.2a]"},
+       {"%-+1.2a", flagPrinter{}, "[%+-1.2a]"},
+       {"%-+1.2abc", flagPrinter{}, "[%+-1.2a]bc"},
+       {"%-1.2abc", flagPrinter{}, "[%-1.2a]bc"},
 
        // composite values with the 'a' verb
-       {"%a", [1]fp{}, "[[%a]]"},
-       {"%-a", [1]fp{}, "[[%-a]]"},
-       {"%+a", [1]fp{}, "[[%+a]]"},
-       {"%#a", [1]fp{}, "[[%#a]]"},
-       {"% a", [1]fp{}, "[[% a]]"},
-       {"%0a", [1]fp{}, "[[%0a]]"},
-       {"%1.2a", [1]fp{}, "[[%1.2a]]"},
-       {"%-1.2a", [1]fp{}, "[[%-1.2a]]"},
-       {"%+1.2a", [1]fp{}, "[[%+1.2a]]"},
-       {"%-+1.2a", [1]fp{}, "[[%+-1.2a]]"},
-       {"%-+1.2abc", [1]fp{}, "[[%+-1.2a]]bc"},
-       {"%-1.2abc", [1]fp{}, "[[%-1.2a]]bc"},
+       {"%a", [1]flagPrinter{}, "[[%a]]"},
+       {"%-a", [1]flagPrinter{}, "[[%-a]]"},
+       {"%+a", [1]flagPrinter{}, "[[%+a]]"},
+       {"%#a", [1]flagPrinter{}, "[[%#a]]"},
+       {"% a", [1]flagPrinter{}, "[[% a]]"},
+       {"%0a", [1]flagPrinter{}, "[[%0a]]"},
+       {"%1.2a", [1]flagPrinter{}, "[[%1.2a]]"},
+       {"%-1.2a", [1]flagPrinter{}, "[[%-1.2a]]"},
+       {"%+1.2a", [1]flagPrinter{}, "[[%+1.2a]]"},
+       {"%-+1.2a", [1]flagPrinter{}, "[[%+-1.2a]]"},
+       {"%-+1.2abc", [1]flagPrinter{}, "[[%+-1.2a]]bc"},
+       {"%-1.2abc", [1]flagPrinter{}, "[[%-1.2a]]bc"},
 
        // simple values with the 'v' verb
-       {"%v", fp{}, "[%v]"},
-       {"%-v", fp{}, "[%-v]"},
-       {"%+v", fp{}, "[%+v]"},
-       {"%#v", fp{}, "[%#v]"},
-       {"% v", fp{}, "[% v]"},
-       {"%0v", fp{}, "[%0v]"},
-       {"%1.2v", fp{}, "[%1.2v]"},
-       {"%-1.2v", fp{}, "[%-1.2v]"},
-       {"%+1.2v", fp{}, "[%+1.2v]"},
-       {"%-+1.2v", fp{}, "[%+-1.2v]"},
-       {"%-+1.2vbc", fp{}, "[%+-1.2v]bc"},
-       {"%-1.2vbc", fp{}, "[%-1.2v]bc"},
-
-       // composite values with the 'v' verb. Some are still broken.
-       {"%v", [1]fp{}, "[[%v]]"},
-       {"%-v", [1]fp{}, "[[%-v]]"},
-       //{"%+v", [1]fp{}, "[[%+v]]"},
-       {"%#v", [1]fp{}, "[1]fmt_test.fp{[%#v]}"},
-       {"% v", [1]fp{}, "[[% v]]"},
-       {"%0v", [1]fp{}, "[[%0v]]"},
-       {"%1.2v", [1]fp{}, "[[%1.2v]]"},
-       {"%-1.2v", [1]fp{}, "[[%-1.2v]]"},
-       //{"%+1.2v", [1]fp{}, "[[%+1.2v]]"},
-       //{"%-+1.2v", [1]fp{}, "[[%+-1.2v]]"},
-       //{"%-+1.2vbc", [1]fp{}, "[[%+-1.2v]]bc"},
-       {"%-1.2vbc", [1]fp{}, "[[%-1.2v]]bc"},
+       {"%v", flagPrinter{}, "[%v]"},
+       {"%-v", flagPrinter{}, "[%-v]"},
+       {"%+v", flagPrinter{}, "[%+v]"},
+       {"%#v", flagPrinter{}, "[%#v]"},
+       {"% v", flagPrinter{}, "[% v]"},
+       {"%0v", flagPrinter{}, "[%0v]"},
+       {"%1.2v", flagPrinter{}, "[%1.2v]"},
+       {"%-1.2v", flagPrinter{}, "[%-1.2v]"},
+       {"%+1.2v", flagPrinter{}, "[%+1.2v]"},
+       {"%-+1.2v", flagPrinter{}, "[%+-1.2v]"},
+       {"%-+1.2vbc", flagPrinter{}, "[%+-1.2v]bc"},
+       {"%-1.2vbc", flagPrinter{}, "[%-1.2v]bc"},
+
+       // composite values with the 'v' verb.
+       {"%v", [1]flagPrinter{}, "[[%v]]"},
+       {"%-v", [1]flagPrinter{}, "[[%-v]]"},
+       {"%+v", [1]flagPrinter{}, "[[%+v]]"},
+       {"%#v", [1]flagPrinter{}, "[1]fmt_test.flagPrinter{[%#v]}"},
+       {"% v", [1]flagPrinter{}, "[[% v]]"},
+       {"%0v", [1]flagPrinter{}, "[[%0v]]"},
+       {"%1.2v", [1]flagPrinter{}, "[[%1.2v]]"},
+       {"%-1.2v", [1]flagPrinter{}, "[[%-1.2v]]"},
+       {"%+1.2v", [1]flagPrinter{}, "[[%+1.2v]]"},
+       {"%-+1.2v", [1]flagPrinter{}, "[[%+-1.2v]]"},
+       {"%-+1.2vbc", [1]flagPrinter{}, "[[%+-1.2v]]bc"},
+       {"%-1.2vbc", [1]flagPrinter{}, "[[%-1.2v]]bc"},
 }
 
 func TestFormatterFlags(t *testing.T) {

diff --git a/src/fmt/format.go b/src/fmt/format.go
index 355b732622eee623da17e01aebbb36634f95e086..4d97d1443eda1292c403fd086acf7fc627b1c88a 100644 (file)
--- a/src/fmt/format.go
+++ b/src/fmt/format.go
@@ -34,6 +34,25 @@ func init() {
        }
 }
 
+// flags placed in a separate struct for easy clearing.
+type fmtFlags struct {
+       widPresent  bool
+       precPresent bool
+       minus       bool
+       plus        bool
+       sharp       bool
+       space       bool
+       unicode     bool
+       uniQuote    bool // Use 'x'= prefix for %U if printable.
+       zero        bool
+
+       // For the formats %+v %#v, we set the plusV/sharpV flags
+       // and clear the plus/sharp flags since %+v and %#v are in effect
+       // different, flagless formats set at the top level.
+       plusV  bool
+       sharpV bool
+}
+
 // A fmt is the raw formatter used by Printf etc.
 // It prints into a buffer that must be set up separately.
 type fmt struct {
@@ -42,36 +61,11 @@ type fmt struct {
        // width, precision
        wid  int
        prec int
-       // flags
-       widPresent  bool
-       precPresent bool
-       minus       bool
-       plus        bool
-       sharp       bool
-       space       bool
-       // For the format %#v, we set this flag and
-       // clear the plus flag, since it is in effect
-       // a different, flagless format set at the top level.
-       // TODO: plusV could use this too.
-       sharpV   bool
-       unicode  bool
-       uniQuote bool // Use 'x'= prefix for %U if printable.
-       zero     bool
+       fmtFlags
 }
 
 func (f *fmt) clearflags() {
-       f.wid = 0
-       f.widPresent = false
-       f.prec = 0
-       f.precPresent = false
-       f.minus = false
-       f.plus = false
-       f.sharp = false
-       f.space = false
-       f.sharpV = false
-       f.unicode = false
-       f.uniQuote = false
-       f.zero = false
+       f.fmtFlags = fmtFlags{}
 }
 
 func (f *fmt) init(buf *buffer) {

diff --git a/src/fmt/print.go b/src/fmt/print.go
index f141d39daf0311b5e80d584998d1fcf0f9aade99..0c66c57817b8910d236d749ae498ee30da1876a8 100644 (file)
--- a/src/fmt/print.go
+++ b/src/fmt/print.go
@@ -128,7 +128,7 @@ var ppFree = sync.Pool{
        New: func() interface{} { return new(pp) },
 }
 
-// newPrinter allocates a new pp struct or grab a cached one.
+// newPrinter allocates a new pp struct or grabs a cached one.
 func newPrinter() *pp {
        p := ppFree.Get().(*pp)
        p.panicking = false
@@ -317,11 +317,11 @@ func (p *pp) badVerb(verb rune) {
        case p.arg != nil:
                p.buf.WriteString(reflect.TypeOf(p.arg).String())
                p.add('=')
-               p.printArg(p.arg, 'v', false, 0)
+               p.printArg(p.arg, 'v', 0)
        case p.value.IsValid():
                p.buf.WriteString(p.value.Type().String())
                p.add('=')
-               p.printValue(p.value, 'v', false, 0)
+               p.printValue(p.value, 'v', 0)
        default:
                p.buf.Write(nilAngleBytes)
        }
@@ -549,7 +549,7 @@ func (p *pp) fmtBytes(v []byte, verb rune, typ reflect.Type, depth int) {
                                        p.buf.WriteByte(' ')
                                }
                        }
-                       p.printArg(c, 'v', p.fmt.plus, depth+1)
+                       p.printArg(c, 'v', depth+1)
                }
                if p.fmt.sharpV {
                        p.buf.WriteByte('}')
@@ -641,32 +641,41 @@ func (p *pp) catchPanic(arg interface{}, verb rune) {
                p.add(verb)
                p.buf.Write(panicBytes)
                p.panicking = true
-               p.printArg(err, 'v', false, 0)
+               p.printArg(err, 'v', 0)
                p.panicking = false
                p.buf.WriteByte(')')
        }
 }
 
 // clearSpecialFlags pushes %#v back into the regular flags and returns their old state.
-func (p *pp) clearSpecialFlags() bool {
-       ret := p.fmt.sharpV
-       if ret {
+func (p *pp) clearSpecialFlags() (plusV, sharpV bool) {
+       plusV = p.fmt.plusV
+       if plusV {
+               p.fmt.plus = true
+               p.fmt.plusV = false
+       }
+       sharpV = p.fmt.sharpV
+       if sharpV {
                p.fmt.sharp = true
                p.fmt.sharpV = false
        }
-       return ret
+       return
 }
 
 // restoreSpecialFlags, whose argument should be a call to clearSpecialFlags,
-// restores the setting of the sharpV flag.
-func (p *pp) restoreSpecialFlags(sharpV bool) {
+// restores the setting of the plusV and sharpV flags.
+func (p *pp) restoreSpecialFlags(plusV, sharpV bool) {
+       if plusV {
+               p.fmt.plus = false
+               p.fmt.plusV = true
+       }
        if sharpV {
                p.fmt.sharp = false
                p.fmt.sharpV = true
        }
 }
 
-func (p *pp) handleMethods(verb rune, plus bool, depth int) (handled bool) {
+func (p *pp) handleMethods(verb rune, depth int) (handled bool) {
        if p.erroring {
                return
        }
@@ -678,19 +687,14 @@ func (p *pp) handleMethods(verb rune, plus bool, depth int) (handled bool) {
                formatter.Format(p, verb)
                return
        }
-       // Must not touch flags before Formatter looks at them.
-       if plus {
-               p.fmt.plus = false
-       }
 
        // If we're doing Go syntax and the argument knows how to supply it, take care of it now.
        if p.fmt.sharpV {
                if stringer, ok := p.arg.(GoStringer); ok {
                        handled = true
-                       defer p.restoreSpecialFlags(p.clearSpecialFlags())
                        defer p.catchPanic(p.arg, verb)
                        // Print the result of GoString unadorned.
-                       p.fmtString(stringer.GoString(), 's')
+                       p.fmt.fmt_s(stringer.GoString())
                        return
                }
        } else {
@@ -707,13 +711,13 @@ func (p *pp) handleMethods(verb rune, plus bool, depth int) (handled bool) {
                        case error:
                                handled = true
                                defer p.catchPanic(p.arg, verb)
-                               p.printArg(v.Error(), verb, plus, depth)
+                               p.printArg(v.Error(), verb, depth)
                                return
 
                        case Stringer:
                                handled = true
                                defer p.catchPanic(p.arg, verb)
-                               p.printArg(v.String(), verb, plus, depth)
+                               p.printArg(v.String(), verb, depth)
                                return
                        }
                }
@@ -721,7 +725,7 @@ func (p *pp) handleMethods(verb rune, plus bool, depth int) (handled bool) {
        return false
 }
 
-func (p *pp) printArg(arg interface{}, verb rune, plus bool, depth int) (wasString bool) {
+func (p *pp) printArg(arg interface{}, verb rune, depth int) (wasString bool) {
        p.arg = arg
        p.value = reflect.Value{}
 
@@ -738,22 +742,13 @@ func (p *pp) printArg(arg interface{}, verb rune, plus bool, depth int) (wasStri
        // %T (the value's type) and %p (its address) are special; we always do them first.
        switch verb {
        case 'T':
-               p.printArg(reflect.TypeOf(arg).String(), 's', false, 0)
+               p.printArg(reflect.TypeOf(arg).String(), 's', 0)
                return false
        case 'p':
                p.fmtPointer(reflect.ValueOf(arg), verb)
                return false
        }
 
-       // Clear flags for base formatters.
-       // handleMethods needs them, so we must restore them later.
-       // We could call handleMethods here and avoid this work, but
-       // handleMethods is expensive enough to be worth delaying.
-       oldPlus := p.fmt.plus
-       if plus {
-               p.fmt.plus = false
-       }
-
        // Some types can be done without reflection.
        switch f := arg.(type) {
        case bool:
@@ -795,21 +790,19 @@ func (p *pp) printArg(arg interface{}, verb rune, plus bool, depth int) (wasStri
                p.fmtBytes(f, verb, nil, depth)
                wasString = verb == 's'
        default:
-               // Restore flags in case handleMethods finds a Formatter.
-               p.fmt.plus = oldPlus
                // If the type is not simple, it might have methods.
-               if handled := p.handleMethods(verb, plus, depth); handled {
+               if handled := p.handleMethods(verb, depth); handled {
                        return false
                }
                // Need to use reflection
-               return p.printReflectValue(reflect.ValueOf(arg), verb, plus, depth)
+               return p.printReflectValue(reflect.ValueOf(arg), verb, depth)
        }
        p.arg = nil
        return
 }
 
 // printValue is like printArg but starts with a reflect value, not an interface{} value.
-func (p *pp) printValue(value reflect.Value, verb rune, plus bool, depth int) (wasString bool) {
+func (p *pp) printValue(value reflect.Value, verb rune, depth int) (wasString bool) {
        if !value.IsValid() {
                if verb == 'T' || verb == 'v' {
                        p.buf.Write(nilAngleBytes)
@@ -823,7 +816,7 @@ func (p *pp) printValue(value reflect.Value, verb rune, plus bool, depth int) (w
        // %T (the value's type) and %p (its address) are special; we always do them first.
        switch verb {
        case 'T':
-               p.printArg(value.Type().String(), 's', false, 0)
+               p.printArg(value.Type().String(), 's', 0)
                return false
        case 'p':
                p.fmtPointer(value, verb)
@@ -836,18 +829,18 @@ func (p *pp) printValue(value reflect.Value, verb rune, plus bool, depth int) (w
        if value.CanInterface() {
                p.arg = value.Interface()
        }
-       if handled := p.handleMethods(verb, plus, depth); handled {
+       if handled := p.handleMethods(verb, depth); handled {
                return false
        }
 
-       return p.printReflectValue(value, verb, plus, depth)
+       return p.printReflectValue(value, verb, depth)
 }
 
 var byteType = reflect.TypeOf(byte(0))
 
 // printReflectValue is the fallback for both printArg and printValue.
 // It uses reflect to print the value.
-func (p *pp) printReflectValue(value reflect.Value, verb rune, plus bool, depth int) (wasString bool) {
+func (p *pp) printReflectValue(value reflect.Value, verb rune, depth int) (wasString bool) {
        oldValue := p.value
        p.value = value
 BigSwitch:
@@ -892,9 +885,9 @@ BigSwitch:
                                        p.buf.WriteByte(' ')
                                }
                        }
-                       p.printValue(key, verb, plus, depth+1)
+                       p.printValue(key, verb, depth+1)
                        p.buf.WriteByte(':')
-                       p.printValue(f.MapIndex(key), verb, plus, depth+1)
+                       p.printValue(f.MapIndex(key), verb, depth+1)
                }
                if p.fmt.sharpV {
                        p.buf.WriteByte('}')
@@ -916,13 +909,13 @@ BigSwitch:
                                        p.buf.WriteByte(' ')
                                }
                        }
-                       if plus || p.fmt.sharpV {
+                       if p.fmt.plusV || p.fmt.sharpV {
                                if f := t.Field(i); f.Name != "" {
                                        p.buf.WriteString(f.Name)
                                        p.buf.WriteByte(':')
                                }
                        }
-                       p.printValue(getField(v, i), verb, plus, depth+1)
+                       p.printValue(getField(v, i), verb, depth+1)
                }
                p.buf.WriteByte('}')
        case reflect.Interface:
@@ -935,7 +928,7 @@ BigSwitch:
                                p.buf.Write(nilAngleBytes)
                        }
                } else {
-                       wasString = p.printValue(value, verb, plus, depth+1)
+                       wasString = p.printValue(value, verb, depth+1)
                }
        case reflect.Array, reflect.Slice:
                // Byte slices are special:
@@ -980,7 +973,7 @@ BigSwitch:
                                        p.buf.WriteByte(' ')
                                }
                        }
-                       p.printValue(f.Index(i), verb, plus, depth+1)
+                       p.printValue(f.Index(i), verb, depth+1)
                }
                if p.fmt.sharpV {
                        p.buf.WriteByte('}')
@@ -995,11 +988,11 @@ BigSwitch:
                        switch a := f.Elem(); a.Kind() {
                        case reflect.Array, reflect.Slice:
                                p.buf.WriteByte('&')
-                               p.printValue(a, verb, plus, depth+1)
+                               p.printValue(a, verb, depth+1)
                                break BigSwitch
                        case reflect.Struct:
                                p.buf.WriteByte('&')
-                               p.printValue(a, verb, plus, depth+1)
+                               p.printValue(a, verb, depth+1)
                                break BigSwitch
                        }
                }
@@ -1171,13 +1164,19 @@ func (p *pp) doPrintf(format string, a []interface{}) {
                arg := a[argNum]
                argNum++
 
-               if c == 'v' && p.fmt.sharp {
-                       // Go syntax. Set the flag in the fmt and clear the sharp flag.
-                       p.fmt.sharp = false
-                       p.fmt.sharpV = true
+               if c == 'v' {
+                       if p.fmt.sharp {
+                               // Go syntax. Set the flag in the fmt and clear the sharp flag.
+                               p.fmt.sharp = false
+                               p.fmt.sharpV = true
+                       }
+                       if p.fmt.plus {
+                               // Struct-field syntax. Set the flag in the fmt and clear the plus flag.
+                               p.fmt.plus = false
+                               p.fmt.plusV = true
+                       }
                }
-               plus := c == 'v' && p.fmt.plus
-               p.printArg(arg, c, plus, 0)
+               p.printArg(arg, c, 0)
        }
 
        // Check for extra arguments unless the call accessed the arguments
@@ -1191,7 +1190,7 @@ func (p *pp) doPrintf(format string, a []interface{}) {
                                p.buf.WriteString(reflect.TypeOf(arg).String())
                                p.buf.WriteByte('=')
                        }
-                       p.printArg(arg, 'v', false, 0)
+                       p.printArg(arg, 'v', 0)
                        if argNum+1 < len(a) {
                                p.buf.Write(commaSpaceBytes)
                        }
@@ -1212,7 +1211,7 @@ func (p *pp) doPrint(a []interface{}, addspace, addnewline bool) {
                                p.buf.WriteByte(' ')
                        }
                }
-               prevString = p.printArg(arg, 'v', false, 0)
+               prevString = p.printArg(arg, 'v', 0)
        }
        if addnewline {
                p.buf.WriteByte('\n')