expand: redesign variables to remove forced allocs

We were fitting string and []string into the Value interface{}, which
always meant allocations. This is because neither type was a pointer,
and interface values can only fit directly if they are pointers.

We also tried using an interface with implementations that were
exclusively pointers, but that was clumsy. For one, strings and slices
already contain pointers, so it added double indirection. It also
required quite a bit more boilerplate code for the user.

In the end, a Kind field and three flat fields is the best of both
worlds. With no interface and no double pointers, the data is always one
pointer away, and setting a value never requires an allocation.

This results in a very noticeable drop in allocs/op, particularly for
looping operations like Environ.Each.

There's a small bump in alloc/op, since Variable is now a bit heavier.
We'll recover part of that in a follow-up commit by merging NameRef into
Kind, since that will too make nameref variables simpler.

Overall, using expand.Variable is a tiny bit more verbose and perhaps
less idiomatic, but since it's a fairly low-level and integral part of
the expand and interp packages, we can't have it be inefficient.

name   old time/op    new time/op    delta
Run-8    1.46ms ± 1%    1.45ms ± 0%     ~     (p=0.093 n=6+6)

name   old alloc/op   new alloc/op   delta
Run-8    79.4kB ± 0%    81.2kB ± 0%   +2.30%  (p=0.002 n=6+6)

name   old allocs/op  new allocs/op  delta
Run-8     1.03k ± 0%     0.88k ± 0%  -14.52%  (p=0.008 n=5+5)

expand/environ.go

@@ -46,12 +46,20 @@ type WriteEnviron interface {
 	Set(name string, vr Variable) error
 }
 
+type ValueKind int
+
+const (
+	Unset ValueKind = iota
+	String
+	Indexed
+	Associative
+)
+
 // Variable describes a shell variable, which can have a number of attributes
 // and a value.
 //
-// A Variable is unset if its Value field is untyped nil, which can be checked
-// via Variable.IsSet. The zero value of a Variable is thus a valid unset
-// variable.
+// A Variable is unset if its Kind field is Unset, which can be checked via
+// Variable.IsSet. The zero value of a Variable is thus a valid unset variable.
 //
 // If a variable is set, its Value field will be a []string if it is an indexed
 // array, a map[string]string if it's an associative array, or a string
