diff options
author | Rob Pike <r@golang.org> | 2014-10-03 13:23:35 -0700 |
---|---|---|
committer | Rob Pike <r@golang.org> | 2014-10-03 13:23:35 -0700 |
commit | 0a635319fc000e052bd3d6e2823b170d046e06f5 (patch) | |
tree | 9f0c82ad35a879fb1527d00d9a1568e85990d9bf | |
parent | a81469cf0b101e552d1968469f528456d9d7835a (diff) | |
download | go-0a635319fc000e052bd3d6e2823b170d046e06f5.tar.gz |
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 issue 8835.
LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://codereview.appspot.com/154820043
-rw-r--r-- | src/fmt/fmt_test.go | 122 | ||||
-rw-r--r-- | src/fmt/format.go | 48 | ||||
-rw-r--r-- | src/fmt/print.go | 111 |
3 files changed, 127 insertions, 154 deletions
diff --git a/src/fmt/fmt_test.go b/src/fmt/fmt_test.go index f3b527d1f..4c3ba8fad 100644 --- 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 355b73262..4d97d1443 100644 --- 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 f141d39da..0c66c5781 100644 --- 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') |