From 6b4cf2be9385ea25fed011f7c862d9e023b71df6 Mon Sep 17 00:00:00 2001 From: Dan Scales Date: Thu, 30 Sep 2021 15:45:29 -0700 Subject: [PATCH] cmd/compile: improving printing of type names in func/meth instantiations Change to using types.(*Type).LinkString() for printing names of types in function/method instantiations. (e.g. f[int] or Value[p.Myint].Set()) LinkString already generates a unique string description for t, using package paths, except that it uses "" for the local package path. The "" will be expanded in the linker, so the names in the executable will have full package paths everywhere and de-duplication of function/method instantiations will work properly. We do need to add an explicit substitution of "" in ReadImports() for function/method names. We previously were using NameString(), which doesn't use full package paths, so is not fully unique. We had also discussed that we would prefer to minimize spaces in function/method instantiation names. So, I changed LinkString() to eliminate all unneeded spaces. In the one case where we need a separator, which is between field names and types, we use a "#" instead of a space. This change has the advantage of eliminating spaces in some existing non-generic function name - mainly .type.eq functions for anonymous types (e.g. "type..eq.struct { runtime.gList; runtime.n int32 }") shows up in a hello-world executable (as Cherry pointed out). We do not need an analogous function for types2 right now, since we create all instantiations using types1 types. In the one case where we need to create an instantiation during types2-to-types1 translation, we convert the types to types1 first (see (*irgen).instTypeName2). Change-Id: Iac4748fa0d0d6f89af59bd51076266986daee945 Reviewed-on: https://go-review.googlesource.com/c/go/+/353609 Trust: Dan Scales Run-TryBot: Dan Scales Reviewed-by: Matthew Dempsky --- src/cmd/compile/internal/typecheck/iimport.go | 11 ++++- src/cmd/compile/internal/typecheck/subr.go | 14 +++--- src/cmd/compile/internal/types/fmt.go | 47 +++++++++++++++---- 3 files changed, 53 insertions(+), 19 deletions(-) diff --git a/src/cmd/compile/internal/typecheck/iimport.go b/src/cmd/compile/internal/typecheck/iimport.go index 77119ce9bd..01ac1679b2 100644 --- a/src/cmd/compile/internal/typecheck/iimport.go +++ b/src/cmd/compile/internal/typecheck/iimport.go @@ -188,9 +188,18 @@ func ReadImports(pkg *types.Pkg, data string) { // Inline body index. for nPkgs := ird.uint64(); nPkgs > 0; nPkgs-- { pkg := p.pkgAt(ird.uint64()) + pkgPrefix := pkg.Prefix + "." for nSyms := ird.uint64(); nSyms > 0; nSyms-- { - s := pkg.Lookup(p.stringAt(ird.uint64())) + s2 := p.stringAt(ird.uint64()) + // Function/method instantiation names may include "" to + // represent the path name of the imported package (in type + // names), so replace "" with pkg.Prefix. The "" in the names + // will get replaced by the linker as well, so will not + // appear in the executable. Include the dot to avoid + // matching with struct tags ending in '"'. + s2 = strings.Replace(s2, "\"\".", pkgPrefix, -1) + s := pkg.Lookup(s2) off := ird.uint64() if _, ok := inlineImporter[s]; !ok { diff --git a/src/cmd/compile/internal/typecheck/subr.go b/src/cmd/compile/internal/typecheck/subr.go index 56e6ec0e27..68240329f5 100644 --- a/src/cmd/compile/internal/typecheck/subr.go +++ b/src/cmd/compile/internal/typecheck/subr.go @@ -919,14 +919,11 @@ func addTargs(b *bytes.Buffer, targs []*types.Type) { if i > 0 { b.WriteByte(',') } - // Use NameString(), which includes the package name for the local - // package, to make sure that type arguments (including type params), - // are uniquely specified. - tstring := targ.NameString() - // types1 uses "interface {" and types2 uses "interface{" - convert - // to consistent types2 format. Same for "struct {" - tstring = strings.Replace(tstring, "interface {", "interface{", -1) - tstring = strings.Replace(tstring, "struct {", "struct{", -1) + // Make sure that type arguments (including type params), are + // uniquely specified. LinkString() eliminates all spaces + // and includes the package path (local package path is "" before + // linker substitution). + tstring := targ.LinkString() b.WriteString(tstring) } b.WriteString("]") @@ -1440,6 +1437,7 @@ func Shapify(t *types.Type, index int) *types.Type { return s } + // LinkString specifies the type uniquely, but has no spaces. nm := fmt.Sprintf("%s_%d", u.LinkString(), index) sym := types.ShapePkg.Lookup(nm) if sym.Def != nil { diff --git a/src/cmd/compile/internal/types/fmt.go b/src/cmd/compile/internal/types/fmt.go index 2f81c7b2e1..c70e22c946 100644 --- a/src/cmd/compile/internal/types/fmt.go +++ b/src/cmd/compile/internal/types/fmt.go @@ -64,7 +64,7 @@ var NumImport = make(map[string]int) // The default is regular Go syntax (fmtGo). // fmtDebug is like fmtGo but for debugging dumps and prints the type kind too. // fmtTypeID and fmtTypeIDName are for generating various unique representations -// of types used in hashes and the linker. +// of types used in hashes, the linker, and function/method instantiations. type fmtMode int const ( @@ -461,15 +461,25 @@ func tconv2(b *bytes.Buffer, t *Type, verb rune, mode fmtMode, visited map[*Type case TINTER: if t.IsEmptyInterface() { - b.WriteString("interface {}") + if mode == fmtTypeID { + b.WriteString("interface{}") + } else { + b.WriteString("interface {}") + } break } - b.WriteString("interface {") + if mode == fmtTypeID { + b.WriteString("interface{") + } else { + b.WriteString("interface {") + } for i, f := range t.AllMethods().Slice() { if i != 0 { b.WriteByte(';') } - b.WriteByte(' ') + if mode != fmtTypeID { + b.WriteByte(' ') + } switch { case f.Sym == nil: // Check first that a symbol is defined for this type. @@ -485,7 +495,7 @@ func tconv2(b *bytes.Buffer, t *Type, verb rune, mode fmtMode, visited map[*Type } tconv2(b, f.Type, 'S', mode, visited) } - if t.AllMethods().Len() != 0 { + if t.AllMethods().Len() != 0 && mode != fmtTypeID { b.WriteByte(' ') } b.WriteByte('}') @@ -560,15 +570,21 @@ func tconv2(b *bytes.Buffer, t *Type, verb rune, mode fmtMode, visited map[*Type } b.WriteByte(byte(close)) } else { - b.WriteString("struct {") + if mode == fmtTypeID { + b.WriteString("struct{") + } else { + b.WriteString("struct {") + } for i, f := range t.Fields().Slice() { if i != 0 { b.WriteByte(';') } - b.WriteByte(' ') + if mode != fmtTypeID { + b.WriteByte(' ') + } fldconv(b, f, 'L', mode, visited, funarg) } - if t.NumFields() != 0 { + if t.NumFields() != 0 && mode != fmtTypeID { b.WriteByte(' ') } b.WriteByte('}') @@ -652,7 +668,14 @@ func fldconv(b *bytes.Buffer, f *Field, verb rune, mode fmtMode, visited map[*Ty if name != "" { b.WriteString(name) - b.WriteString(" ") + if mode == fmtTypeID { + // This is the one case where we can't omit the space, since + // we need a separate between field name and type, so we use + // "#" instead. + b.WriteString("#") + } else { + b.WriteString(" ") + } } if f.IsDDD() { @@ -667,7 +690,11 @@ func fldconv(b *bytes.Buffer, f *Field, verb rune, mode fmtMode, visited map[*Ty } if verb != 'S' && funarg == FunargNone && f.Note != "" { - b.WriteString(" ") + if mode != fmtTypeID { + b.WriteString(" ") + } + // TODO: for fmtTypeID, we should possibly using %-quoting, so + // space is %20, etc. b.WriteString(strconv.Quote(f.Note)) } } -- 2.50.0