cmd/compile: fix generic type handling when crawling inline body

For base generic type that is written to export file, we need to mark
all of its methods, include exported+unexported methods, as reachable,
so they can be available for instantiation if necessary. But markType
only looks for exported methods, thus causing the crash in #49143.

To fix this, we introduce new method p.markGeneric, to mark all methods
of the base generic type.

This issue has happend for a while (maybe since we add generic
import/export during go1.18 cycle), and was un-intentionally "fixed" in
CL 356254, when we agresssively call p.markEmbed(t). CL 357232 fixed
that wrong agressive behavior, thus reproduce the bug on tip.

Fixes #49143

Change-Id: Ie64574a05fffb282e9dcc8739df4378c5b6b0468
Reviewed-on: https://go-review.googlesource.com/c/go/+/358814
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>

diff --git a/src/cmd/compile/internal/typecheck/crawler.go b/src/cmd/compile/internal/typecheck/crawler.go
index e1489ceeddd840f25924a5aef4e8f0ee725b0b0e..ae2b3b1df4e1ca8d95692533071e71128cf6d325 100644 (file)
--- a/src/cmd/compile/internal/typecheck/crawler.go
+++ b/src/cmd/compile/internal/typecheck/crawler.go
@@ -20,6 +20,7 @@ func crawlExports(exports []*ir.Name) {
        p := crawler{
                marked:   make(map[*types.Type]bool),
                embedded: make(map[*types.Type]bool),
+               generic:  make(map[*types.Type]bool),
        }
        for _, n := range exports {
                p.markObject(n)
@@ -29,6 +30,7 @@ func crawlExports(exports []*ir.Name) {
 type crawler struct {
        marked   map[*types.Type]bool // types already seen by markType
        embedded map[*types.Type]bool // types already seen by markEmbed
+       generic  map[*types.Type]bool // types already seen by markGeneric
 }
 
 // markObject visits a reachable object (function, method, global type, or global variable)
@@ -168,6 +170,30 @@ func (p *crawler) markEmbed(t *types.Type) {
        }
 }
 
+// markGeneric takes an instantiated type or a base generic type t, and
+// marks all the methods of the base generic type of t. If a base generic
+// type is written to export file, even if not explicitly marked for export,
+// all of its methods need to be available for instantiation if needed.
+func (p *crawler) markGeneric(t *types.Type) {
+       if t.IsPtr() {
+               t = t.Elem()
+       }
+       if t.OrigSym() != nil {
+               // Convert to the base generic type.
+               t = t.OrigSym().Def.Type()
+       }
+       if p.generic[t] {
+               return
+       }
+       p.generic[t] = true
+
+       if t.Sym() != nil && t.Kind() != types.TINTER {
+               for _, m := range t.Methods().Slice() {
+                       p.markObject(m.Nname.(*ir.Name))
+               }
+       }
+}
+
 // markInlBody marks n's inline body for export and recursively
 // ensures all called functions are marked too.
 func (p *crawler) markInlBody(n *ir.Name) {
@@ -197,12 +223,7 @@ func (p *crawler) markInlBody(n *ir.Name) {
                t := n.Type()
                if t != nil {
                        if t.HasTParam() || t.IsFullyInstantiated() {
-                               // Ensure that we call markType() on any base generic type
-                               // that is written to the export file (even if not explicitly
-                               // marked for export), so we will call markInlBody on its
-                               // methods, and the methods will be available for
-                               // instantiation if needed.
-                               p.markType(t)
+                               p.markGeneric(t)
                        }
                        if base.Debug.Unified == 0 {
                                // If a method of un-exported type is promoted and accessible by

diff --git a/test/fixedbugs/issue49143.dir/a.go b/test/fixedbugs/issue49143.dir/a.go
new file mode 100644 (file)
index 0000000..5aefcd8
--- /dev/null
+++ b/test/fixedbugs/issue49143.dir/a.go
@@ -0,0 +1,24 @@
+// Copyright 2021 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.
+
+package a
+
+import "sync"
+
+type Loader[K comparable, R any] struct {
+       batch *LoaderBatch[K, R]
+}
+
+func (l *Loader[K, R]) Load() error {
+       l.batch.f()
+       return nil
+}
+
+type LoaderBatch[K comparable, R any] struct {
+       once    *sync.Once
+}
+
+func (b *LoaderBatch[K, R]) f() {
+       b.once.Do(func() {})
+}

diff --git a/test/fixedbugs/issue49143.dir/b.go b/test/fixedbugs/issue49143.dir/b.go
new file mode 100644 (file)
index 0000000..48eecdb
--- /dev/null
+++ b/test/fixedbugs/issue49143.dir/b.go
@@ -0,0 +1,16 @@
+// Copyright 2021 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.
+
+package b
+
+import "./a"
+
+
+type Loaders struct {
+       Loader *a.Loader[int, int]
+}
+
+func NewLoaders() *Loaders {
+       return new(Loaders)
+}

diff --git a/test/fixedbugs/issue49143.dir/c.go b/test/fixedbugs/issue49143.dir/c.go
new file mode 100644 (file)
index 0000000..89262e3
--- /dev/null
+++ b/test/fixedbugs/issue49143.dir/c.go
@@ -0,0 +1,15 @@
+// Copyright 2021 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.
+
+package c
+
+import "./b"
+
+type Resolver struct{}
+
+type todoResolver struct{ *Resolver }
+
+func (r *todoResolver) F() {
+       b.NewLoaders().Loader.Load()
+}

diff --git a/test/fixedbugs/issue49143.dir/p.go b/test/fixedbugs/issue49143.dir/p.go
new file mode 100644 (file)
index 0000000..f11d2f2
--- /dev/null
+++ b/test/fixedbugs/issue49143.dir/p.go
@@ -0,0 +1,11 @@
+// Copyright 2021 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.
+
+package p
+
+import (
+       "./c"
+)
+
+var _ = &c.Resolver{}

diff --git a/test/fixedbugs/issue49143.go b/test/fixedbugs/issue49143.go
new file mode 100644 (file)
index 0000000..87b4ff4
--- /dev/null
+++ b/test/fixedbugs/issue49143.go
@@ -0,0 +1,7 @@
+// compiledir -G=3
+
+// Copyright 2021 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.
+
+package ignored