cmd/compile: add and enable (internal) option to only track named types

The new export format keeps track of all types that are exported.
If a type is seen that was exported before, only a reference to
that type is emitted. The importer maintains a list of all the
seen types and uses that list to resolve type references.

The existing compiler infrastructure's invariants assumes that
only named types are referred to before they are fully set up.
Referring to unnamed incomplete types causes problems. One of
the issues was #15548.

Added a new internal flag 'trackAllTypes' to enable/disable
this type tracking. With this change only named types are
tracked.

Verified that this fix also addresses #15548, even w/o the
prior fix for that issue (in fact that prior fix is turned
off if trackAllTypes is disabled because it's not needed).

The test for #15548 covers also this change.

For #15548.

Change-Id: Id0b3ff983629703d025a442823f99649fd728a56
Reviewed-on: https://go-review.googlesource.com/22839
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

src/cmd/compile/internal/gc/bexport.go

@@ -125,6 +125,17 @@ const exportVersion = "v0"
 // Leave for debugging.
 const exportInlined = true // default: true
 
+// trackAllTypes enables cycle tracking for all types, not just named
+// types. The existing compiler invariants assume that unnamed types
+// that are not completely set up are not used, or else there are spurious
+// errors.
+// If disabled, only named types are tracked, possibly leading to slightly
+// less efficient encoding in rare cases. It also prevents the export of
+// some corner-case type declarations (but those are not handled correctly
+// with with the textual export format either).
+// TODO(gri) enable and remove once issues caused by it are fixed
+const trackAllTypes = false
+
 type exporter struct {
 	out *bufio.Writer
 
@@ -159,6 +170,10 @@ func export(out *bufio.Writer, trace bool) int {
 		trace:         trace,
 	}
 
+	// TODO(gri) clean up the ad-hoc encoding of the file format below
+	// (we need this so we can read the builtin package export data
+	// easily w/o being affected by format changes)
+
 	// first byte indicates low-level encoding format
 	var format byte = 'c' // compact
 	if debugFormat {
@@ -166,6 +181,12 @@ func export(out *bufio.Writer, trace bool) int {
 	}
 	p.rawByte(format)
 
+	format = 'n' // track named types only
+	if trackAllTypes {
+		format = 'a'
+	}
+	p.rawByte(format)
+
 	// posInfo exported or not?
 	p.bool(p.posInfoFormat)
 
@@ -585,14 +606,21 @@ func (p *exporter) typ(t *Type) {
 	}
 
 	// otherwise, remember the type, write the type tag (< 0) and type data
-	if p.trace {
-		p.tracef("T%d = {>\n", len(p.typIndex))
-		defer p.tracef("<\n} ")
+	if trackAllTypes {
+		if p.trace {
+			p.tracef("T%d = {>\n", len(p.typIndex))
+			defer p.tracef("<\n} ")
+		}
+		p.typIndex[t] = len(p.typIndex)
 	}
-	p.typIndex[t] = len(p.typIndex)
 
 	// pick off named types
 	if tsym := t.Sym; tsym != nil {
+		if !trackAllTypes {
+			// if we don't track all types, track named types now
+			p.typIndex[t] = len(p.typIndex)
+		}
+
 		// Predeclared types should have been found in the type map.
 		if t.Orig == t {
 			Fatalf("exporter: predeclared type missing from type map?")

src/cmd/compile/internal/gc/bimport.go

@@ -24,10 +24,11 @@ type importer struct {
 	buf []byte // reused for reading strings
 
 	// object lists, in order of deserialization
-	strList  []string
-	pkgList  []*Pkg
-	typList  []*Type
-	funcList []*Node // nil entry means already declared
+	strList       []string
+	pkgList       []*Pkg
+	typList       []*Type
+	funcList      []*Node // nil entry means already declared
+	trackAllTypes bool
 
 	// for delayed type verification
 	cmpList []struct{ pt, t *Type }
@@ -59,6 +60,8 @@ func Import(in *bufio.Reader) {
 		Fatalf("importer: invalid encoding format in export data: got %q; want 'c' or 'd'", format)
 	}
 
+	p.trackAllTypes = p.rawByte() == 'a'
+
 	p.posInfoFormat = p.bool()
 
 	// --- generic export data ---
@@ -331,7 +334,9 @@ func (p *importer) pos() {
 
 func (p *importer) newtyp(etype EType) *Type {
 	t := typ(etype)
-	p.typList = append(p.typList, t)
+	if p.trackAllTypes {
+		p.typList = append(p.typList, t)
+	}
 	return t
 }
 
@@ -389,7 +394,13 @@ func (p *importer) typ() *Type {
 		// read underlying type
 		// parser.go:hidden_type
 		t0 := p.typ()
-		p.importtype(t, t0) // parser.go:hidden_import
+		if p.trackAllTypes {
+			// If we track all types, we cannot check equality of previously
+			// imported types until later. Use customized version of importtype.
+			p.importtype(t, t0)
+		} else {
+			importtype(t, t0)
+		}
 
 		// interfaces don't have associated methods
 		if t0.IsInterface() {

src/go/internal/gcimporter/bimport.go

@@ -23,9 +23,10 @@ type importer struct {
 	buf     []byte // for reading strings
 
 	// object lists
-	strList []string         // in order of appearance
-	pkgList []*types.Package // in order of appearance
-	typList []types.Type     // in order of appearance
+	strList       []string         // in order of appearance
+	pkgList       []*types.Package // in order of appearance
+	typList       []types.Type     // in order of appearance
+	trackAllTypes bool
 
 	// position encoding
 	posInfoFormat bool
@@ -59,6 +60,8 @@ func BImportData(imports map[string]*types.Package, data []byte, path string) (i
 		return p.read, nil, fmt.Errorf("invalid encoding format in export data: got %q; want 'c' or 'd'", format)
 	}
 
+	p.trackAllTypes = p.rawByte() == 'a'
+
 	p.posInfoFormat = p.int() != 0
 
 	// --- generic export data ---
@@ -93,7 +96,12 @@ func BImportData(imports map[string]*types.Package, data []byte, path string) (i
 
 	// complete interfaces
 	for _, typ := range p.typList {
-		if it, ok := typ.(*types.Interface); ok {
+		// If we only record named types (!p.trackAllTypes),
+		// we must check the underlying types here. If we
+		// track all types, the Underlying() method call is
+		// not needed.
+		// TODO(gri) Remove if p.trackAllTypes is gone.
+		if it, ok := typ.Underlying().(*types.Interface); ok {
 			it.Complete()
 		}
 	}
@@ -304,7 +312,9 @@ func (p *importer) typ(parent *types.Package) types.Type {
 
 	case arrayTag:
 		t := new(types.Array)
-		p.record(t)
+		if p.trackAllTypes {
+			p.record(t)
+		}
 
 		n := p.int64()
 		*t = *types.NewArray(p.typ(parent), n)
@@ -312,35 +322,45 @@ func (p *importer) typ(parent *types.Package) types.Type {
 
 	case sliceTag:
 		t := new(types.Slice)
-		p.record(t)
+		if p.trackAllTypes {
+			p.record(t)
+		}
 
 		*t = *types.NewSlice(p.typ(parent))
 		return t
 
 	case dddTag:
 		t := new(dddSlice)
-		p.record(t)
+		if p.trackAllTypes {
+			p.record(t)
+		}
 
 		t.elem = p.typ(parent)
 		return t
 
 	case structTag:
 		t := new(types.Struct)
-		p.record(t)
+		if p.trackAllTypes {
+			p.record(t)
+		}
 
 		*t = *types.NewStruct(p.fieldList(parent))
 		return t
 
 	case pointerTag:
 		t := new(types.Pointer)
-		p.record(t)
+		if p.trackAllTypes {
+			p.record(t)
+		}
 
 		*t = *types.NewPointer(p.typ(parent))
 		return t
 
 	case signatureTag:
 		t := new(types.Signature)
-		p.record(t)
+		if p.trackAllTypes {
+			p.record(t)
+		}
 
 		params, isddd := p.paramList()
 		result, _ := p.paramList()
@@ -353,7 +373,9 @@ func (p *importer) typ(parent *types.Package) types.Type {
 		// such cycle must contain a named type which would have been
 		// first defined earlier.
 		n := len(p.typList)
-		p.record(nil)
+		if p.trackAllTypes {
+			p.record(nil)
+		}
 
 		// no embedded interfaces with gc compiler
 		if p.int() != 0 {
@@ -361,12 +383,16 @@ func (p *importer) typ(parent *types.Package) types.Type {
 		}
 
 		t := types.NewInterface(p.methodList(parent), nil)
-		p.typList[n] = t
+		if p.trackAllTypes {
+			p.typList[n] = t
+		}
 		return t
 
 	case mapTag:
 		t := new(types.Map)
-		p.record(t)
+		if p.trackAllTypes {
+			p.record(t)
+		}
 
 		key := p.typ(parent)
 		val := p.typ(parent)
@@ -375,7 +401,9 @@ func (p *importer) typ(parent *types.Package) types.Type {
 
 	case chanTag:
 		t := new(types.Chan)
-		p.record(t)
+		if p.trackAllTypes {
+			p.record(t)
+		}
 
 		var dir types.ChanDir
 		// tag values must match the constants in cmd/compile/internal/gc/go.go

src/go/internal/gcimporter/gcimporter_test.go

@@ -351,9 +351,9 @@ func TestIssue13898(t *testing.T) {
 	}
 
 	// lookup go/types.Object.Pkg method
-	m, _, _ := types.LookupFieldOrMethod(typ, false, nil, "Pkg")
+	m, index, indirect := types.LookupFieldOrMethod(typ, false, nil, "Pkg")
 	if m == nil {
-		t.Fatal("go/types.Object.Pkg not found")
+		t.Fatalf("go/types.Object.Pkg not found (index = %v, indirect = %v)", index, indirect)
 	}
 
 	// the method must belong to go/types

test/fixedbugs/bug398.go

@@ -8,17 +8,20 @@
 
 package p
 
-type I1 interface {
-      F() interface{I1}
+type i1 interface {
+      F() interface{i1}
 }
 
-type I2 interface {
-      F() interface{I2}
+type i2 interface {
+      F() interface{i2}
 }       
 
-var v1 I1
-var v2 I2
+var v1 i1
+var v2 i2
 
 func f() bool {
        return v1 == v2
 }
+
+// TODO(gri) Change test to use exported interfaces.
+// See issue #15596 for details.
\ No newline at end of file