Reimplement element assignment using Assoc.

This resolves #392 and #422.

eval/compile_lvalue.go

@@ -122,7 +122,7 @@ func (cp *compiler) lvaluesOne(n *parse.Indexing, msg string) (bool, LValuesOpFu
 		}
 	}
 
-	p := n.Begin()
+	headBegin, headEnd := n.Head.Begin(), n.Head.End()
 	indexOps := cp.arrayOps(n.Indicies)
 
 	return explode, func(ec *EvalCtx) []Variable {
@@ -131,33 +131,59 @@ func (cp *compiler) lvaluesOne(n *parse.Indexing, msg string) (bool, LValuesOpFu
 			throwf("variable $%s does not exisit, compiler bug", varname)
 		}
 
-		// Indexing. Do Index up to the last but one index.
-		value := variable.Get()
-		n := len(indexOps)
-		// TODO set location information according.
-		for _, op := range indexOps[:n-1] {
-			indexer := mustIndexer(value, ec)
+		// Evaluate assocers and indices.
+		// Assignment of indexed variables actually assignes the variable, with
+		// the right hand being a nested series of Assocs. As the simplest
+		// example, `a[0] = x` is equivalent to `a = (assoc $a 0 x)`. A more
+		// complex example is that `a[0][1][2] = x` is equivalent to
+		//	`a = (assoc $a 0 (assoc $a[0] 1 (assoc $a[0][1] 2 x)))`.
+		// Note that in each assoc form, the first two arguments can be
+		// determined now, while the last argument is only known when the
+		// right-hand-side is known. So here we evaluate the first two arguments
+		// of each assoc form and put them in two slices, assocers and indicies.
+		// In the previous example, the two slices will contain:
+		//
+		// assocers: $a $a[0] $a[0][1]
+		// indicies:  0     1        2
+		//
+		// When the right-hand side of the assignment becomes available, the new
+		// value for $a is evaluated by doing Assoc from inside out.
+		assocers := make([]Assocer, len(indexOps))
+		indicies := make([]Value, len(indexOps))
+		varValue, ok := variable.Get().(IndexOneAssocer)
+		if !ok {
+			ec.errorpf(headBegin, headEnd, "cannot be indexed for setting")
+		}
+		assocers[0] = varValue
+		for i, op := range indexOps {
+			var lastAssocer IndexOneer
+			if i < len(indexOps)-1 {
+				var ok bool
+				lastAssocer, ok = assocers[i].(IndexOneer)
+				if !ok {
+					// This cannot occur when i==0, since varValue as already
+					// asserted to be an IndexOnner.
+					ec.errorpf(headBegin, indexOps[i-1].End, "cannot be indexed")
+				}
+			}
 
-			indicies := op.Exec(ec)
-			values := indexer.Index(indicies)
+			values := op.Exec(ec)
+			// TODO: Implement multi-indexing.
 			if len(values) != 1 {
 				throw(errors.New("multi indexing not implemented"))
 			}
-			value = values[0]
-		}
-		// Now this must be an IndexSetter.
-		indexSetter, ok := value.(IndexSetter)
-		if !ok {
-			// XXX the indicated end location will fall on or after the opening
-			// bracket of the last index, instead of exactly on the penultimate
-			// index.
-			ec.errorpf(p, indexOps[n-1].Begin, "cannot be indexed for setting (value is %s, type %s)", value.Repr(NoPretty), value.Kind())
-		}
-		// XXX Duplicate code.
-		indicies := indexOps[n-1].Exec(ec)
-		if len(indicies) != 1 {
-			ec.errorpf(indexOps[n-1].Begin, indexOps[n-1].End, "index must eval to a single Value (got %v)", indicies)
+			index := values[0]
+			indicies[i] = index
+
+			if i < len(indexOps)-1 {
+				assocer, ok := lastAssocer.IndexOne(index).(Assocer)
+				if !ok {
+					ec.errorpf(headBegin, indexOps[i].End,
+						"cannot be indexed for setting")
+				}
+				assocers[i+1] = assocer
+			}
 		}
-		return []Variable{elemVariable{indexSetter, indicies[0]}}
+		return []Variable{&elemVariable{variable, assocers, indicies, nil}}
 	}
 }

