go/types: externalize union type sets

Move calculated type sets for unions into a map, rather than storing
them on the Union type.

Type sets for unions only matter during calculation of interface type
sets, and to a lesser extent inside of Identical. The latter should not
be encountered during type checking, as Identical uses the precomputed
interface type set when comparing interfaces, and unions do not arise
outside of interface types.

Removing the tset field from Union potentially frees up memory, and
eliminates a source of races via calls to NewUnion and Identical. It
also sets the stage for recording Unions for every subexpression of
union terms, which preserves an existing invariant that BinaryExprs and
UnaryExprs should have a recorded type.

Updates #50093

Change-Id: I5956fa59be6b0907c3a71faeba9fa5dd8aae0d65
Reviewed-on: https://go-review.googlesource.com/c/go/+/371756
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>

diff --git a/src/go/types/check.go b/src/go/types/check.go
index 2dd38e2e1e406cd94b40b01151bb6d2b04914bfa..bad4d5c9cd19b8e15aafd5238b1f1292a48283c3 100644 (file)
--- a/src/go/types/check.go
+++ b/src/go/types/check.go
@@ -136,6 +136,7 @@ type Checker struct {
        imports       []*PkgName                // list of imported packages
        dotImportMap  map[dotImportKey]*PkgName // maps dot-imported objects to the package they were dot-imported through
        recvTParamMap map[*ast.Ident]*TypeParam // maps blank receiver type parameters to their type
+       unionTypeSets map[*Union]*_TypeSet      // computed type sets for union types
        mono          monoGraph                 // graph for detecting non-monomorphizable instantiation loops
 
        firstErr error                 // first error encountered
@@ -323,6 +324,7 @@ func (check *Checker) checkFiles(files []*ast.File) (err error) {
        check.pkgPathMap = nil
        check.seenPkgMap = nil
        check.recvTParamMap = nil
+       check.unionTypeSets = nil
        check.defTypes = nil
        check.ctxt = nil
 

diff --git a/src/go/types/predicates.go b/src/go/types/predicates.go
index 22ccdd7744db6951a825a02aecf5c35f5b05902e..1202db4049e822f90820a621b1c4875d41d20a51 100644 (file)
--- a/src/go/types/predicates.go
+++ b/src/go/types/predicates.go
@@ -289,8 +289,11 @@ func identical(x, y Type, cmpTags bool, p *ifacePair) bool {
 
        case *Union:
                if y, _ := y.(*Union); y != nil {
-                       xset := computeUnionTypeSet(nil, token.NoPos, x)
-                       yset := computeUnionTypeSet(nil, token.NoPos, y)
+                       // TODO(rfindley): can this be reached during type checking? If so,
+                       // consider passing a type set map.
+                       unionSets := make(map[*Union]*_TypeSet)
+                       xset := computeUnionTypeSet(nil, unionSets, token.NoPos, x)
+                       yset := computeUnionTypeSet(nil, unionSets, token.NoPos, y)
                        return xset.terms.equal(yset.terms)
                }
 

diff --git a/src/go/types/sizeof_test.go b/src/go/types/sizeof_test.go
index 69571d1159d0674854a49a52b087cec260d45f26..24cbc2283957740c43e7576a2788cee8746f971c 100644 (file)
--- a/src/go/types/sizeof_test.go
+++ b/src/go/types/sizeof_test.go
@@ -26,7 +26,7 @@ func TestSizeof(t *testing.T) {
                {Pointer{}, 8, 16},
                {Tuple{}, 12, 24},
                {Signature{}, 28, 56},
-               {Union{}, 16, 32},
+               {Union{}, 12, 24},
                {Interface{}, 44, 88},
                {Map{}, 16, 32},
                {Chan{}, 12, 24},

diff --git a/src/go/types/subst.go b/src/go/types/subst.go
index 169540365b69b2472264a24f29d88b888099ce90..b7e3b127798b4fbc78918c9a6c152462695345c3 100644 (file)
--- a/src/go/types/subst.go
+++ b/src/go/types/subst.go
@@ -130,7 +130,7 @@ func (subst *subster) typ(typ Type) Type {
                        // term list substitution may introduce duplicate terms (unlikely but possible).
                        // This is ok; lazy type set computation will determine the actual type set
                        // in normal form.
-                       return &Union{terms, nil}
+                       return &Union{terms}
                }
 
        case *Interface:

diff --git a/src/go/types/typeset.go b/src/go/types/typeset.go
index d39483f2549642d72254872e028bbf6246c259a3..0f2897b8c6f730c00333bd39bd159b4741ac4a12 100644 (file)
--- a/src/go/types/typeset.go
+++ b/src/go/types/typeset.go
@@ -201,6 +201,16 @@ func computeInterfaceTypeSet(check *Checker, pos token.Pos, ityp *Interface) *_T
        // reason.
        ityp.tset = &_TypeSet{terms: allTermlist} // TODO(gri) is this sufficient?
 
+       var unionSets map[*Union]*_TypeSet
+       if check != nil {
+               if check.unionTypeSets == nil {
+                       check.unionTypeSets = make(map[*Union]*_TypeSet)
+               }
+               unionSets = check.unionTypeSets
+       } else {
+               unionSets = make(map[*Union]*_TypeSet)
+       }
+
        // Methods of embedded interfaces are collected unchanged; i.e., the identity
        // of a method I.m's Func Object of an interface I is the same as that of
        // the method m in an interface that embeds interface I. On the other hand,
@@ -288,7 +298,7 @@ func computeInterfaceTypeSet(check *Checker, pos token.Pos, ityp *Interface) *_T
                                check.errorf(atPos(pos), _InvalidIfaceEmbed, "embedding interface element %s requires go1.18 or later", u)
                                continue
                        }
-                       tset := computeUnionTypeSet(check, pos, u)
+                       tset := computeUnionTypeSet(check, unionSets, pos, u)
                        if tset == &invalidTypeSet {
                                continue // ignore invalid unions
                        }
@@ -356,13 +366,13 @@ var invalidTypeSet _TypeSet
 
 // computeUnionTypeSet may be called with check == nil.
 // The result is &invalidTypeSet if the union overflows.
-func computeUnionTypeSet(check *Checker, pos token.Pos, utyp *Union) *_TypeSet {
-       if utyp.tset != nil {
-               return utyp.tset
+func computeUnionTypeSet(check *Checker, unionSets map[*Union]*_TypeSet, pos token.Pos, utyp *Union) *_TypeSet {
+       if tset, _ := unionSets[utyp]; tset != nil {
+               return tset
        }
 
        // avoid infinite recursion (see also computeInterfaceTypeSet)
-       utyp.tset = new(_TypeSet)
+       unionSets[utyp] = new(_TypeSet)
 
        var allTerms termlist
        for _, t := range utyp.terms {
@@ -389,11 +399,11 @@ func computeUnionTypeSet(check *Checker, pos token.Pos, utyp *Union) *_TypeSet {
                        if check != nil {
                                check.errorf(atPos(pos), _InvalidUnion, "cannot handle more than %d union terms (implementation limitation)", maxTermCount)
                        }
-                       utyp.tset = &invalidTypeSet
-                       return utyp.tset
+                       unionSets[utyp] = &invalidTypeSet
+                       return unionSets[utyp]
                }
        }
-       utyp.tset.terms = allTerms
+       unionSets[utyp].terms = allTerms
 
-       return utyp.tset
+       return unionSets[utyp]
 }

diff --git a/src/go/types/union.go b/src/go/types/union.go
index 1437bd4624a19af627427b306f134839bbfe7ece..9dd67a0db4dc49b7b64b215a8b217e9cf6d8a8d4 100644 (file)
--- a/src/go/types/union.go
+++ b/src/go/types/union.go
@@ -14,8 +14,7 @@ import (
 
 // A Union represents a union of terms embedded in an interface.
 type Union struct {
-       terms []*Term   // list of syntactical terms (not a canonicalized termlist)
-       tset  *_TypeSet // type set described by this union, computed lazily
+       terms []*Term // list of syntactical terms (not a canonicalized termlist)
 }
 
 // NewUnion returns a new Union type with the given terms.
@@ -24,7 +23,7 @@ func NewUnion(terms []*Term) *Union {
        if len(terms) == 0 {
                panic("empty union")
        }
-       return &Union{terms, nil}
+       return &Union{terms}
 }
 
 func (u *Union) Len() int         { return len(u.terms) }
@@ -110,7 +109,7 @@ func parseUnion(check *Checker, uexpr ast.Expr) Type {
                }
        })
 
-       u := &Union{terms, nil}
+       u := &Union{terms}
        check.recordTypeAndValue(uexpr, typexpr, u, nil)
        return u
 }