fmt: refactor pointer formatting and improve tests

Uses a switch statement for direct format function selection
similar to other types verb handling in fmt.

Applies padding also to nil pointers formatted with %v.

Guards against "slice bounds out of range" panic in TestSprintf
when a pointer test results in a formatted string s
that is shorter than the index i the pointer should appear in.

Adds more and rearranges tests.

Fixes #14712
Fixes #14714

Change-Id: Iaf5ae37b7e6ba7d27d528d199f2b2eb9d5829b8c
Reviewed-on: https://go-review.googlesource.com/20371
Run-TryBot: Rob Pike <r@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>

diff --git a/src/fmt/fmt_test.go b/src/fmt/fmt_test.go
index 2ff3515c528f4ee66e7390367c4e15d092badb06..5b197d8a398f62eea4b8b24cbe5683281bb1a624 100644 (file)
--- a/src/fmt/fmt_test.go
+++ b/src/fmt/fmt_test.go
@@ -48,19 +48,23 @@ func TestFmtInterface(t *testing.T) {
        }
 }
 
+const (
+       b32 uint32 = 1<<32 - 1
+       b64 uint64 = 1<<64 - 1
+)
+
 var (
        NaN    = math.NaN()
        posInf = math.Inf(1)
        negInf = math.Inf(-1)
-)
 
-const b32 uint32 = 1<<32 - 1
-const b64 uint64 = 1<<64 - 1
+       intVar = 0
 
