]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/internal/objabi: make GOEXPERIMENT be a diff from default experiments
authorAustin Clements <austin@google.com>
Tue, 6 Apr 2021 20:11:17 +0000 (16:11 -0400)
committerAustin Clements <austin@google.com>
Thu, 8 Apr 2021 02:17:20 +0000 (02:17 +0000)
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
src/cmd/go/testdata/script/README
src/cmd/go/testdata/script/build_tag_goexperiment.txt
src/cmd/internal/objabi/exp.go
src/cmd/internal/objabi/flag.go
src/cmd/internal/objabi/util.go
src/internal/goexperiment/flags.go

index 1f38be8ee44c3135322eee863119eac41408c490..c353a9cb015784b4a0aa5541d8366f79a4f74e06 100644 (file)
@@ -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,
index d658cebfce0170e8fcd2956b2bd2e11d74a2555c..b4dcb1f5a2e404ef4786c3012fe9a1b5279bdec5 100644 (file)
@@ -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>
index 26ad029845ee6edf58e459725e60d98303b4b696..dfda3d2629d55cd088f6070741c1b3ad406387f7 100644 (file)
@@ -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)
        }
 }
index 21a70d5dfece46a5fcdfebb50791f79e5e477f1b..eaa86208077af20fe48245e51e3e3108325234c0 100644 (file)
@@ -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)
 }
index 25b0185f645a51f5f487340b5b8c04f1fd379918..6a8a69116df141e0e6d0abc01e90c6bfc601cf53 100644 (file)
@@ -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
 }
index c2c05bd1b270fd6ce93966e76d5bfab8986772ce..5a7a74cfde8d0441ddc839421aeb8c487cdfc3a7 100644 (file)
@@ -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(), ","))
 }
index c5e3d2c91ac9c4771de9c18e2767c0a0b350af6e..1c513d5a709a8346d8b6be944d9af4207e85806d 100644 (file)
@@ -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{}