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

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')