eval/eval_test.go

@@ -57,12 +57,13 @@ var evalTests = []struct {
 	// TODO: Add a useful hybrid pipeline sample
 
 	// List element assignment
-	// {"li=[foo bar]; li[0]=233; put $@li", strs("233", "bar")},
+	{"li=[foo bar]; li[0]=233; put $@li",
+		want{out: strs("233", "bar")}},
 	// Map element assignment
-	//{"di=[&k=v]; di[k]=lorem; di[k2]=ipsum; put $di[k] $di[k2]",
-	//	want{out: strs("lorem", "ipsum")}},
-	//{"d=[&a=[&b=v]]; put $d[a][b]; d[a][b]=u; put $d[a][b]",
-	//	want{out: strs("v", "u")}},
+	{"di=[&k=v]; di[k]=lorem; di[k2]=ipsum; put $di[k] $di[k2]",
+		want{out: strs("lorem", "ipsum")}},
+	{"d=[&a=[&b=v]]; put $d[a][b]; d[a][b]=u; put $d[a][b]",
+		want{out: strs("v", "u")}},
 	// Multi-assignments.
 	{"{a,b}=(put a b); put $a $b", want{out: strs("a", "b")}},
 	{"@a=(put a b); put $@a", want{out: strs("a", "b")}},

eval/value.go

@@ -131,12 +131,6 @@ type IndexOneer interface {
 	IndexOne(idx Value) Value
 }
 
-// IndexSetter is a Value whose elements can be get as well as set.
-type IndexSetter interface {
-	IndexOneer
-	IndexSet(idx Value, v Value)
-}
-
 func mustIndexer(v Value, ec *EvalCtx) Indexer {
 	indexer, ok := getIndexer(v, ec)
 	if !ok {
@@ -177,6 +171,12 @@ type Assocer interface {
 	Assoc(k, v Value) Value
 }
 
+// IndexOneAssocer combines IndexOneer and Assocer.
+type IndexOneAssocer interface {
+	IndexOneer
+	Assocer
+}
+
 // FromJSONInterface converts a interface{} that results from json.Unmarshal to
 // a Value.
 func FromJSONInterface(v interface{}) Value {

eval/variable.go

@@ -99,18 +99,34 @@ func (cv roCbVariable) Get() Value {
 	return cv()
 }
 
-// elemVariable is an element of a IndexSetter.
+// elemVariable is a (arbitrary nested) element.
+// XXX(xiaq): This is an ephemeral "variable" and is a bad hack.
 type elemVariable struct {
-	container IndexSetter
-	index     Value
+	variable Variable
+	assocers []Assocer
+	indices  []Value
+	setValue Value
 }
 
-func (ev elemVariable) Set(val Value) {
-	ev.container.IndexSet(ev.index, val)
+var errCannotIndex = errors.New("cannot index")
+
+func (ev *elemVariable) Set(v0 Value) {
+	v := v0
+	// Evaluate the actual new value from inside out. See comments in
+	// compile_lvalue.go for how assignment of indexed variables work.
+	for i := len(ev.assocers) - 1; i >= 0; i-- {
+		v = ev.assocers[i].Assoc(ev.indices[i], v)
+	}
+	ev.variable.Set(v)
+	// XXX(xiaq): Remember the set value for use in Get.
+	ev.setValue = v0
 }
 
-func (ev elemVariable) Get() Value {
-	return ev.container.IndexOne(ev.index)
+func (ev *elemVariable) Get() Value {
+	// XXX(xiaq): This is only called from fixNilVariables. We don't want to
+	// waste time accessing the variable, so we simply return the value that was
+	// set.
+	return ev.setValue
 }
 
 // envVariable represents an environment variable.