From 626129237d3cf9192cfcec4ec544cedcc216ce72 Mon Sep 17 00:00:00 2001 From: Pierre Gimalac Date: Tue, 18 Feb 2025 00:26:39 +0100 Subject: [PATCH 1/2] fix: refactor appendTypeName to avoid disabling DCE --- cmp/internal/value/name.go | 86 +++++++++++++++++++++++--------------- cmp/path.go | 2 +- 2 files changed, 54 insertions(+), 34 deletions(-) diff --git a/cmp/internal/value/name.go b/cmp/internal/value/name.go index 7b498bb..0744b93 100644 --- a/cmp/internal/value/name.go +++ b/cmp/internal/value/name.go @@ -14,10 +14,19 @@ var anyType = reflect.TypeOf((*interface{})(nil)).Elem() // TypeString is nearly identical to reflect.Type.String, // but has an additional option to specify that full type names be used. func TypeString(t reflect.Type, qualified bool) string { - return string(appendTypeName(nil, t, qualified, false)) + interfaceName := interfaceNameNotQualified + if qualified { + interfaceName = interfaceNameQualified + } + + return string(appendTypeName(nil, t, interfaceName, qualified, false)) +} + +func TypeStringNotQualified(t reflect.Type) string { + return string(appendTypeName(nil, t, interfaceNameNotQualified, false, false)) } -func appendTypeName(b []byte, t reflect.Type, qualified, elideFunc bool) []byte { +func appendTypeName(b []byte, t reflect.Type, interfaceName interfaceNamer, qualified, elideFunc bool) []byte { // BUG: Go reflection provides no way to disambiguate two named types // of the same name and within the same package, // but declared within the namespace of different functions. @@ -57,7 +66,7 @@ func appendTypeName(b []byte, t reflect.Type, qualified, elideFunc bool) []byte b = append(b, "<-"...) } b = append(b, ' ') - b = appendTypeName(b, t.Elem(), qualified, false) + b = appendTypeName(b, t.Elem(), interfaceName, qualified, false) case reflect.Func: if !elideFunc { b = append(b, "func"...) @@ -69,9 +78,9 @@ func appendTypeName(b []byte, t reflect.Type, qualified, elideFunc bool) []byte } if i == t.NumIn()-1 && t.IsVariadic() { b = append(b, "..."...) - b = appendTypeName(b, t.In(i).Elem(), qualified, false) + b = appendTypeName(b, t.In(i).Elem(), interfaceName, qualified, false) } else { - b = appendTypeName(b, t.In(i), qualified, false) + b = appendTypeName(b, t.In(i), interfaceName, qualified, false) } } b = append(b, ')') @@ -80,14 +89,14 @@ func appendTypeName(b []byte, t reflect.Type, qualified, elideFunc bool) []byte // Do nothing case 1: b = append(b, ' ') - b = appendTypeName(b, t.Out(0), qualified, false) + b = appendTypeName(b, t.Out(0), interfaceName, qualified, false) default: b = append(b, " ("...) for i := 0; i < t.NumOut(); i++ { if i > 0 { b = append(b, ", "...) } - b = appendTypeName(b, t.Out(i), qualified, false) + b = appendTypeName(b, t.Out(i), interfaceName, qualified, false) } b = append(b, ')') } @@ -108,7 +117,7 @@ func appendTypeName(b []byte, t reflect.Type, qualified, elideFunc bool) []byte b = append(b, sf.Name...) b = append(b, ' ') } - b = appendTypeName(b, sf.Type, qualified, false) + b = appendTypeName(b, sf.Type, interfaceName, qualified, false) if sf.Tag != "" { b = append(b, ' ') b = strconv.AppendQuote(b, string(sf.Tag)) @@ -126,39 +135,50 @@ func appendTypeName(b []byte, t reflect.Type, qualified, elideFunc bool) []byte b = strconv.AppendUint(b, uint64(t.Len()), 10) } b = append(b, ']') - b = appendTypeName(b, t.Elem(), qualified, false) + b = appendTypeName(b, t.Elem(), interfaceName, qualified, false) case reflect.Map: b = append(b, "map["...) - b = appendTypeName(b, t.Key(), qualified, false) + b = appendTypeName(b, t.Key(), interfaceName, qualified, false) b = append(b, ']') - b = appendTypeName(b, t.Elem(), qualified, false) + b = appendTypeName(b, t.Elem(), interfaceName, qualified, false) case reflect.Ptr: b = append(b, '*') - b = appendTypeName(b, t.Elem(), qualified, false) + b = appendTypeName(b, t.Elem(), interfaceName, qualified, false) case reflect.Interface: - b = append(b, "interface{ "...) - for i := 0; i < t.NumMethod(); i++ { - if i > 0 { - b = append(b, "; "...) - } - m := t.Method(i) - if qualified && m.PkgPath != "" { - b = append(b, '"') - b = append(b, m.PkgPath...) - b = append(b, '"') - b = append(b, '.') - } - b = append(b, m.Name...) - b = appendTypeName(b, m.Type, qualified, true) - } - if b[len(b)-1] == ' ' { - b = b[:len(b)-1] - } else { - b = append(b, ' ') - } - b = append(b, '}') + b = interfaceName(b, t) default: panic("invalid kind: " + k.String()) } return b } + +type interfaceNamer func([]byte, reflect.Type) []byte + +func interfaceNameQualified(b []byte, t reflect.Type) []byte { + b = append(b, "interface{ "...) + for i := 0; i < t.NumMethod(); i++ { + if i > 0 { + b = append(b, "; "...) + } + m := t.Method(i) + if m.PkgPath != "" { + b = append(b, '"') + b = append(b, m.PkgPath...) + b = append(b, '"') + b = append(b, '.') + } + b = append(b, m.Name...) + b = appendTypeName(b, m.Type, interfaceNameQualified, true, true) + } + if b[len(b)-1] == ' ' { + b = b[:len(b)-1] + } else { + b = append(b, ' ') + } + b = append(b, '}') + return b +} + +func interfaceNameNotQualified(b []byte, t reflect.Type) []byte { + return append(b, t.String()...) +} diff --git a/cmp/path.go b/cmp/path.go index c3c1456..9c30623 100644 --- a/cmp/path.go +++ b/cmp/path.go @@ -168,7 +168,7 @@ func (ps pathStep) String() string { if ps.typ == nil { return "" } - s := value.TypeString(ps.typ, false) + s := value.TypeStringNotQualified(ps.typ) if s == "" || strings.ContainsAny(s, "{}\n") { return "root" // Type too simple or complex to print } From 20b292f2783930aefb6510c750e8548bcf68453d Mon Sep 17 00:00:00 2001 From: Pierre Gimalac Date: Tue, 18 Feb 2025 13:25:00 +0100 Subject: [PATCH 2/2] fix: tests --- cmp/internal/value/name.go | 2 +- cmp/internal/value/name_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmp/internal/value/name.go b/cmp/internal/value/name.go index 0744b93..16d8c6d 100644 --- a/cmp/internal/value/name.go +++ b/cmp/internal/value/name.go @@ -155,7 +155,7 @@ func appendTypeName(b []byte, t reflect.Type, interfaceName interfaceNamer, qual type interfaceNamer func([]byte, reflect.Type) []byte func interfaceNameQualified(b []byte, t reflect.Type) []byte { - b = append(b, "interface{ "...) + b = append(b, "interface { "...) for i := 0; i < t.NumMethod(); i++ { if i > 0 { b = append(b, "; "...) diff --git a/cmp/internal/value/name_test.go b/cmp/internal/value/name_test.go index c177e72..e6f6e0a 100644 --- a/cmp/internal/value/name_test.go +++ b/cmp/internal/value/name_test.go @@ -114,7 +114,7 @@ func TestTypeString(t *testing.T) { want: "*any", }, { in: (*interface{ Read([]byte) (int, error) })(nil), - want: "*interface{ Read([]uint8) (int, error) }", + want: "*interface { Read([]uint8) (int, error) }", }, { in: (*interface { F1() @@ -123,7 +123,7 @@ func TestTypeString(t *testing.T) { F4(int, Named) (int, error) F5(...Named) })(nil), - want: "*interface{ F1(); F2($PackagePath.Named); F3() $PackagePath.Named; F4(int, $PackagePath.Named) (int, error); F5(...$PackagePath.Named) }", + want: "*interface { F1(); F2($PackagePath.Named); F3() $PackagePath.Named; F4(int, $PackagePath.Named) (int, error); F5(...$PackagePath.Named) }", }} for _, tt := range tests {