diff --git a/interp/api.go b/interp/api.go index 9492a781..aed36e9e 100644 --- a/interp/api.go +++ b/interp/api.go @@ -814,27 +814,9 @@ func (r *Runner) Subshell() *Runner { origStdout: r.origStdout, // used for process substitutions } - // Env vars and funcs are copied, since they might be modified. - // TODO(v4): lazy copying? it would probably be enough to add a - // copyOnWrite bool field to Variable, then a Modify method that must be - // used when one needs to modify a variable. ideally with some way to - // catch direct modifications without the use of Modify and panic, - // perhaps via a check when getting or setting vars at some level. - oenv := &overlayEnviron{parent: expand.ListEnviron()} - r.writeEnv.Each(func(name string, vr expand.Variable) bool { - vr2 := vr - // Make deeper copies of List and Map, but ensure that they remain nil - // if they are nil in vr. - vr2.List = append([]string(nil), vr.List...) - if vr.Map != nil { - vr2.Map = make(map[string]string, len(vr.Map)) - for k, vr := range vr.Map { - vr2.Map[k] = vr - } - } - oenv.Set(name, vr2) - return true - }) + // Funcs are copied, since they might be modified. + // Env vars aren't copied; setVar will copy lists and maps as needed. + oenv := &overlayEnviron{parent: r.writeEnv} r2.writeEnv = oenv r2.Funcs = make(map[string]*syntax.Stmt, len(r.Funcs)) for k, v := range r.Funcs { diff --git a/interp/interp_test.go b/interp/interp_test.go index 5c1f1525..eb271da1 100644 --- a/interp/interp_test.go +++ b/interp/interp_test.go @@ -3868,6 +3868,16 @@ func TestRunnerOpts(t *testing.T) { "set bar_interp_missing; echo $@", "bar_interp_missing\n", }, + { + opts(interp.Env(expand.FuncEnviron(func(name string) string { + if name == "foo" { + return "bar" + } + return "" + }))), + "(echo $foo); echo x | echo $foo", + "bar\nbar\n", + }, } p := syntax.NewParser() for _, c := range cases { diff --git a/interp/vars.go b/interp/vars.go index feda4285..45447094 100644 --- a/interp/vars.go +++ b/interp/vars.go @@ -31,21 +31,19 @@ func (o *overlayEnviron) Get(name string) expand.Variable { } func (o *overlayEnviron) Set(name string, vr expand.Variable) error { - // Manipulation of a global var inside a function + // Manipulation of a global var inside a function. if o.funcScope && !vr.Local && !o.values[name].Local { - // "foo=bar" on a global var in a function updates the global scope if vr.IsSet() { - return o.parent.(expand.WriteEnviron).Set(name, vr) - } - // "foo=bar" followed by "export foo" or "readonly foo" - if vr.Exported || vr.ReadOnly { + // "foo=bar" on a global var in a function updates the global scope + } else if vr.Exported || vr.ReadOnly { + // "foo=bar" followed by "export foo" or "readonly foo" prev := o.Get(name) prev.Exported = prev.Exported || vr.Exported prev.ReadOnly = prev.ReadOnly || vr.ReadOnly vr = prev - return o.parent.(expand.WriteEnviron).Set(name, vr) } - // "unset" is handled below + // In a function, the parent environment is ours, so it's always read-write. + return o.parent.(expand.WriteEnviron).Set(name, vr) } prev := o.Get(name) @@ -71,10 +69,6 @@ func (o *overlayEnviron) Set(name string, vr expand.Variable) error { return nil } delete(o.values, name) - if writeEnv, _ := o.parent.(expand.WriteEnviron); writeEnv != nil { - writeEnv.Set(name, vr) - return nil - } } else if prev.Exported { // variable is set and was marked as exported vr.Exported = true @@ -234,7 +228,9 @@ func (r *Runner) setVar(name string, index syntax.ArithmExpr, vr expand.Variable case expand.String: list = append(list, cur.Str) case expand.Indexed: - list = cur.List + // TODO: slices.Clone + // TODO: only clone when inside a subshell and getting a var from outside for the first time + list = append([]string(nil), cur.List...) case expand.Associative: // if the existing variable is already an AssocArray, try our // best to convert the key to a string @@ -243,6 +239,14 @@ func (r *Runner) setVar(name string, index syntax.ArithmExpr, vr expand.Variable return } k := r.literal(w) + + // TODO: maps.Clone + // TODO: only clone when inside a subshell and getting a var from outside for the first time + m2 := make(map[string]string, len(cur.Map)) + for k, vr := range cur.Map { + m2[k] = vr + } + cur.Map = m2 cur.Map[k] = valStr r.setVarInternal(name, cur) return