code review 146650043: fmt: make the %#v verb a special flag

fmt: make the %#v verb a special flag
The %#v verb is special: it says all values below need to print as %#v.
However, for some situations the # flag has other meanings and this
causes some issues, particularly in how Formatters work. Since %#v
dominates all formatting, translate it into actual state of the formatter
and decouple it from the # flag itself within the calculations (although
it must be restored when methods are doing the work.)
The result is cleaner code and correct handling of # for Formatters.
TODO: Apply the same thinking to the + flag in a followup CL.

Also, the wasString return value in handleMethods is always false,
so eliminate it.

Update issue 8835

[r, 2014-10-01 17:50:48]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://code.google.com/p/go

[iant, 2014-10-02 19:53:32]

LGTM

Various minor suggestions.

https://codereview.appspot.com/146650043/diff/20001/src/fmt/format.go
File src/fmt/format.go (right):

https://codereview.appspot.com/146650043/diff/20001/src/fmt/format.go#newcode52
src/fmt/format.go:52: // For the format %#v, we set these flags and
Which flags does this refer to?  Do you mean s/these flags/this flag/ ?

https://codereview.appspot.com/146650043/diff/20001/src/fmt/format.go#newcode53
src/fmt/format.go:53: // clear the plus and sharp flags, since they are in
effect
Maybe
s/the plus //
s/flags/flag/
s/they are/it is/

https://codereview.appspot.com/146650043/diff/20001/src/fmt/print.go
File src/fmt/print.go (right):

https://codereview.appspot.com/146650043/diff/20001/src/fmt/print.go#newcode679
src/fmt/print.go:679: return
Instead of setting handled = true above, just return true here.  Same change
several times below.

https://codereview.appspot.com/146650043/diff/20001/src/fmt/print.go#newcode704
src/fmt/print.go:704: // setting handled and deferring catchPanic,
Remove trailing comma.

https://codereview.appspot.com/146650043/diff/20001/src/fmt/print.go#newcode721
src/fmt/print.go:721: handled = false
You can remove this line now.

[r, 2014-10-02 21:17:02]

*** Submitted as https://code.google.com/p/go/source/detail?r=1ebe0bc97711 ***

fmt: make the %#v verb a special flag
The %#v verb is special: it says all values below need to print as %#v.
However, for some situations the # flag has other meanings and this
causes some issues, particularly in how Formatters work. Since %#v
dominates all formatting, translate it into actual state of the formatter
and decouple it from the # flag itself within the calculations (although
it must be restored when methods are doing the work.)
The result is cleaner code and correct handling of # for Formatters.
TODO: Apply the same thinking to the + flag in a followup CL.

Also, the wasString return value in handleMethods is always false,
so eliminate it.

Update issue 8835

LGTM=iant
R=golang-codereviews, iant
CC=golang-codereviews
https://codereview.appspot.com/146650043

https://codereview.appspot.com/146650043/diff/20001/src/fmt/format.go
File src/fmt/format.go (right):

https://codereview.appspot.com/146650043/diff/20001/src/fmt/format.go#newcode52
src/fmt/format.go:52: // For the format %#v, we set these flags and
On 2014/10/02 19:53:31, iant wrote:
> Which flags does this refer to?  Do you mean s/these flags/this flag/ ?

Done. It will go back to these when plusV is here too.

https://codereview.appspot.com/146650043/diff/20001/src/fmt/format.go#newcode53
src/fmt/format.go:53: // clear the plus and sharp flags, since they are in
effect
ditto

https://codereview.appspot.com/146650043/diff/20001/src/fmt/print.go
File src/fmt/print.go (right):

https://codereview.appspot.com/146650043/diff/20001/src/fmt/print.go#newcode679
src/fmt/print.go:679: return
On 2014/10/02 19:53:31, iant wrote:
> Instead of setting handled = true above, just return true here.  Same change
> several times below.

But then handled will be false if we panic, and we'll (try to) print it twice.

https://codereview.appspot.com/146650043/diff/20001/src/fmt/print.go#newcode704
src/fmt/print.go:704: // setting handled and deferring catchPanic,
On 2014/10/02 19:53:31, iant wrote:
> Remove trailing comma.

Done.

https://codereview.appspot.com/146650043/diff/20001/src/fmt/print.go#newcode721
src/fmt/print.go:721: handled = false
On 2014/10/02 19:53:31, iant wrote:
> You can remove this line now.

Done.

[gobot, 2014-10-03 05:38:04]

This changed caused perf changes on windows-amd64-perf:


json-1                    old          new      delta
cputime             170937500    176875000      +3.47
time                170854052    176779402      +3.47

json-2                    old          new      delta
cputime             173281250    178125000      +2.80
time                 86998447     89851020      +3.28

json-4                    old          new      delta
cputime             175468750    180625000      +2.94
time                 44395029     45766268      +3.09

json-8                    old          new      delta
time                 23865519     24434031      +2.38

http://build.golang.org/perfdetail?commit=1ebe0bc977119956e197cc5c2a41c104bea...



—gobot

[Chris Hines, 2014-10-03 19:21:49]

LGTM

just one observation

https://codereview.appspot.com/146650043/diff/40001/src/fmt/fmt_test.go
File src/fmt/fmt_test.go (right):

https://codereview.appspot.com/146650043/diff/40001/src/fmt/fmt_test.go#newco...
src/fmt/fmt_test.go:1214: func (fp) Format(f State, c rune) {
Type fp and its Format method are an exact duplicate of type flagPrinter (line
920 this file).

[r, 2014-10-03 20:16:30]

Thanks, well spotted. Fixed in the next CL.

On Fri, Oct 3, 2014 at 12:21 PM,  <chris.cs.guy@gmail.com> wrote:
> LGTM
>
> just one observation
>
>
> https://codereview.appspot.com/146650043/diff/40001/src/fmt/fmt_test.go
> File src/fmt/fmt_test.go (right):
>
>
https://codereview.appspot.com/146650043/diff/40001/src/fmt/fmt_test.go#newco...
> src/fmt/fmt_test.go:1214: func (fp) Format(f State, c rune) {
> Type fp and its Format method are an exact duplicate of type flagPrinter
> (line 920 this file).
>
> https://codereview.appspot.com/146650043/