cmd/compile, test: updated comments in crawler.go, added test

Added a test to make sure that the private methods of a local generic
type are properly exported, if there is a global variable with that
type.

Added comments in crawler.go, to give more detail and to give more about
the overall purpose.

Fixed one place where t.isFullyInstantiated() should be replaced by
isPtrFullyInstantiated(t), so that we catch pointers to generic types
that may be used as a method receiver.

Change-Id: I9c42d14eb6ebe14d249df7c8fa39e889f7cd3f22
Reviewed-on: https://go-review.googlesource.com/c/go/+/374754
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Trust: Dan Scales <danscales@google.com>
Run-TryBot: Dan Scales <danscales@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

1 files changed, 29 insertions, 9 deletions

diff --git a/src/cmd/compile/internal/typecheck/crawler.go b/src/cmd/compile/internal/typecheck/crawler.go
index 5a9649e7a1..cdb1c46509 100644
--- a/src/cmd/compile/internal/typecheck/crawler.go
+++ b/src/cmd/compile/internal/typecheck/crawler.go
@@ -11,11 +11,22 @@ import (
 )
 
 // crawlExports crawls the type/object graph rooted at the given list of exported
-// objects. It descends through all parts of types and follows any methods on defined
-// types. Any functions that are found to be potentially callable by importers are
-// marked with ExportInline, so that iexport.go knows to re-export their inline body.
-// Also, any function or global referenced by a function marked by ExportInline() is
-// marked for export (whether its name is exported or not).
+// objects (which are variables, functions, and types). It descends through all parts
+// of types and follows methods on defined types. Any functions that are found to be
+// potentially callable by importers directly or after inlining are marked with
+// ExportInline, so that iexport.go knows to export their inline body.
+//
+// The overall purpose of crawlExports is to AVOID exporting inlineable methods
+// that cannot actually be referenced, thereby reducing the size of the exports
+// significantly.
+//
+// For non-generic defined types reachable from global variables, we only set
+// ExportInline for exported methods. For defined types that are directly named or are
+// embedded recursively in such a type, we set ExportInline for all methods, since
+// these types can be embedded in another local type. For instantiated types that are
+// used anywhere in a inlineable function, we set ExportInline on all methods of the
+// base generic type, since all methods will be needed for creating any instantiated
+// type.
 func crawlExports(exports []*ir.Name) {
 	p := crawler{
 		marked:   make(map[*types.Type]bool),
@@ -170,10 +181,12 @@ 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.
+// 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 out for
+// export, even if not explicitly marked for export, then all of its methods need to
+// be available for instantiation, since we always create all methods of a specified
+// instantiated type. Non-exported methods must generally be instantiated, since they may
+// be called by the exported methods or other generic function in the same package.
 func (p *crawler) markGeneric(t *types.Type) {
 	if t.IsPtr() {
 		t = t.Elem()
@@ -222,6 +235,9 @@ func (p *crawler) markInlBody(n *ir.Name) {
 	doFlood = func(n ir.Node) {
 		t := n.Type()
 		if t != nil {
+			if t.IsPtr() {
+				t = t.Elem()
+			}
 			if t.IsFullyInstantiated() && !t.HasShape() && !t.IsInterface() && t.Methods().Len() > 0 {
 				// For any fully-instantiated type, the relevant
 				// dictionaries and shape instantiations will have
@@ -287,6 +303,10 @@ func (p *crawler) markInlBody(n *ir.Name) {
 			switch n.Class {
 			case ir.PFUNC:
 				p.markInlBody(n)
+				// Note: this Export() and the one below seem unneeded,
+				// since any function/extern name encountered in an
+				// exported function body will be exported
+				// automatically via qualifiedIdent() in iexport.go.
 				Export(n)
 			case ir.PEXTERN:
 				Export(n)