From ca5f65d771bc24b9717dca615fa4ad25dcd94fad Mon Sep 17 00:00:00 2001 From: Cuong Manh Le Date: Mon, 25 Oct 2021 14:16:53 +0700 Subject: [PATCH] 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 Trust: Dan Scales Run-TryBot: Cuong Manh Le TryBot-Result: Go Bot Reviewed-by: Keith Randall Reviewed-by: Dan Scales --- src/cmd/compile/internal/typecheck/crawler.go | 33 +++++++++++++++---- test/fixedbugs/issue49143.dir/a.go | 24 ++++++++++++++ test/fixedbugs/issue49143.dir/b.go | 16 +++++++++ test/fixedbugs/issue49143.dir/c.go | 15 +++++++++ test/fixedbugs/issue49143.dir/p.go | 11 +++++++ test/fixedbugs/issue49143.go | 7 ++++ 6 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 test/fixedbugs/issue49143.dir/a.go create mode 100644 test/fixedbugs/issue49143.dir/b.go create mode 100644 test/fixedbugs/issue49143.dir/c.go create mode 100644 test/fixedbugs/issue49143.dir/p.go create mode 100644 test/fixedbugs/issue49143.go diff --git a/src/cmd/compile/internal/typecheck/crawler.go b/src/cmd/compile/internal/typecheck/crawler.go index e1489ceedd..ae2b3b1df4 100644 --- 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 index 0000000000..5aefcd8780 --- /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 0000000000..48eecdbaaf --- /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 0000000000..89262e374a --- /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 0000000000..f11d2f22eb --- /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 0000000000..87b4ff46c1 --- /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 -- 2.50.0