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>

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", func() {}, "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", 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"
-			}
-			j := i
-			for ; j < len(s); j++ {
-				c := s[j]
-				if !strings.ContainsRune(chars, rune(c)) {
+				chars = "0123456789abcdef"
+			case strings.HasPrefix(tt.out[i:], "PTR_X"):
+				pattern = "PTR_X"
+				chars = "0123456789ABCDEF"
+			default:
+				pattern = "PTR"
+				chars = "0123456789abcdefABCDEF"
+			}
+			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 {

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,6 +550,8 @@ func (p *pp) fmtPointer(value reflect.Value, verb rune) {
 		return
 	}
 
+	switch verb {
+	case 'v':
 		if p.fmt.sharpV {
 			p.buf.WriteByte('(')
 			p.buf.WriteString(value.Type().String())
@@ -572,14 +562,19 @@ func (p *pp) fmtPointer(value reflect.Value, verb rune) {
 				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)
+			if u == 0 {
+				p.fmt.padString(nilAngleString)
 			} else {
-			p.fmtUint64(uint64(u), verb)
+				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)
 	}
 }