internal/gcimporter: support materialized aliases
This CL adds support for preserving materialized aliases
while types transit export data (indexed or unified).
(This CL depends on CL 574737 to the compiler and
go/types.)
Updates golang/go#65294
Updates golang/go#64581
Change-Id: I1a0a08357e4f6a480ba6250fbea327922e455873
Reviewed-on: https://go-review.googlesource.com/c/tools/+/574717
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
diff --git a/internal/gcimporter/gcimporter_test.go b/internal/gcimporter/gcimporter_test.go
index 95cc36c..b93cd2a 100644
--- a/internal/gcimporter/gcimporter_test.go
+++ b/internal/gcimporter/gcimporter_test.go
@@ -190,6 +190,7 @@
"nested.go": "fails to compile", // TODO(rfindley): investigate this.
"issue47631.go": "can not handle local type declarations",
"issue55101.go": "fails to compile",
+ "issue50259.go": "compiler crashes if GODEBUG=gotypesalias=1", // TODO(adonovan): delete when #66550 is fixed.
}
}
diff --git a/internal/gcimporter/iexport.go b/internal/gcimporter/iexport.go
index 638fc1d..683bd73 100644
--- a/internal/gcimporter/iexport.go
+++ b/internal/gcimporter/iexport.go
@@ -21,6 +21,7 @@
"sort"
"strconv"
"strings"
+ "unsafe"
"golang.org/x/tools/go/types/objectpath"
"golang.org/x/tools/internal/aliases"
@@ -464,7 +465,7 @@
switch obj := obj.(type) {
case *types.Var:
- w.tag('V')
+ w.tag(varTag)
w.pos(obj.Pos())
w.typ(obj.Type(), obj.Pkg())
@@ -482,9 +483,9 @@
// Function.
if sig.TypeParams().Len() == 0 {
- w.tag('F')
+ w.tag(funcTag)
} else {
- w.tag('G')
+ w.tag(genericFuncTag)
}
w.pos(obj.Pos())
// The tparam list of the function type is the declaration of the type
@@ -500,7 +501,7 @@
w.signature(sig)
case *types.Const:
- w.tag('C')
+ w.tag(constTag)
w.pos(obj.Pos())
w.value(obj.Type(), obj.Val())
@@ -508,7 +509,7 @@
t := obj.Type()
if tparam, ok := aliases.Unalias(t).(*types.TypeParam); ok {
- w.tag('P')
+ w.tag(typeParamTag)
w.pos(obj.Pos())
constraint := tparam.Constraint()
if p.version >= iexportVersionGo1_18 {
@@ -523,8 +524,13 @@
}
if obj.IsAlias() {
- w.tag('A')
+ w.tag(aliasTag)
w.pos(obj.Pos())
+ if alias, ok := t.(*aliases.Alias); ok {
+ // Preserve materialized aliases,
+ // even of non-exported types.
+ t = aliasRHS(alias)
+ }
w.typ(t, obj.Pkg())
break
}
@@ -536,9 +542,9 @@
}
if named.TypeParams().Len() == 0 {
- w.tag('T')
+ w.tag(typeTag)
} else {
- w.tag('U')
+ w.tag(genericTypeTag)
}
w.pos(obj.Pos())
@@ -548,7 +554,7 @@
w.tparamList(obj.Name(), named.TypeParams(), obj.Pkg())
}
- underlying := obj.Type().Underlying()
+ underlying := named.Underlying()
w.typ(underlying, obj.Pkg())
if types.IsInterface(t) {
@@ -739,7 +745,10 @@
}()
}
switch t := t.(type) {
- // TODO(adonovan): support types.Alias.
+ case *aliases.Alias:
+ // TODO(adonovan): support parameterized aliases, following *types.Named.
+ w.startType(aliasType)
+ w.qualifiedType(t.Obj())
case *types.Named:
if targs := t.TypeArgs(); targs.Len() > 0 {
@@ -1322,3 +1331,19 @@
func internalErrorf(format string, args ...interface{}) error {
return internalError(fmt.Sprintf(format, args...))
}
+
+// aliasRHS removes exactly one Alias constructor.
+func aliasRHS(alias *aliases.Alias) types.Type {
+ // TODO(adonovan): if proposal #66559 is accepted, this will
+ // become Alias.RHS(alias). In the meantime, we must punch
+ // through the drywall.
+ type go123Alias struct {
+ _ *types.TypeName
+ _ *types.TypeParamList
+ RHS types.Type
+ _ types.Type
+ }
+ var raw *go123Alias
+ *(**aliases.Alias)(unsafe.Pointer(&raw)) = alias
+ return raw.RHS
+}
diff --git a/internal/gcimporter/iexport_go118_test.go b/internal/gcimporter/iexport_go118_test.go
index c748fb3..3aed235 100644
--- a/internal/gcimporter/iexport_go118_test.go
+++ b/internal/gcimporter/iexport_go118_test.go
@@ -123,6 +123,12 @@
t.Fatal(err)
}
+ // TODO(adonovan): delete when #66550 is fixed.
+ if strings.Contains(os.Getenv("GODEBUG"), "gotypesalias=1") &&
+ entry.Name() == "issue50259.go" {
+ t.Skip("Skipping test of defined<->alias cycle under gotypesaliases=1 (#66550)")
+ }
+
if !bytes.HasPrefix(src, []byte("// run")) && !bytes.HasPrefix(src, []byte("// compile")) {
// We're bypassing the logic of run.go here, so be conservative about
// the files we consider in an attempt to make this test more robust to
diff --git a/internal/gcimporter/iexport_test.go b/internal/gcimporter/iexport_test.go
index 4ee79da..0da2599 100644
--- a/internal/gcimporter/iexport_test.go
+++ b/internal/gcimporter/iexport_test.go
@@ -32,6 +32,7 @@
"golang.org/x/tools/go/buildutil"
"golang.org/x/tools/go/gcexportdata"
"golang.org/x/tools/go/loader"
+ "golang.org/x/tools/internal/aliases"
"golang.org/x/tools/internal/gcimporter"
"golang.org/x/tools/internal/testenv"
"golang.org/x/tools/internal/typeparams/genericfeatures"
@@ -145,6 +146,7 @@
t.Errorf("Loaded only %d packages, want at least %d", numPkgs, want)
}
+ // TODO(adonovan): opt: parallelize this slow loop.
for _, pkg := range sorted {
if exportdata, err := iexport(conf.Fset, version, pkg); err != nil {
t.Error(err)
@@ -333,14 +335,15 @@
if xalias, yalias := x.IsAlias(), y.(*types.TypeName).IsAlias(); xalias != yalias {
return fmt.Errorf("mismatching IsAlias(): %s vs %s", x, y)
}
+
// equalType does not recurse into the underlying types of named types, so
// we must pass the underlying type explicitly here. However, in doing this
// we may skip checking the features of the named types themselves, in
// situations where the type name is not referenced by the underlying or
// any other top-level declarations. Therefore, we must explicitly compare
// named types here, before passing their underlying types into equalType.
- xn, _ := xt.(*types.Named)
- yn, _ := yt.(*types.Named)
+ xn, _ := aliases.Unalias(xt).(*types.Named)
+ yn, _ := aliases.Unalias(yt).(*types.Named)
if (xn == nil) != (yn == nil) {
return fmt.Errorf("mismatching types: %T vs %T", xt, yt)
}
diff --git a/internal/gcimporter/iimport.go b/internal/gcimporter/iimport.go
index 4d50eb8..2732121 100644
--- a/internal/gcimporter/iimport.go
+++ b/internal/gcimporter/iimport.go
@@ -80,6 +80,20 @@
typeParamType
instanceType
unionType
+ aliasType
+)
+
+// Object tags
+const (
+ varTag = 'V'
+ funcTag = 'F'
+ genericFuncTag = 'G'
+ constTag = 'C'
+ aliasTag = 'A'
+ genericAliasTag = 'B'
+ typeParamTag = 'P'
+ typeTag = 'T'
+ genericTypeTag = 'U'
)
// IImportData imports a package from the serialized package data
@@ -324,7 +338,7 @@
}
// SetConstraint can't be called if the constraint type is not yet complete.
- // When type params are created in the 'P' case of (*importReader).obj(),
+ // When type params are created in the typeParamTag case of (*importReader).obj(),
// the associated constraint type may not be complete due to recursion.
// Therefore, we defer calling SetConstraint there, and call it here instead
// after all types are complete.
@@ -546,25 +560,29 @@
pos := r.pos()
switch tag {
- case 'A':
+ case aliasTag:
typ := r.typ()
+ // TODO(adonovan): support generic aliases:
+ // if tag == genericAliasTag {
+ // tparams := r.tparamList()
+ // alias.SetTypeParams(tparams)
+ // }
+ r.declare(aliases.NewAlias(pos, r.currPkg, name, typ))
- r.declare(types.NewTypeName(pos, r.currPkg, name, typ))
-
- case 'C':
+ case constTag:
typ, val := r.value()
r.declare(types.NewConst(pos, r.currPkg, name, typ, val))
- case 'F', 'G':
+ case funcTag, genericFuncTag:
var tparams []*types.TypeParam
- if tag == 'G' {
+ if tag == genericFuncTag {
tparams = r.tparamList()
}
sig := r.signature(nil, nil, tparams)
r.declare(types.NewFunc(pos, r.currPkg, name, sig))
- case 'T', 'U':
+ case typeTag, genericTypeTag:
// Types can be recursive. We need to setup a stub
// declaration before recursing.
obj := types.NewTypeName(pos, r.currPkg, name, nil)
@@ -572,7 +590,7 @@
// Declare obj before calling r.tparamList, so the new type name is recognized
// if used in the constraint of one of its own typeparams (see #48280).
r.declare(obj)
- if tag == 'U' {
+ if tag == genericTypeTag {
tparams := r.tparamList()
named.SetTypeParams(tparams)
}
@@ -604,7 +622,7 @@
}
}
- case 'P':
+ case typeParamTag:
// We need to "declare" a typeparam in order to have a name that
// can be referenced recursively (if needed) in the type param's
// bound.
@@ -637,7 +655,7 @@
// completely set up all types in ImportData.
r.p.later = append(r.p.later, setConstraintArgs{t: t, constraint: constraint})
- case 'V':
+ case varTag:
typ := r.typ()
r.declare(types.NewVar(pos, r.currPkg, name, typ))
@@ -854,7 +872,7 @@
errorf("unexpected kind tag in %q: %v", r.p.ipath, k)
return nil
- case definedType:
+ case aliasType, definedType:
pkg, name := r.qualifiedIdent()
r.p.doDecl(pkg, name)
return pkg.Scope().Lookup(name).(*types.TypeName).Type()
diff --git a/internal/gcimporter/ureader_yes.go b/internal/gcimporter/ureader_yes.go
index f4edc46..b3be452 100644
--- a/internal/gcimporter/ureader_yes.go
+++ b/internal/gcimporter/ureader_yes.go
@@ -524,7 +524,7 @@
case pkgbits.ObjAlias:
pos := r.pos()
typ := r.typ()
- declare(types.NewTypeName(pos, objPkg, objName, typ))
+ declare(aliases.NewAlias(pos, objPkg, objName, typ))
case pkgbits.ObjConst:
pos := r.pos()