From 8e2315678667f498f50f0b38dd064e8cc7f6c215 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Mo=CC=88hrmann?= Date: Sun, 6 Mar 2016 00:39:37 +0100 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Rob Pike --- src/fmt/fmt_test.go | 110 +++++++++++++++++++++++++++----------------- src/fmt/print.go | 51 +++++++++----------- 2 files changed, 90 insertions(+), 71 deletions(-) diff --git a/src/fmt/fmt_test.go b/src/fmt/fmt_test.go index 2ff3515c528f4..5b197d8a398f6 100644 --- 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, " "}, {"%-10T", 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=)"}, - {"%q", new(int), "%!q(*int=0xPTR)"}, - - // %v on pointers formats 0 as + {"%p", func() {}, "0xPTR"}, + {"%p", 27, "%!p(int=27)"}, // not a pointer at all + {"%p", nil, "%!p()"}, // nil on its own has no type ... + {"%#p", nil, "%!p()"}, // ... 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, ""}, + {"%#v", nil, ""}, {"%v", (*int)(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), " "}, + {"%-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 9fb33b25a454f..1402695536957 100644 --- 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) } }