cmd/internal/objabi: centralize GOEXPERIMENT parsing

objabi parses GOEXPERIMENT, but most of the consumers look at the raw
GOEXPERIMENT string that objabi gets from the environment. Centralize
this logic by only exposing the parsed GOEXPERIMENT value from objabi.
This sets us up for the next few changes. It also has the nice but
mostly useless property that the order of experiment names will be
canonicalized in build cache hashes.

After this, the only remaining place that looks at raw GOEXPERIMENT is
cmd/dist, which we'll fix in the next CL.

For #40724.

Change-Id: Idb150f848e17c184fae91372ca8b361591472f51
Reviewed-on: https://go-review.googlesource.com/c/go/+/302049
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>

5 files changed, 60 insertions, 45 deletions

diff --git a/src/cmd/go/internal/cfg/cfg.go b/src/cmd/go/internal/cfg/cfg.go
index a91b6a57b9..810189c15d 100644
--- a/src/cmd/go/internal/cfg/cfg.go
+++ b/src/cmd/go/internal/cfg/cfg.go
@@ -252,13 +252,12 @@ var (
 	GOMODCACHE   = envOr("GOMODCACHE", gopathDir("pkg/mod"))
 
 	// Used in envcmd.MkEnv and build ID computations.
-	GOARM        = envOr("GOARM", fmt.Sprint(objabi.GOARM))
-	GO386        = envOr("GO386", objabi.GO386)
-	GOMIPS       = envOr("GOMIPS", objabi.GOMIPS)
-	GOMIPS64     = envOr("GOMIPS64", objabi.GOMIPS64)
-	GOPPC64      = envOr("GOPPC64", fmt.Sprintf("%s%d", "power", objabi.GOPPC64))
-	GOWASM       = envOr("GOWASM", fmt.Sprint(objabi.GOWASM))
-	GOEXPERIMENT = envOr("GOEXPERIMENT", objabi.GOEXPERIMENT)
+	GOARM    = envOr("GOARM", fmt.Sprint(objabi.GOARM))
+	GO386    = envOr("GO386", objabi.GO386)
+	GOMIPS   = envOr("GOMIPS", objabi.GOMIPS)
+	GOMIPS64 = envOr("GOMIPS64", objabi.GOMIPS64)
+	GOPPC64  = envOr("GOPPC64", fmt.Sprintf("%s%d", "power", objabi.GOPPC64))
+	GOWASM   = envOr("GOWASM", fmt.Sprint(objabi.GOWASM))
 
 	GOPROXY    = envOr("GOPROXY", "https://proxy.golang.org,direct")
 	GOSUMDB    = envOr("GOSUMDB", "sum.golang.org")
diff --git a/src/cmd/go/internal/work/exec.go b/src/cmd/go/internal/work/exec.go
index fd3d3e03bb..11b7360d26 100644
--- a/src/cmd/go/internal/work/exec.go
+++ b/src/cmd/go/internal/work/exec.go
@@ -8,7 +8,6 @@ package work
 
 import (
 	"bytes"
-	"cmd/go/internal/fsys"
 	"context"
 	"encoding/json"
 	"errors"
@@ -31,10 +30,12 @@ import (
 	"cmd/go/internal/base"
 	"cmd/go/internal/cache"
 	"cmd/go/internal/cfg"
+	"cmd/go/internal/fsys"
 	"cmd/go/internal/load"
 	"cmd/go/internal/modload"
 	"cmd/go/internal/str"
 	"cmd/go/internal/trace"
+	"cmd/internal/objabi"
 )
 
 // actionList returns the list of actions in the dag rooted at root
@@ -276,8 +277,8 @@ func (b *Builder) buildActionID(a *Action) cache.ActionID {
 		key, val := cfg.GetArchEnv()
 		fmt.Fprintf(h, "%s=%s\n", key, val)
 
-		if exp := cfg.GOEXPERIMENT; exp != "" {
-			fmt.Fprintf(h, "GOEXPERIMENT=%q\n", exp)
+		if objabi.GOEXPERIMENT != "" {
+			fmt.Fprintf(h, "GOEXPERIMENT=%q\n", objabi.GOEXPERIMENT)
 		}
 
 		// TODO(rsc): Convince compiler team not to add more magic environment variables,
@@ -1250,8 +1251,8 @@ func (b *Builder) printLinkerConfig(h io.Writer, p *load.Package) {
 		key, val := cfg.GetArchEnv()
 		fmt.Fprintf(h, "%s=%s\n", key, val)
 
-		if exp := cfg.GOEXPERIMENT; exp != "" {
-			fmt.Fprintf(h, "GOEXPERIMENT=%q\n", exp)
+		if objabi.GOEXPERIMENT != "" {
+			fmt.Fprintf(h, "GOEXPERIMENT=%q\n", objabi.GOEXPERIMENT)
 		}
 
 		// The linker writes source file paths that say GOROOT_FINAL, but
diff --git a/src/cmd/go/internal/work/init.go b/src/cmd/go/internal/work/init.go
index ba7c7c2fbb..30c9a2b7cc 100644
--- a/src/cmd/go/internal/work/init.go
+++ b/src/cmd/go/internal/work/init.go
@@ -52,14 +52,9 @@ func BuildInit() {
 	// build tag with the same name but prefixed by "goexperiment." which can be
 	// used for compiling alternative files for the experiment. This allows
 	// changes for the experiment, like extra struct fields in the runtime,
-	// without affecting the base non-experiment code at all. [2:] strips the
-	// leading "X:" from objabi.Expstring().
-	exp := objabi.Expstring()[2:]
-	if exp != "none" {
-		experiments := strings.Split(exp, ",")
-		for _, expt := range experiments {
-			cfg.BuildContext.BuildTags = append(cfg.BuildContext.BuildTags, "goexperiment."+expt)
-		}
+	// without affecting the base non-experiment code at all.
+	for _, expt := range strings.Split(objabi.GOEXPERIMENT, ",") {
+		cfg.BuildContext.BuildTags = append(cfg.BuildContext.BuildTags, "goexperiment."+expt)
 	}
 }
 
diff --git a/src/cmd/internal/objabi/flag.go b/src/cmd/internal/objabi/flag.go
index 3fd73f3c57..25b0185f64 100644
--- a/src/cmd/internal/objabi/flag.go
+++ b/src/cmd/internal/objabi/flag.go
@@ -95,7 +95,7 @@ func (versionFlag) Set(s string) error {
 	// to distinguish go1.10.2 with an experiment
 	// from go1.10.2 without an experiment.
 	p := Expstring()
-	if p == DefaultExpstring() {
+	if p == defaultExpstring {
 		p = ""
 	}
 	sep := ""
diff --git a/src/cmd/internal/objabi/util.go b/src/cmd/internal/objabi/util.go
index de8e6de4e6..81e2b28600 100644
--- a/src/cmd/internal/objabi/util.go
+++ b/src/cmd/internal/objabi/util.go
@@ -21,18 +21,23 @@ func envOr(key, value string) string {
 var (
 	defaultGOROOT string // set by linker
 
-	GOROOT       = envOr("GOROOT", defaultGOROOT)
-	GOARCH       = envOr("GOARCH", defaultGOARCH)
-	GOOS         = envOr("GOOS", defaultGOOS)
-	GOEXPERIMENT = envOr("GOEXPERIMENT", defaultGOEXPERIMENT)
-	GO386        = envOr("GO386", defaultGO386)
-	GOARM        = goarm()
-	GOMIPS       = gomips()
-	GOMIPS64     = gomips64()
-	GOPPC64      = goppc64()
-	GOWASM       = gowasm()
-	GO_LDSO      = defaultGO_LDSO
-	Version      = version
+	GOROOT   = envOr("GOROOT", defaultGOROOT)
+	GOARCH   = envOr("GOARCH", defaultGOARCH)
+	GOOS     = envOr("GOOS", defaultGOOS)
+	GO386    = envOr("GO386", defaultGO386)
+	GOARM    = goarm()
+	GOMIPS   = gomips()
+	GOMIPS64 = gomips64()
+	GOPPC64  = goppc64()
+	GOWASM   = gowasm()
+	GO_LDSO  = defaultGO_LDSO
+	Version  = version
+
+	// GOEXPERIMENT is a comma-separated list of enabled
+	// experiments. This is derived from the GOEXPERIMENT
+	// environment variable if set, or the value of GOEXPERIMENT
+	// when make.bash was run if not.
+	GOEXPERIMENT string // Set by package init
 )
 
 const (
@@ -125,7 +130,12 @@ func Getgoextlinkenabled() string {
 }
 
 func init() {
-	for _, f := range strings.Split(GOEXPERIMENT, ",") {
+	// Capture "default" experiments.
+	defaultExpstring = Expstring()
+
+	goexperiment := envOr("GOEXPERIMENT", defaultGOEXPERIMENT)
+
+	for _, f := range strings.Split(goexperiment, ",") {
 		if f != "" {
 			addexp(f)
 		}
@@ -135,6 +145,9 @@ func init() {
 	if GOARCH != "amd64" {
 		Regabi_enabled = 0
 	}
+
+	// Set GOEXPERIMENT to the parsed and canonicalized set of experiments.
+	GOEXPERIMENT = expList()
 }
 
 // Note: must agree with runtime.framepointer_enabled.
@@ -171,7 +184,6 @@ var (
 // Toolchain experiments.
 // These are controlled by the GOEXPERIMENT environment
 // variable recorded when the toolchain is built.
-// This list is also known to cmd/gc.
 var exper = []struct {
 	name string
 	val  *int
@@ -182,21 +194,29 @@ var exper = []struct {
 	{"regabi", &Regabi_enabled},
 }
 
-var defaultExpstring = Expstring()
-
-func DefaultExpstring() string {
-	return defaultExpstring
-}
+var defaultExpstring string
 
-func Expstring() string {
-	buf := "X"
+// expList returns the list of enabled GOEXPERIMENTS as a
+// commas-separated list.
+func expList() string {
+	buf := ""
 	for i := range exper {
 		if *exper[i].val != 0 {
 			buf += "," + exper[i].name
 		}
 	}
-	if buf == "X" {
-		buf += ",none"
+	if len(buf) == 0 {
+		return ""
+	}
+	return buf[1:]
+}
+
+// Expstring returns the GOEXPERIMENT string that should appear in Go
+// version signatures. This always starts with "X:".
+func Expstring() string {
+	list := expList()
+	if list == "" {
+		return "X:none"
 	}
-	return "X:" + buf[2:]
+	return "X:" + list
 }