Skip to content

Commit

Permalink
apidiff: avoid comparing instantiated aliases
Browse files Browse the repository at this point in the history
During the initial step of comparing APIs, we associate
named types in both the old and new worlds. This doesn't work
if the named type is an alias to an instantiated type, so skip
that case. The proper comparison will happen subsequently.

Fixes golang/go#67051.

Change-Id: Ia405f842e8bf5e258cbcc0748a644d2cea83117a
Reviewed-on: https://go-review.googlesource.com/c/exp/+/618617
Reviewed-by: Tim King <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
  • Loading branch information
jba committed Oct 9, 2024
1 parent 225e2ab commit f66d83c
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 8 deletions.
14 changes: 14 additions & 0 deletions apidiff/apidiff.go
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,27 @@ func (d *differ) checkPackage(oldRootPackagePath string) {
// from T to T", which can happen if a correspondence between an alias
// and a named type is established first.
// See testdata/order.go.
// Example of what this loop looks for:
// type A = B // old
// type B ... // new
//
// We want to ensure that old B and new B correspond.
//
// This loop is unnecessary for correctness; skipping symbols will not introduce bugs.
for _, name := range d.old.Scope().Names() {
oldobj := d.old.Scope().Lookup(name)
if tn, ok := oldobj.(*types.TypeName); ok {
if oldn, ok := tn.Type().(*types.Named); ok {
if !oldn.Obj().Exported() {
continue
}
// Skip aliases to instantiated generic types. They end up getting
// compared to the origin type, which will fail.
// For example, we don't want to try to make A[int] correspond with
// A[T any].
if tn.IsAlias() && isInstantiated(oldn) {
continue
}
// Does new have a named type of the same name? Look up using
// the old named type's name, oldn.Obj().Name(), not the
// TypeName tn, which may be an alias.
Expand Down
2 changes: 1 addition & 1 deletion apidiff/apidiff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ func splitIntoPackages(t *testing.T, file, dir string) (incompatibles, compatibl
if err := os.MkdirAll(filepath.Join(dir, "src", "apidiff"), 0700); err != nil {
t.Fatal(err)
}
if err := os.WriteFile(filepath.Join(dir, "src", "apidiff", "go.mod"), []byte("module apidiff\ngo 1.18\n"), 0600); err != nil {
if err := os.WriteFile(filepath.Join(dir, "src", "apidiff", "go.mod"), []byte("module apidiff\ngo 1.22\n"), 0600); err != nil {
t.Fatal(err)
}

Expand Down
16 changes: 9 additions & 7 deletions apidiff/correspondence.go
Original file line number Diff line number Diff line change
Expand Up @@ -208,17 +208,14 @@ func (d *differ) establishCorrespondence(old *types.Named, new types.Type) bool
// Two generic named types correspond if their type parameter lists correspond.
// Since one or the other of those lists will be empty, it doesn't hurt
// to check both.
oldOrigin := old.Origin()
newOrigin := newn.Origin()
if oldOrigin != old {
// old is an instantiated type.
if newOrigin == newn {
// new is not; they cannot correspond.
if isInstantiated(old) {
if !isInstantiated(new) {
// old is an instantiated type but new is not; they cannot correspond.
return false
}
// Two instantiated types correspond if their origins correspond and
// their type argument lists correspond.
if !d.correspond(oldOrigin, newOrigin) {
if !d.correspond(old.Origin(), new.Origin()) {
return false
}
if !d.typeListsCorrespond(old.TypeArgs(), newn.TypeArgs()) {
Expand Down Expand Up @@ -301,3 +298,8 @@ type ifacePair struct {
func (p *ifacePair) identical(q *ifacePair) bool {
return p.x == q.x && p.y == q.y || p.x == q.y && p.y == q.x
}

// isInstantiated reports whether t is instantiated from a generic type.
func isInstantiated(t *types.Named) bool {
return t.Origin() != t
}
5 changes: 5 additions & 0 deletions apidiff/testdata/aliases.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,8 @@ type C int
// new
// OK: merging types
type C = int

// both
// OK: identical (but this fails on 07ab4e7)
type TypedBucketRateLimiter[T comparable] struct{}
type BucketRateLimiter = TypedBucketRateLimiter[any]

0 comments on commit f66d83c

Please sign in to comment.