Skip to content

G
go
Commit a2242456 authored by Rob Pike's avatar Rob Pike
Browse files
Options

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
parent 13da3608
Hide whitespace changes
Inline Side-by-side
Showing with 127 additions and 154 deletions
src/fmt/fmt_test.go
View file @ a2242456
......@@ -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) {
......
src/fmt/format.go
View file @ a2242456
......@@ -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) {
......
src/fmt/print.go
View file @ a2242456
......@@ -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')
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7