From 14dbd6e7762662f8bec95d537281f09ad3d508e4 Mon Sep 17 00:00:00 2001
From: Austin Clements <austin@google.com>
Date: Thu, 15 Apr 2021 16:18:30 -0400
Subject: [PATCH] internal/buildcfg: make regabi an alias for regabi
 sub-experiments

Currently, specifying GOEXPERIMENT=regabi will turn on all regabi
sub-experiments, but GOEXPERIMENT=noregabi won't turn anything off.
Regabi also isn't a "real" experiment in the sense that nothing in the
code base should depend on it as an experiment flag (it should depend
on the appropriate sub-experiments).

Hence, drop Regabi from goexperiment.Flags and make "regabi" in
GOEXPERIMENT be a real alias for all of the sub-flags, so regabi will
turn on all of the sub-flags and noregabi will turn off all of the
sub-flags.

This way, once we enable the sub-experiments in the baseline
configuration, it will be easy to turn off with "noregabi".

For #40724.

Change-Id: I0fb95be42f756d412e729a396be607d629ae2bab
Reviewed-on: https://go-review.googlesource.com/c/go/+/310609
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
---
 src/internal/buildcfg/exp.go       | 31 ++++++++++++++++--------------
 src/internal/goexperiment/flags.go |  7 ++++---
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/src/internal/buildcfg/exp.go b/src/internal/buildcfg/exp.go
index dc0adb09631..6eaf2bd7c25 100644
--- a/src/internal/buildcfg/exp.go
+++ b/src/internal/buildcfg/exp.go
@@ -45,12 +45,25 @@ func parseExperiments() goexperiment.Flags {
 
 	if env != "" {
 		// Create a map of known experiment names.
-		names := make(map[string]reflect.Value)
+		names := make(map[string]func(bool))
 		rv := reflect.ValueOf(&flags).Elem()
 		rt := rv.Type()
 		for i := 0; i < rt.NumField(); i++ {
 			field := rv.Field(i)
-			names[strings.ToLower(rt.Field(i).Name)] = field
+			names[strings.ToLower(rt.Field(i).Name)] = field.SetBool
+		}
+
+		// "regabi" is an alias for all working regabi
+		// subexperiments, and not an experiment itself. Doing
+		// this as an alias make both "regabi" and "noregabi"
+		// do the right thing.
+		names["regabi"] = func(v bool) {
+			flags.RegabiWrappers = v
+			flags.RegabiG = v
+			flags.RegabiReflect = v
+			flags.RegabiDefer = v
+			// Not ready yet:
+			//flags.RegabiArgs = v
 		}
 
 		// Parse names.
@@ -69,33 +82,23 @@ func parseExperiments() goexperiment.Flags {
 			if strings.HasPrefix(f, "no") {
 				f, val = f[2:], false
 			}
-			field, ok := names[f]
+			set, ok := names[f]
 			if !ok {
 				fmt.Printf("unknown experiment %s\n", f)
 				os.Exit(2)
 			}
-			field.SetBool(val)
+			set(val)
 		}
 	}
 
 	// regabi is only supported on amd64.
 	if GOARCH != "amd64" {
-		flags.Regabi = false
 		flags.RegabiWrappers = false
 		flags.RegabiG = false
 		flags.RegabiReflect = false
 		flags.RegabiDefer = false
 		flags.RegabiArgs = false
 	}
-	// Setting regabi sets working sub-experiments.
-	if flags.Regabi {
-		flags.RegabiWrappers = true
-		flags.RegabiG = true
-		flags.RegabiReflect = true
-		flags.RegabiDefer = true
-		// Not ready yet:
-		//flags.RegabiArgs = true
-	}
 	// Check regabi dependencies.
 	if flags.RegabiG && !flags.RegabiWrappers {
 		panic("GOEXPERIMENT regabig requires regabiwrappers")
diff --git a/src/internal/goexperiment/flags.go b/src/internal/goexperiment/flags.go
index e77734caa4c..cd4c1788184 100644
--- a/src/internal/goexperiment/flags.go
+++ b/src/internal/goexperiment/flags.go
@@ -60,9 +60,10 @@ type Flags struct {
 	StaticLockRanking bool
 
 	// Regabi is split into several sub-experiments that can be
-	// enabled individually. GOEXPERIMENT=regabi implies the
-	// subset that are currently "working". Not all combinations work.
-	Regabi bool
+	// enabled individually. Not all combinations work.
+	// The "regabi" GOEXPERIMENT is an alias for all "working"
+	// subexperiments.
+
 	// RegabiWrappers enables ABI wrappers for calling between
 	// ABI0 and ABIInternal functions. Without this, the ABIs are
 	// assumed to be identical so cross-ABI calls are direct.