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 e1489ce..ae2b3b1 100644
--- a/src/cmd/compile/internal/typecheck/crawler.go
+++ b/src/cmd/compile/internal/typecheck/crawler.go
@@ -20,6 +20,7 @@
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 @@
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 @@
}
}
+// 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 @@
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
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
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
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
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
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
