Skip to content

Commit

Permalink
[dev.typeparams] go/types, types2: revert fancy struct printing (fixe…
Browse files Browse the repository at this point in the history
…s x/tools tests)

An embedded struct field is embedded by mentioning its type.
The fact that the field name may be different and derived
from the type doesn't matter for the struct type.

Do print the embedded type rather than the derived field
name, as we have always done in the past. Remove the fancy
new code which was just plain wrong.

The struct output printing is only relevant for debugging
and test cases. Reverting to the original code (pre-generics)
fixes a couple of x/tools tests.

Unfortunately, the original code is (also) not correct for
embedded type aliases. Adjusted a gccgoimporter test
accordingly and filed issue #44410.

This is a follow-up on https://golang.org/cl/293961 which
addressed the issue only partially and left the incorrect
code in place.

Change-Id: Icb7a89c12ef7929c221fb1a5792f144f7fcd5855
Reviewed-on: https://go-review.googlesource.com/c/go/+/293962
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
  • Loading branch information
griesemer authored and findleyr committed Feb 19, 2021
1 parent 2f37939 commit dfe0ef9
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 68 deletions.
36 changes: 6 additions & 30 deletions src/cmd/compile/internal/types2/typestring.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,19 +126,14 @@ func writeType(buf *bytes.Buffer, typ Type, qf Qualifier, visited []Type) {
if i > 0 {
buf.WriteString("; ")
}
buf.WriteString(f.name)
if f.embedded {
// emphasize that the embedded field's name
// doesn't match the field's type name
if f.name != embeddedFieldName(f.typ) {
buf.WriteString(" /* = ")
writeType(buf, f.typ, qf, visited)
buf.WriteString(" */")
}
} else {
// This doesn't do the right thing for embedded type
// aliases where we should print the alias name, not
// the aliased type (see issue #44410).
if !f.embedded {
buf.WriteString(f.name)
buf.WriteByte(' ')
writeType(buf, f.typ, qf, visited)
}
writeType(buf, f.typ, qf, visited)
if tag := t.Tag(i); tag != "" {
fmt.Fprintf(buf, " %q", tag)
}
Expand Down Expand Up @@ -423,25 +418,6 @@ func writeSignature(buf *bytes.Buffer, sig *Signature, qf Qualifier, visited []T
writeTuple(buf, sig.results, false, qf, visited)
}

// embeddedFieldName returns an embedded field's name given its type.
// The result is "" if the type doesn't have an embedded field name.
func embeddedFieldName(typ Type) string {
switch t := typ.(type) {
case *Basic:
return t.name
case *Named:
return t.obj.name
case *Pointer:
// *T is ok, but **T is not
if _, ok := t.base.(*Pointer); !ok {
return embeddedFieldName(t.base)
}
case *instance:
return t.base.obj.name
}
return "" // not a (pointer to) a defined type
}

// subscript returns the decimal (utf8) representation of x using subscript digits.
func subscript(x uint64) string {
const w = len("₀") // all digits 0...9 have the same utf8 width
Expand Down
2 changes: 1 addition & 1 deletion src/go/internal/gccgoimporter/importer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ var importerTests = [...]importerTest{
{pkgpath: "nointerface", name: "I", want: "type I int"},
{pkgpath: "issue29198", name: "FooServer", gccgoVersion: 7, want: "type FooServer struct{FooServer *FooServer; user string; ctx context.Context}"},
{pkgpath: "issue30628", name: "Apple", want: "type Apple struct{hey sync.RWMutex; x int; RQ [517]struct{Count uintptr; NumBytes uintptr; Last uintptr}}"},
{pkgpath: "issue31540", name: "S", gccgoVersion: 7, want: "type S struct{b int; A2 /* = map[Y]Z */}"},
{pkgpath: "issue31540", name: "S", gccgoVersion: 7, want: "type S struct{b int; map[Y]Z}"}, // should want "type S struct{b int; A2}" (issue #44410)
{pkgpath: "issue34182", name: "T1", want: "type T1 struct{f *T2}"},
{pkgpath: "notinheap", name: "S", want: "type S struct{}"},
}
Expand Down
43 changes: 6 additions & 37 deletions src/go/types/typestring.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,26 +126,14 @@ func writeType(buf *bytes.Buffer, typ Type, qf Qualifier, visited []Type) {
if i > 0 {
buf.WriteString("; ")
}
// For compatibility with versions < go1.16, qualify the field name
// of embedded fields with the package name. Various tests (such as
// in x/tools/cmd/guru) depend on this output; and x/tools packages
// are run against earlier versions of Go.
if n, _ := f.typ.(*Named); f.embedded && n != nil && n.obj != nil && n.obj.pkg != nil {
writePackage(buf, n.obj.pkg, qf)
}
buf.WriteString(f.name)
if f.embedded {
// emphasize that the embedded field's name
// doesn't match the field's type name
if f.name != embeddedFieldName(f.typ) {
buf.WriteString(" /* = ")
writeType(buf, f.typ, qf, visited)
buf.WriteString(" */")
}
} else {
// This doesn't do the right thing for embedded type
// aliases where we should print the alias name, not
// the aliased type (see issue #44410).
if !f.embedded {
buf.WriteString(f.name)
buf.WriteByte(' ')
writeType(buf, f.typ, qf, visited)
}
writeType(buf, f.typ, qf, visited)
if tag := t.Tag(i); tag != "" {
fmt.Fprintf(buf, " %q", tag)
}
Expand Down Expand Up @@ -431,25 +419,6 @@ func writeSignature(buf *bytes.Buffer, sig *Signature, qf Qualifier, visited []T
writeTuple(buf, sig.results, false, qf, visited)
}

// embeddedFieldName returns an embedded field's name given its type.
// The result is "" if the type doesn't have an embedded field name.
func embeddedFieldName(typ Type) string {
switch t := typ.(type) {
case *Basic:
return t.name
case *Named:
return t.obj.name
case *Pointer:
// *T is ok, but **T is not
if _, ok := t.base.(*Pointer); !ok {
return embeddedFieldName(t.base)
}
case *instance:
return t.base.obj.name
}
return "" // not a (pointer to) a defined type
}

// subscript returns the decimal (utf8) representation of x using subscript digits.
func subscript(x uint64) string {
const w = len("₀") // all digits 0...9 have the same utf8 width
Expand Down

0 comments on commit dfe0ef9

Please sign in to comment.