[dev.unified] go/internal: set underlying types in proper order

This problem appeared in google-internal testing.
If the run-later functions are run in the wrong order,
type definitions won't resolve properly.

Change-Id: I9da0775976282e92ca036d20fd9fd6650900daf9
Reviewed-on: https://go-review.googlesource.com/c/go/+/419996
Run-TryBot: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>

diff --git a/src/go/internal/gcimporter/gcimporter_test.go b/src/go/internal/gcimporter/gcimporter_test.go
index b32de1791032d70ce46589fcae0a81f276adfcaf..68a077c190bd423d45a98aff77235520d648a51b 100644 (file)
--- a/src/go/internal/gcimporter/gcimporter_test.go
+++ b/src/go/internal/gcimporter/gcimporter_test.go
@@ -583,6 +583,30 @@ func TestIssue13566(t *testing.T) {
        }
 }
 
+func TestTypeNamingOrder(t *testing.T) {
+       skipSpecialPlatforms(t)
+
+       // This package only handles gc export data.
+       if runtime.Compiler != "gc" {
+               t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
+       }
+
+       // On windows, we have to set the -D option for the compiler to avoid having a drive
+       // letter and an illegal ':' in the import path - just skip it (see also issue #3483).
+       if runtime.GOOS == "windows" {
+               t.Skip("avoid dealing with relative paths/drive letters on windows")
+       }
+
+       tmpdir := mktmpdir(t)
+       defer os.RemoveAll(tmpdir)
+       testoutdir := filepath.Join(tmpdir, "testdata")
+
+       compile(t, "testdata", "g.go", testoutdir)
+
+       // import must succeed (test for issue at hand)
+       _ = importPkg(t, "./testdata/g", tmpdir)
+}
+
 func TestIssue13898(t *testing.T) {
        skipSpecialPlatforms(t)
 

diff --git a/src/go/internal/gcimporter/testdata/g.go b/src/go/internal/gcimporter/testdata/g.go
new file mode 100644 (file)
index 0000000..301c142
--- /dev/null
+++ b/src/go/internal/gcimporter/testdata/g.go
@@ -0,0 +1,23 @@
+// Copyright 2016 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Input for TestTypeNamingOrder
+
+// ensures that the order in which "type A B" declarations are
+// processed is correct; this was a problem for unified IR imports.
+
+package g
+
+type Client struct {
+       common service
+       A      *AService
+       B      *BService
+}
+
+type service struct {
+       client *Client
+}
+
+type AService service
+type BService service

diff --git a/src/go/internal/gcimporter/ureader.go b/src/go/internal/gcimporter/ureader.go
index 5e133f890b1b3f5d2b9fda392fe9216f1542dd51..97f0664fe3c42ae8fb6030a8c43ef13fbba96c20 100644 (file)
--- a/src/go/internal/gcimporter/ureader.go
+++ b/src/go/internal/gcimporter/ureader.go
@@ -31,6 +31,8 @@ type pkgReader struct {
        // laterFns holds functions that need to be invoked at the end of
        // import reading.
        laterFns []func()
+       // laterFors is used in case of 'type A B' to ensure that B is processed before A.
+       laterFors map[types.Type]int
 }
 
 // later adds a function to be invoked at the end of import reading.
@@ -38,6 +40,15 @@ func (pr *pkgReader) later(fn func()) {
        pr.laterFns = append(pr.laterFns, fn)
 }
 
+// laterFor adds a function to be invoked at the end of import reading, and records the type that function is finishing.
+func (pr *pkgReader) laterFor(t types.Type, fn func()) {
+       if pr.laterFors == nil {
+               pr.laterFors = make(map[types.Type]int)
+       }
+       pr.laterFors[t] = len(pr.laterFns)
+       pr.laterFns = append(pr.laterFns, fn)
+}
+
 // readUnifiedPackage reads a package description from the given
 // unified IR export data decoder.
 func readUnifiedPackage(fset *token.FileSet, ctxt *types.Context, imports map[string]*types.Package, input pkgbits.PkgDecoder) *types.Package {
@@ -487,7 +498,15 @@ func (pr *pkgReader) objIdx(idx pkgbits.Index) (*types.Package, string) {
                        // unit tests expected that), but cmd/compile doesn't care
                        // about it, so maybe we can avoid worrying about that here.
                        rhs := r.typ()
-                       r.p.later(func() {
+                       pk := r.p
+                       pk.laterFor(named, func() {
+                               // First be sure that the rhs is initialized, if it needs to be initialized.
+                               delete(pk.laterFors, named) // prevent cycles
+                               if i, ok := pk.laterFors[rhs]; ok {
+                                       f := pk.laterFns[i]
+                                       pk.laterFns[i] = func() {} // function is running now, so replace it with a no-op
+                                       f()                        // initialize RHS
+                               }
                                underlying := rhs.Underlying()
                                named.SetUnderlying(underlying)
                        })