@@ -60,27 +68,32 @@ type Variable struct {
 	Local    bool
 	Exported bool
 	ReadOnly bool
-	NameRef  bool        // if true, Value must be string
-	Value    interface{} // string, []string, or map[string]string
+	NameRef  bool // if true, Value must be string
+
+	Kind ValueKind
+
+	Str  string            // Used when Kind is String.
+	List []string          // Used when Kind is Indexed.
+	Map  map[string]string // Used when Kind is Associative.
 }
 
 // IsSet returns whether the variable is set. An empty variable is set, but an
 // undeclared variable is not.
 func (v Variable) IsSet() bool {
-	return v.Value != nil
+	return v.Kind != Unset
 }
 
 // String returns the variable's value as a string. In general, this only makes
 // sense if the variable has a string value or no value at all.
 func (v Variable) String() string {
-	switch x := v.Value.(type) {
-	case string:
-		return x
-	case []string:
-		if len(x) > 0 {
-			return x[0]
+	switch v.Kind {
+	case String:
+		return v.Str
+	case Indexed:
+		if len(v.List) > 0 {
+			return v.List[0]
 		}
-	case map[string]string:
+	case Associative:
 		// nothing to do
 	}
 	return ""
@@ -99,7 +112,7 @@ func (v Variable) Resolve(env Environ) (string, Variable) {
 		if !v.NameRef {
 			return name, v
 		}
-		name = v.Value.(string)
+		name = v.Str // keep name for the next iteration
 		v = env.Get(name)
 	}
 	return name, Variable{}
@@ -121,7 +134,7 @@ func (f funcEnviron) Get(name string) Variable {
 	if value == "" {
 		return Variable{}
 	}
-	return Variable{Exported: true, Value: value}
+	return Variable{Exported: true, Kind: String, Str: value}
 }
 
 func (f funcEnviron) Each(func(name string, vr Variable) bool) {}
@@ -177,7 +190,7 @@ func (l listEnviron) Get(name string) Variable {
 	prefix := name + "="
 	i := sort.SearchStrings(l, prefix)
 	if i < len(l) && strings.HasPrefix(l[i], prefix) {
-		return Variable{Exported: true, Value: strings.TrimPrefix(l[i], prefix)}
+		return Variable{Exported: true, Kind: String, Str: strings.TrimPrefix(l[i], prefix)}
 	}
 	return Variable{}
 }
@@ -190,7 +203,7 @@ func (l listEnviron) Each(fn func(name string, vr Variable) bool) {
 			panic("expand.listEnviron: did not expect malformed name-value pair: " + pair)
 		}
 		name, value := pair[:i], pair[i+1:]
-		if !fn(name, Variable{Exported: true, Value: value}) {
+		if !fn(name, Variable{Exported: true, Kind: String, Str: value}) {
 			return
 		}
 	}

expand/expand.go

@@ -118,7 +118,7 @@ func (cfg *Config) envSet(name, value string) error {
 	if !ok {
 		return fmt.Errorf("environment is read-only")
 	}
-	return wenv.Set(name, Variable{Value: value})
+	return wenv.Set(name, Variable{Kind: String, Str: value})
 }
 
 // Literal expands a single shell word. It is similar to Fields, but the result
@@ -540,14 +540,14 @@ func (cfg *Config) quotedElems(pe *syntax.ParamExp) []string {
 		return nil
 	}
 	if pe.Param.Value == "@" {
-		return cfg.Env.Get("@").Value.([]string)
+		return cfg.Env.Get("@").List
 	}
 	if nodeLit(pe.Index) != "@" {
 		return nil
 	}
-	val := cfg.Env.Get(pe.Param.Value).Value
-	if x, ok := val.([]string); ok {
-		return x
+	vr := cfg.Env.Get(pe.Param.Value)
+	if vr.Kind == Indexed {
+		return vr.List
 	}
 	return nil
 }

expand/param.go

@@ -50,7 +50,7 @@ func (cfg *Config) paramExp(pe *syntax.ParamExp) (string, error) {
 		// This is the only parameter expansion that the environment
 		// interface cannot satisfy.
 		line := uint64(cfg.curParam.Pos().Line())
-		vr.Value = strconv.FormatUint(line, 10)
+		vr = Variable{Kind: String, Str: strconv.FormatUint(line, 10)}
 	default:
 		vr = cfg.Env.Get(name)
 	}
@@ -74,11 +74,11 @@ func (cfg *Config) paramExp(pe *syntax.ParamExp) (string, error) {
 	elems := []string{str}
 	switch nodeLit(index) {
 	case "@", "*":
-		switch x := vr.Value.(type) {
-		case nil:
+		switch vr.Kind {
+		case Unset:
 			elems = nil
-		case []string:
-			elems = x
+		case Indexed:
+			elems = vr.List
 		}
 	}
 	switch {
@@ -95,15 +95,15 @@ func (cfg *Config) paramExp(pe *syntax.ParamExp) (string, error) {
 		if pe.Names != 0 {
 			strs = cfg.namesByPrefix(pe.Param.Value)
 		} else if orig.NameRef {
-			strs = append(strs, orig.Value.(string))
-		} else if x, ok := vr.Value.([]string); ok {
-			for i, e := range x {
+			strs = append(strs, orig.Str)
+		} else if vr.Kind == Indexed {
+			for i, e := range vr.List {
 				if e != "" {
 					strs = append(strs, strconv.Itoa(i))
 				}
 			}
-		} else if x, ok := vr.Value.(map[string]string); ok {
-			for k := range x {
+		} else if vr.Kind == Associative {
+			for k := range vr.Map {
 				strs = append(strs, k)
 			}
 		} else if str != "" {
@@ -289,40 +289,40 @@ func (cfg *Config) varInd(vr Variable, idx syntax.ArithmExpr) (string, error) {
 	if idx == nil {
 		return vr.String(), nil
 	}
-	switch x := vr.Value.(type) {
-	case string:
+	switch vr.Kind {
+	case String:
 		n, err := Arithm(cfg, idx)
 		if err != nil {
 			return "", err
 		}
 		if n == 0 {
-			return x, nil
+			return vr.Str, nil
 		}
-	case []string:
+	case Indexed:
 		switch nodeLit(idx) {
 		case "@":
-			return strings.Join(x, " "), nil
+			return strings.Join(vr.List, " "), nil
 		case "*":
-			return cfg.ifsJoin(x), nil
+			return cfg.ifsJoin(vr.List), nil
 		}
 		i, err := Arithm(cfg, idx)
 		if err != nil {
 			return "", err
 		}
-		if len(x) > 0 {
-			return x[i], nil
+		if len(vr.List) > 0 {
+			return vr.List[i], nil
 		}
-	case map[string]string:
+	case Associative:
 		switch lit := nodeLit(idx); lit {
 		case "@", "*":
 			var strs []string
-			keys := make([]string, 0, len(x))
-			for k := range x {
+			keys := make([]string, 0, len(vr.Map))
+			for k := range vr.Map {
 				keys = append(keys, k)
 			}
 			sort.Strings(keys)
 			for _, k := range keys {
-				strs = append(strs, x[k])
+				strs = append(strs, vr.Map[k])
 			}
 			if lit == "*" {
 				return cfg.ifsJoin(strs), nil
@@ -333,7 +333,7 @@ func (cfg *Config) varInd(vr Variable, idx syntax.ArithmExpr) (string, error) {
 		if err != nil {
 			return "", err
 		}
-		return x[val], nil
+		return vr.Map[val], nil
 	}
 	return "", nil
 }

interp/builtin.go

@@ -469,7 +469,7 @@ func (r *Runner) builtinCode(ctx context.Context, pos syntax.Pos, name string, a
 			if i < len(values) {
 				val = values[i]
 			}
-			r.setVar(name, nil, expand.Variable{Value: val})
+			r.setVar(name, nil, expand.Variable{Kind: expand.String, Str: val})
 		}
 
 		return 0
@@ -621,7 +621,7 @@ func (r *Runner) changeDir(path string) int {
 	}
 	r.Dir = path
 	r.Vars["OLDPWD"] = r.Vars["PWD"]
-	r.Vars["PWD"] = expand.Variable{Value: path}
+	r.Vars["PWD"] = expand.Variable{Kind: expand.String, Str: path}
 	return 0
 }
 

interp/interp.go

@@ -530,17 +530,17 @@ func (r *Runner) Reset() {
 	}
 	if vr := r.Env.Get("HOME"); !vr.IsSet() {
 		u, _ := user.Current()
-		r.Vars["HOME"] = expand.Variable{Value: u.HomeDir}
+		r.Vars["HOME"] = expand.Variable{Kind: expand.String, Str: u.HomeDir}
 	}
-	r.Vars["PWD"] = expand.Variable{Value: r.Dir}
-	r.Vars["IFS"] = expand.Variable{Value: " \t\n"}
-	r.Vars["OPTIND"] = expand.Variable{Value: "1"}
+	r.Vars["PWD"] = expand.Variable{Kind: expand.String, Str: r.Dir}
+	r.Vars["IFS"] = expand.Variable{Kind: expand.String, Str: " \t\n"}
+	r.Vars["OPTIND"] = expand.Variable{Kind: expand.String, Str: "1"}
 
 	if runtime.GOOS == "windows" {
 		// convert $PATH to a unix path list
 		path := r.Env.Get("PATH").String()
 		path = strings.Join(filepath.SplitList(path), ":")
-		r.Vars["PATH"] = expand.Variable{Value: path}
+		r.Vars["PATH"] = expand.Variable{Kind: expand.String, Str: path}
 	}
 
 	r.dirStack = append(r.dirStack, r.Dir)
@@ -570,7 +570,7 @@ func (r *Runner) modCtx(ctx context.Context) context.Context {
 		oenv.Set(name, vr)
 	}
 	for name, value := range r.cmdVars {
-		oenv.Set(name, expand.Variable{Exported: true, Value: value})
+		oenv.Set(name, expand.Variable{Exported: true, Kind: expand.String, Str: value})
 	}
 	mc.Env = oenv
 	return context.WithValue(ctx, moduleCtxKey{}, mc)
@@ -744,16 +744,15 @@ func (r *Runner) cmd(ctx context.Context, cm syntax.Command) {
 		fields := r.fields(x.Args...)
 		if len(fields) == 0 {
 			for _, as := range x.Assigns {
-				vr := r.lookupVar(as.Name.Value)
-				vr.Value = r.assignVal(as, "")
+				vr := r.assignVal(as, "")
 				r.setVar(as.Name.Value, as.Index, vr)
 			}
 			break
 		}
 		for _, as := range x.Assigns {
-			val := r.assignVal(as, "")
+			vr := r.assignVal(as, "")
 			// we know that inline vars must be strings
-			r.cmdVars[as.Name.Value] = val.(string)
+			r.cmdVars[as.Name.Value] = vr.Str
 		}
 		r.call(ctx, x.Args[0].Pos(), fields)
 		// cmdVars can be nuked here, as they are never useful
@@ -908,8 +907,7 @@ func (r *Runner) cmd(ctx context.Context, cm syntax.Command) {
 					r.exit = 1
 					return
 				}
-				vr := r.lookupVar(as.Name.Value)
-				vr.Value = r.assignVal(as, valType)
+				vr := r.assignVal(as, valType)
 				if global {
 					vr.Local = false
 				} else if local {