Skip to content

Commit

Permalink
internal/gcimporter: support materialized aliases
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
adonovan committed Apr 2, 2024
1 parent 85b6527 commit 0a4fc72
Show file tree
Hide file tree
Showing 6 changed files with 79 additions and 26 deletions.
1 change: 1 addition & 0 deletions internal/gcimporter/gcimporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,7 @@ func TestImportTypeparamTests(t *testing.T) {
"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.
}
}

Expand Down
45 changes: 35 additions & 10 deletions internal/gcimporter/iexport.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"sort"
"strconv"
"strings"
"unsafe"

"golang.org/x/tools/go/types/objectpath"
"golang.org/x/tools/internal/aliases"
Expand Down Expand Up @@ -464,7 +465,7 @@ func (p *iexporter) doDecl(obj types.Object) {

switch obj := obj.(type) {
case *types.Var:
w.tag('V')
w.tag(varTag)
w.pos(obj.Pos())
w.typ(obj.Type(), obj.Pkg())

Expand All @@ -482,9 +483,9 @@ func (p *iexporter) doDecl(obj types.Object) {

// 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
Expand All @@ -500,15 +501,15 @@ func (p *iexporter) doDecl(obj types.Object) {
w.signature(sig)

case *types.Const:
w.tag('C')
w.tag(constTag)
w.pos(obj.Pos())
w.value(obj.Type(), obj.Val())

case *types.TypeName:
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 {
Expand All @@ -523,8 +524,13 @@ func (p *iexporter) doDecl(obj types.Object) {
}

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
}
Expand All @@ -536,9 +542,9 @@ func (p *iexporter) doDecl(obj types.Object) {
}

if named.TypeParams().Len() == 0 {
w.tag('T')
w.tag(typeTag)
} else {
w.tag('U')
w.tag(genericTypeTag)
}
w.pos(obj.Pos())

Expand All @@ -548,7 +554,7 @@ func (p *iexporter) doDecl(obj types.Object) {
w.tparamList(obj.Name(), named.TypeParams(), obj.Pkg())
}

underlying := obj.Type().Underlying()
underlying := named.Underlying()
w.typ(underlying, obj.Pkg())

if types.IsInterface(t) {
Expand Down Expand Up @@ -739,7 +745,10 @@ func (w *exportWriter) doTyp(t types.Type, pkg *types.Package) {
}()
}
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 {
Expand Down Expand Up @@ -1322,3 +1331,19 @@ func (e internalError) Error() string { return "gcimporter: " + string(e) }
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
}
6 changes: 6 additions & 0 deletions internal/gcimporter/iexport_go118_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,12 @@ func TestImportTypeparamTests(t *testing.T) {
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
Expand Down
7 changes: 5 additions & 2 deletions internal/gcimporter/iexport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"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"
Expand Down Expand Up @@ -145,6 +146,7 @@ type UnknownType undefined
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)
Expand Down Expand Up @@ -333,14 +335,15 @@ func cmpObj(x, y types.Object) error {
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)
}
Expand Down
44 changes: 31 additions & 13 deletions internal/gcimporter/iimport.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,20 @@ const (
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
Expand Down Expand Up @@ -324,7 +338,7 @@ func iimportCommon(fset *token.FileSet, getPackages GetPackagesFunc, data []byte
}

// 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.
Expand Down Expand Up @@ -546,33 +560,37 @@ func (r *importReader) obj(name string) {
pos := r.pos()

switch tag {
case 'A':
case aliasTag:
typ := r.typ()

r.declare(types.NewTypeName(pos, r.currPkg, name, typ))

case 'C':
// TODO(adonovan): support generic aliases:
// if tag == genericAliasTag {
// tparams := r.tparamList()
// alias.SetTypeParams(tparams)
// }
r.declare(aliases.NewAlias(pos, r.currPkg, name, typ))

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)
named := types.NewNamed(obj, nil, nil)
// 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)
}
Expand Down Expand Up @@ -604,7 +622,7 @@ func (r *importReader) obj(name string) {
}
}

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.
Expand Down Expand Up @@ -637,7 +655,7 @@ func (r *importReader) obj(name string) {
// 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))
Expand Down Expand Up @@ -854,7 +872,7 @@ func (r *importReader) doType(base *types.Named) (res types.Type) {
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()
Expand Down
2 changes: 1 addition & 1 deletion internal/gcimporter/ureader_yes.go
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,7 @@ func (pr *pkgReader) objIdx(idx pkgbits.Index) (*types.Package, string) {
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()
Expand Down

0 comments on commit 0a4fc72

Please sign in to comment.