-var array = [5]int{1, 2, 3, 4, 5}
-var iarray = [4]interface{}{1, "hello", 2.5, nil}
-var slice = array[:]
-var islice = iarray[:]
+       array  = [5]int{1, 2, 3, 4, 5}
+       iarray = [4]interface{}{1, "hello", 2.5, nil}
+       slice  = array[:]
+       islice = iarray[:]
+)
 
 type A struct {
        i int
@@ -130,8 +134,6 @@ func (byteFormatter) Format(f State, _ rune) {
 
 var byteFormatterSlice = []byteFormatter{97, 98, 99, 100}
 
-var b byte
-
 var fmtTests = []struct {
        fmt string
        val interface{}
@@ -598,7 +600,7 @@ var fmtTests = []struct {
 
        // go syntax
        {"%#v", A{1, 2, "a", []int{1, 2}}, `fmt_test.A{i:1, j:0x2, s:"a", x:[]int{1, 2}}`},
-       {"%#v", &b, "(*uint8)(0xPTR)"},
+       {"%#v", new(byte), "(*uint8)(0xPTR)"},
        {"%#v", TestFmtInterface, "(func(*testing.T))(0xPTR)"},
        {"%#v", make(chan int), "(chan int)(0xPTR)"},
        {"%#v", uint64(1<<64 - 1), "0xffffffffffffffff"},
@@ -729,31 +731,40 @@ var fmtTests = []struct {
        {"%10T", nil, "     <nil>"},
        {"%-10T", nil, "<nil>     "},
 
-       // %p
-       {"p0=%p", new(int), "p0=0xPTR"},
-       {"p1=%s", &pValue, "p1=String(p)"}, // String method...
-       {"p2=%p", &pValue, "p2=0xPTR"},     // ... not called with %p
-       {"p3=%p", (*int)(nil), "p3=0x0"},
-       {"p4=%#p", new(int), "p4=PTR"},
-
+       // %p with pointers
+       {"%p", (*int)(nil), "0x0"},
+       {"%#p", (*int)(nil), "0"},
+       {"%p", &intVar, "0xPTR"},
+       {"%#p", &intVar, "PTR"},
+       {"%p", &array, "0xPTR"},
+       {"%p", &slice, "0xPTR"},
+       {"%8.2p", (*int)(nil), "    0x00"},
+       {"%-20.16p", &intVar, "0xPTR  "},
        // %p on non-pointers
        {"%p", make(chan int), "0xPTR"},
        {"%p", make(map[int]int), "0xPTR"},
-       {"%p", make([]int, 1), "0xPTR"},
-       {"%p", 27, "%!p(int=27)"}, // not a pointer at all
-
-       // %q on pointers
-       {"%q", (*int)(nil), "%!q(*int=<nil>)"},
-       {"%q", new(int), "%!q(*int=0xPTR)"},
-
-       // %v on pointers formats 0 as <nil>
+       {"%p", func() {}, "0xPTR"},
+       {"%p", 27, "%!p(int=27)"},  // not a pointer at all
+       {"%p", nil, "%!p(<nil>)"},  // nil on its own has no type ...
+       {"%#p", nil, "%!p(<nil>)"}, // ... and hence is not a pointer type.
+       // pointers with specified base
+       {"%b", &intVar, "PTR_b"},
+       {"%d", &intVar, "PTR_d"},
+       {"%o", &intVar, "PTR_o"},
+       {"%x", &intVar, "PTR_x"},
+       {"%X", &intVar, "PTR_X"},
+       // %v on pointers
+       {"%v", nil, "<nil>"},
+       {"%#v", nil, "<nil>"},
        {"%v", (*int)(nil), "<nil>"},
-       {"%v", new(int), "0xPTR"},
-
-       // %d etc. pointers use specified base.
-       {"%d", new(int), "PTR_d"},
-       {"%o", new(int), "PTR_o"},
-       {"%x", new(int), "PTR_x"},
+       {"%#v", (*int)(nil), "(*int)(nil)"},
+       {"%v", &intVar, "0xPTR"},
+       {"%#v", &intVar, "(*int)(0xPTR)"},
+       {"%8.2v", (*int)(nil), "   <nil>"},
+       {"%-20.16v", &intVar, "0xPTR  "},
+       // string method on pointer
+       {"%s", &pValue, "String(p)"}, // String method...
+       {"%p", &pValue, "0xPTR"},     // ... is not called with %p.
 
        // %d on Stringer should give integer if possible
        {"%s", time.Time{}.Month(), "January"},
@@ -961,6 +972,9 @@ var fmtTests = []struct {
        {"%☠", float32(1.2345678), "%!☠(float32=1.2345678)"},
        {"%☠", 1.2345678 + 1.2345678i, "%!☠(complex128=(1.2345678+1.2345678i))"},
        {"%☠", complex64(1.2345678 + 1.2345678i), "%!☠(complex64=(1.2345678+1.2345678i))"},
+       {"%☠", &intVar, "%!☠(*int=0xPTR)"},
+       {"%☠", make(chan int), "%!☠(chan int=0xPTR)"},
+       {"%☠", func() {}, "%!☠(func()=0xPTR)"},
 }
 
 // zeroFill generates zero-filled strings of the specified width. The length
@@ -972,27 +986,37 @@ func zeroFill(prefix string, width int, suffix string) string {
 func TestSprintf(t *testing.T) {
        for _, tt := range fmtTests {
                s := Sprintf(tt.fmt, tt.val)
-               if i := strings.Index(tt.out, "PTR"); i >= 0 {
-                       pattern := "PTR"
-                       chars := "0123456789abcdefABCDEF"
+               i := strings.Index(tt.out, "PTR")
+               if i >= 0 && i < len(s) {
+                       var pattern, chars string
                        switch {
-                       case strings.HasPrefix(tt.out[i:], "PTR_d"):
-                               pattern = "PTR_d"
-                               chars = chars[:10]
+                       case strings.HasPrefix(tt.out[i:], "PTR_b"):
+                               pattern = "PTR_b"
+                               chars = "01"
                        case strings.HasPrefix(tt.out[i:], "PTR_o"):
                                pattern = "PTR_o"
-                               chars = chars[:8]
+                               chars = "01234567"
+                       case strings.HasPrefix(tt.out[i:], "PTR_d"):
+                               pattern = "PTR_d"
+                               chars = "0123456789"
                        case strings.HasPrefix(tt.out[i:], "PTR_x"):
                                pattern = "PTR_x"
+                               chars = "0123456789abcdef"
+                       case strings.HasPrefix(tt.out[i:], "PTR_X"):
+                               pattern = "PTR_X"
+                               chars = "0123456789ABCDEF"
+                       default:
+                               pattern = "PTR"
+                               chars = "0123456789abcdefABCDEF"
                        }
-                       j := i
-                       for ; j < len(s); j++ {
-                               c := s[j]
-                               if !strings.ContainsRune(chars, rune(c)) {
+                       p := s[:i] + pattern
+                       for j := i; j < len(s); j++ {
+                               if !strings.ContainsRune(chars, rune(s[j])) {
+                                       p += s[j:]
                                        break
                                }
                        }
-                       s = s[0:i] + pattern + s[j:]
+                       s = p
                }
                if s != tt.out {
                        if _, ok := tt.val.(string); ok {

diff --git a/src/fmt/print.go b/src/fmt/print.go
index 9fb33b25a454f79f1354dffff5ac3da908914d37..140269553695769dee66f278879cb21d24ddd0db 100644 (file)
--- a/src/fmt/print.go
+++ b/src/fmt/print.go
@@ -541,18 +541,6 @@ func (p *pp) fmtBytes(v []byte, verb rune, typeString string) {
 }
 
 func (p *pp) fmtPointer(value reflect.Value, verb rune) {
-       use0x64 := true
-       switch verb {
-       case 'p', 'v':
-               // ok
-       case 'b', 'd', 'o', 'x', 'X':
-               use0x64 = false
-               // ok
-       default:
-               p.badVerb(verb)
-               return
-       }
-
        var u uintptr
        switch value.Kind() {
        case reflect.Chan, reflect.Func, reflect.Map, reflect.Ptr, reflect.Slice, reflect.UnsafePointer:
@@ -562,24 +550,31 @@ func (p *pp) fmtPointer(value reflect.Value, verb rune) {
                return
        }
 
-       if p.fmt.sharpV {
-               p.buf.WriteByte('(')
-               p.buf.WriteString(value.Type().String())
-               p.buf.WriteString(")(")
-               if u == 0 {
-                       p.buf.WriteString(nilString)
-               } else {
-                       p.fmt0x64(uint64(u), true)
-               }
-               p.buf.WriteByte(')')
-       } else if verb == 'v' && u == 0 {
-               p.buf.WriteString(nilAngleString)
-       } else {
-               if use0x64 {
-                       p.fmt0x64(uint64(u), !p.fmt.sharp)
+       switch verb {
+       case 'v':
+               if p.fmt.sharpV {
+                       p.buf.WriteByte('(')
+                       p.buf.WriteString(value.Type().String())
+                       p.buf.WriteString(")(")
+                       if u == 0 {
+                               p.buf.WriteString(nilString)
+                       } else {
+                               p.fmt0x64(uint64(u), true)
+                       }
+                       p.buf.WriteByte(')')
                } else {
-                       p.fmtUint64(uint64(u), verb)
+                       if u == 0 {
+                               p.fmt.padString(nilAngleString)
+                       } else {
+                               p.fmt0x64(uint64(u), !p.fmt.sharp)
+                       }
                }
+       case 'p':
+               p.fmt0x64(uint64(u), !p.fmt.sharp)
+       case 'b', 'o', 'd', 'x', 'X':
+               p.fmtUint64(uint64(u), verb)
+       default:
+               p.badVerb(verb)
        }
 }