cmd/internal/objabi: make GOEXPERIMENT be a diff from default experiments

Right now the rules around handling default-on experiments are
complicated and a bit inconsistent. Notably, objabi.GOEXPERIMENT is
set to a comma-separated list of enabled experiments, but this may not
be the string a user should set the GOEXPERIMENT environment variable
to get that list of experiments: if an experiment is enabled by
default but gets turned off by GOEXPERIMENT, then the string we report
needs to include "no"+experiment to capture that default override.

This complication also seeps into the version string we print for "go
tool compile -V", etc. This logic is further complicated by the fact
that it only wants to include an experiment string if the set of
experiments varies from the default.

This CL rethinks how we handle default-on experiments. Now that
experiment state is all captured in a struct, we can simplify a lot of
this logic. objabi.GOEXPERIMENT will be set based on the delta from
the default set of experiments, which reflects what a user would
actually need to pass on the command line. Likewise, we include this
delta in the "-V" output, which simplifies this logic because if
there's nothing to show in the version string, the delta will be
empty.

Change-Id: I7ed307329541fc2c9f90edd463fbaf8e0cc9e8ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/307819
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>

src/cmd/go/script_test.go

@@ -165,7 +165,7 @@ func (ts *testScript) setup() {
 		"GOCACHE=" + testGOCACHE,
 		"GODEBUG=" + os.Getenv("GODEBUG"),
 		"GOEXE=" + cfg.ExeSuffix,
-		"GOEXPSTRING=" + objabi.Expstring()[2:],
+		"GOEXPERIMENT=" + objabi.GOEXPERIMENT(),
 		"GOOS=" + runtime.GOOS,
 		"GOPATH=" + filepath.Join(ts.workdir, "gopath"),
 		"GOPROXY=" + proxyURL,

src/cmd/go/testdata/script/README

@@ -29,7 +29,7 @@ Scripts also have access to these other environment variables:
 	GOARCH=<target GOARCH>
 	GOCACHE=<actual GOCACHE being used outside the test>
 	GOEXE=<executable file suffix: .exe on Windows, empty on other systems>
-	GOEXPSTRING=<value of objabi.Expstring(), from GOEXPERIMENT when toolchain built>
+	GOEXPERIMENT=<value of objabi.GOEXPERIMENT>
 	GOOS=<target GOOS>
 	GOPATH=$WORK/gopath
 	GOPROXY=<local module proxy serving from cmd/go/testdata/mod>

src/cmd/go/testdata/script/build_tag_goexperiment.txt

@@ -1,6 +1,6 @@
 # compile_ext will fail if the buildtags that are enabled (or not enabled) for the
 # framepointer and fieldtrack experiments are not consistent with the value of
-# GOEXPSTRING (which comes from objabi.Expstring()).
+# objabi.GOEXPERIMENT.
 
 [short] skip
 go run m
@@ -19,10 +19,8 @@ func main() {
 }
 
 func hasExpEntry(s string) bool {
-	// script_test.go defines GOEXPSTRING to be the value of
-	// objabi.Expstring(), which gives the enabled experiments baked into the
-	// toolchain.
-	g := os.Getenv("GOEXPSTRING")
+	// script_test.go defines GOEXPERIMENT to be the enabled experiments.
+	g := os.Getenv("GOEXPERIMENT")
 	for _, f := range strings.Split(g, ",") {
 		if f == s {
 			return true
@@ -43,7 +41,7 @@ import (
 
 func fp() {
 	if hasExpEntry("framepointer") {
-		fmt.Println("in !framepointer build, but objabi.Expstring() has 'framepointer'")
+		fmt.Println("in !framepointer build, but objabi.GOEXPERIMENT has 'framepointer'")
 		os.Exit(1)
 	}
 }
@@ -60,7 +58,7 @@ import (
 
 func fp() {
 	if !hasExpEntry("framepointer") {
-		fmt.Println("in framepointer build, but objabi.Expstring() does not have 'framepointer', is", os.Getenv("GOEXPSTRING"))
+		fmt.Println("in framepointer build, but objabi.GOEXPERIMENT does not have 'framepointer', is", os.Getenv("GOEXPERIMENT"))
 		os.Exit(1)
 	}
 }
@@ -77,7 +75,7 @@ import (
 
 func ft() {
 	if hasExpEntry("fieldtrack") {
-		fmt.Println("in !fieldtrack build, but objabi.Expstring() has 'fieldtrack'")
+		fmt.Println("in !fieldtrack build, but objabi.GOEXPERIMENT has 'fieldtrack'")
 		os.Exit(1)
 	}
 }
@@ -94,7 +92,7 @@ import (
 
 func ft() {
 	if !hasExpEntry("fieldtrack") {
-		fmt.Println("in fieldtrack build, but objabi.Expstring() does not have 'fieldtrack', is", os.Getenv("GOEXPSTRING"))
+		fmt.Println("in fieldtrack build, but objabi.GOEXPERIMENT does not have 'fieldtrack', is", os.Getenv("GOEXPERIMENT"))
 		os.Exit(1)
 	}
 }

src/cmd/internal/objabi/exp.go

@@ -20,8 +20,6 @@ import (
 // was built with.)
 var Experiment goexperiment.Flags
 
-var defaultExpstring string // Set by package init
-
 // FramePointerEnabled enables the use of platform conventions for
 // saving frame pointers.
 //
@@ -32,13 +30,15 @@ var defaultExpstring string // Set by package init
 var FramePointerEnabled = GOARCH == "amd64" || GOARCH == "arm64"
 
 func init() {
-	// Capture "default" experiments.
-	defaultExpstring = Expstring()
+	// Start with the baseline configuration.
+	Experiment = goexperiment.BaselineFlags
 
-	goexperiment := envOr("GOEXPERIMENT", defaultGOEXPERIMENT)
+	// Pick up any changes to the baseline configuration from the
+	// GOEXPERIMENT environment. This can be set at make.bash time
+	// and overridden at build time.
+	env := envOr("GOEXPERIMENT", defaultGOEXPERIMENT)
 
-	// GOEXPERIMENT=none overrides all experiments enabled at dist time.
-	if goexperiment != "none" {
+	if env != "" {
 		// Create a map of known experiment names.
 		names := make(map[string]reflect.Value)
 		rv := reflect.ValueOf(&Experiment).Elem()
@@ -49,10 +49,16 @@ func init() {
 		}
 
 		// Parse names.
-		for _, f := range strings.Split(goexperiment, ",") {
+		for _, f := range strings.Split(env, ",") {
 			if f == "" {
 				continue
 			}
+			if f == "none" {
+				// GOEXPERIMENT=none restores the baseline configuration.
+				// (This is useful for overriding make.bash-time settings.)
+				Experiment = goexperiment.BaselineFlags
+				continue
+			}
 			val := true
 			if strings.HasPrefix(f, "no") {
 				f, val = f[2:], false
@@ -93,40 +99,45 @@ func init() {
 	}
 }
 
-// expList returns the list of enabled GOEXPERIMENTs names.
-func expList(flags *goexperiment.Flags) []string {
+// expList returns the list of lower-cased experiment names for
+// experiments that differ from base. base may be nil to indicate no
+// experiments.
+func expList(exp, base *goexperiment.Flags) []string {
 	var list []string
-	rv := reflect.ValueOf(&Experiment).Elem()
+	rv := reflect.ValueOf(exp).Elem()
+	var rBase reflect.Value
+	if base != nil {
+		rBase = reflect.ValueOf(base).Elem()
+	}
 	rt := rv.Type()
 	for i := 0; i < rt.NumField(); i++ {
+		name := strings.ToLower(rt.Field(i).Name)
 		val := rv.Field(i).Bool()
-		if val {
-			field := rt.Field(i)
-			list = append(list, strings.ToLower(field.Name))
+		baseVal := false
+		if base != nil {
+			baseVal = rBase.Field(i).Bool()
+		}
+		if val != baseVal {
+			if val {
+				list = append(list, name)
+			} else {
+				list = append(list, "no"+name)
+			}
 		}
 	}
 	return list
 }
 
-// Expstring returns the GOEXPERIMENT string that should appear in Go
-// version signatures. This always starts with "X:".
-func Expstring() string {
-	list := expList(&Experiment)
-	if len(list) == 0 {
-		return "X:none"
-	}
-	return "X:" + strings.Join(list, ",")
-}
-
-// GOEXPERIMENT returns 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 is a comma-separated list of enabled or disabled
+// experiments that differ from the baseline experiment configuration.
+// GOEXPERIMENT is exactly what a user would set on the command line
+// to get the set of enabled experiments.
 func GOEXPERIMENT() string {
-	return strings.Join(expList(&Experiment), ",")
+	return strings.Join(expList(&Experiment, &goexperiment.BaselineFlags), ",")
 }
 
 // EnabledExperiments returns a list of enabled experiments, as
 // lower-cased experiment names.
 func EnabledExperiments() []string {
-	return expList(&Experiment)
+	return expList(&Experiment, nil)
 }

src/cmd/internal/objabi/flag.go

@@ -91,16 +91,12 @@ func (versionFlag) Set(s string) error {
 	name = name[strings.LastIndex(name, `\`)+1:]
 	name = strings.TrimSuffix(name, ".exe")
 
-	// If there's an active experiment, include that,
-	// to distinguish go1.10.2 with an experiment
-	// from go1.10.2 without an experiment.
-	p := Expstring()
-	if p == defaultExpstring {
-		p = ""
-	}
-	sep := ""
-	if p != "" {
-		sep = " "
+	p := ""
+
+	// If the enabled experiments differ from the defaults,
+	// include that difference.
+	if goexperiment := GOEXPERIMENT(); goexperiment != "" {
+		p = " X:" + goexperiment
 	}
 
 	// The go command invokes -V=full to get a unique identifier
@@ -114,7 +110,7 @@ func (versionFlag) Set(s string) error {
 		}
 	}
 
-	fmt.Printf("%s version %s%s%s\n", name, Version, sep, p)
+	fmt.Printf("%s version %s%s\n", name, Version, p)
 	os.Exit(0)
 	return nil
 }

src/cmd/internal/objabi/util.go

@@ -128,5 +128,5 @@ func Getgoextlinkenabled() string {
 // or link object files that are incompatible with each other. This
 // string always starts with "go object ".
 func HeaderString() string {
-	return fmt.Sprintf("go object %s %s %s %s\n", GOOS, GOARCH, Version, Expstring())
+	return fmt.Sprintf("go object %s %s %s X:%s\n", GOOS, GOARCH, Version, strings.Join(EnabledExperiments(), ","))
 }

src/internal/goexperiment/flags.go

@@ -75,3 +75,8 @@ type Flags struct {
 	// register arguments to defer/go).
 	RegabiArgs bool
 }
+
+// BaselineFlags specifies the experiment flags that are enabled by
+// default in the current toolchain. This is, in effect, the "control"
+// configuration and any variation from this is an experiment.
+var BaselineFlags = Flags{}