From 0a4fc723d799e9c9f5686fe08b9a3af68c09a0fd Mon Sep 17 00:00:00 2001 From: Alan Donovan Date: Tue, 19 Mar 2024 15:51:16 -0400 Subject: [PATCH] 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 Reviewed-by: Robert Findley --- internal/gcimporter/gcimporter_test.go | 1 + internal/gcimporter/iexport.go | 45 ++++++++++++++++++----- internal/gcimporter/iexport_go118_test.go | 6 +++ internal/gcimporter/iexport_test.go | 7 +++- internal/gcimporter/iimport.go | 44 +++++++++++++++------- internal/gcimporter/ureader_yes.go | 2 +- 6 files changed, 79 insertions(+), 26 deletions(-) diff --git a/internal/gcimporter/gcimporter_test.go b/internal/gcimporter/gcimporter_test.go index 95cc36c4d96..b93cd2a7c7f 100644 --- a/internal/gcimporter/gcimporter_test.go +++ b/internal/gcimporter/gcimporter_test.go @@ -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. } } diff --git a/internal/gcimporter/iexport.go b/internal/gcimporter/iexport.go index 638fc1d3b86..683bd7395a6 100644 --- a/internal/gcimporter/iexport.go +++ b/internal/gcimporter/iexport.go @@ -21,6 +21,7 @@ import ( "sort" "strconv" "strings" + "unsafe" "golang.org/x/tools/go/types/objectpath" "golang.org/x/tools/internal/aliases" @@ -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()) @@ -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 @@ -500,7 +501,7 @@ 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()) @@ -508,7 +509,7 @@ func (p *iexporter) doDecl(obj types.Object) { 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 @@ 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 } @@ -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()) @@ -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) { @@ -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 { @@ -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 +} diff --git a/internal/gcimporter/iexport_go118_test.go b/internal/gcimporter/iexport_go118_test.go index c748fb36165..3aed235a7f8 100644 --- a/internal/gcimporter/iexport_go118_test.go +++ b/internal/gcimporter/iexport_go118_test.go @@ -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 diff --git a/internal/gcimporter/iexport_test.go b/internal/gcimporter/iexport_test.go index 4ee79dac9d0..0da2599d531 100644 --- a/internal/gcimporter/iexport_test.go +++ b/internal/gcimporter/iexport_test.go @@ -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" @@ -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) @@ -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) } diff --git a/internal/gcimporter/iimport.go b/internal/gcimporter/iimport.go index 4d50eb8e587..2732121b5ef 100644 --- a/internal/gcimporter/iimport.go +++ b/internal/gcimporter/iimport.go @@ -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 @@ -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. @@ -546,25 +560,29 @@ 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) @@ -572,7 +590,7 @@ func (r *importReader) obj(name string) { // 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 @@ 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. @@ -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)) @@ -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() diff --git a/internal/gcimporter/ureader_yes.go b/internal/gcimporter/ureader_yes.go index f4edc46ab74..b3be452ae8a 100644 --- a/internal/gcimporter/ureader_yes.go +++ b/internal/gcimporter/ureader_yes.go @@ -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()