cue: various cleanup

- allow comprehensions to be any value
- consistently use recurisive unification
- use new comprehension format
- deprecate Split in favor of Expr

Change-Id: Ie1c75768e05dc0df6316ac27ad5636b0163b1c3d
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/3183
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cue/ast_test.go b/cue/ast_test.go
index 03cdbe4..2663112 100644
--- a/cue/ast_test.go
+++ b/cue/ast_test.go
@@ -213,7 +213,11 @@
 		out: `<0>{a: <1>{<>: <2>(name: string)-><3>{n: <2>.name}, k: 1}, b: <4>{<>: <5>(x: string)-><6>{x: 0, y: 1}, v: <7>{}}}`,
 	}, {
 		in: `
-		a: { "\(k)": v for k, v in b if b.a < k }
+		a: {
+			for k, v in b if b.a < k {
+				"\(k)": v
+			}
+		}
 		b: {
 			a: 1
 			b: 2
@@ -223,7 +227,7 @@
 		out: `<0>{a: <1>{ <2>for k, v in <0>.b if (<0>.b.a < <2>.k) yield (""+<2>.k+""): <2>.v}, b: <3>{a: 1, b: 2, c: 3}}`,
 	}, {
 		in: `
-			a: { "\(v)": v for k, v in b }
+			a: { for k, v in b {"\(v)": v} }
 			b: { a: "aa", b: "bb", c: "cc" }
 			`,
 		out: `<0>{a: <1>{ <2>for k, v in <0>.b yield (""+<2>.v+""): <2>.v}, b: <3>{a: "aa", b: "bb", c: "cc"}}`,
diff --git a/cue/binop.go b/cue/binop.go
index 55286f4..f5a8e44 100644
--- a/cue/binop.go
+++ b/cue/binop.go
@@ -253,7 +253,7 @@
 	case *basicType:
 		switch op {
 		// TODO: other types.
-		case opUnify:
+		case opUnify, opUnifyUnchecked:
 			if k&typeKinds != bottomKind {
 				return &basicType{binSrc(src.Pos(), op, x, other), k & typeKinds}
 			}
@@ -264,7 +264,7 @@
 		return ctx.mkErr(src, codeIncomplete, "%s with incomplete values", op)
 
 	case *numLit:
-		if op == opUnify {
+		if op == opUnify || op == opUnifyUnchecked {
 			if k == y.k {
 				return y
 			}
@@ -637,12 +637,12 @@
 	// we need to apply the template to all elements.
 
 	sz := len(x.comprehensions) + len(y.comprehensions)
-	obj.comprehensions = make([]*fieldComprehension, sz)
+	obj.comprehensions = make([]value, sz)
 	for i, c := range x.comprehensions {
-		obj.comprehensions[i] = ctx.copy(c).(*fieldComprehension)
+		obj.comprehensions[i] = ctx.copy(c)
 	}
 	for i, c := range y.comprehensions {
-		obj.comprehensions[i+len(x.comprehensions)] = ctx.copy(c).(*fieldComprehension)
+		obj.comprehensions[i+len(x.comprehensions)] = ctx.copy(c)
 	}
 
 	for _, a := range x.arcs {
@@ -719,14 +719,14 @@
 			return &boolLit{baseValue: src.base(), b: true}
 		case opNeq:
 			return &boolLit{baseValue: src.base(), b: false}
-		case opUnify:
+		case opUnify, opUnifyUnchecked:
 			return x
 		}
 
 	case *bound:
 		// Not strictly necessary, but handling this results in better error
 		// messages.
-		if op == opUnify {
+		if op == opUnify || op == opUnifyUnchecked {
 			return other.binOp(ctx, src, opUnify, x)
 		}
 
@@ -749,7 +749,7 @@
 
 	case *boolLit:
 		switch op {
-		case opUnify:
+		case opUnify, opUnifyUnchecked:
 			if x.b != y.b {
 				return ctx.mkErr(x, "conflicting values %v and %v", x.b, y.b)
 			}
@@ -777,7 +777,7 @@
 	case *stringLit:
 		str := other.strValue()
 		switch op {
-		case opUnify:
+		case opUnify, opUnifyUnchecked:
 			str := other.strValue()
 			if x.str != str {
 				src := mkBin(ctx, src.Pos(), op, x, other)
@@ -831,7 +831,7 @@
 	case *bytesLit:
 		b := y.b
 		switch op {
-		case opUnify:
+		case opUnify, opUnifyUnchecked:
 			if !bytes.Equal(x.b, b) {
 				return ctx.mkErr(x, "conflicting values %v and %v",
 					ctx.str(x), ctx.str(y))
@@ -914,7 +914,7 @@
 		result = r == -1
 	case opLeq:
 		result = r != 1
-	case opEql, opUnify:
+	case opEql, opUnify, opUnifyUnchecked:
 		result = r == 0
 	case opNeq:
 		result = r != 0
@@ -929,14 +929,14 @@
 func (x *numLit) binOp(ctx *context, src source, op op, other evaluated) evaluated {
 	switch y := other.(type) {
 	case *basicType, *bound, *customValidator: // for better error reporting
-		if op == opUnify {
+		if op == opUnify || op == opUnifyUnchecked {
 			return y.binOp(ctx, src, op, x)
 		}
 	case *numLit:
 		k := unifyType(x.kind(), y.kind())
 		n := newNumBin(k, x, y)
 		switch op {
-		case opUnify:
+		case opUnify, opUnifyUnchecked:
 			if x.v.Cmp(&y.v) != 0 {
 				src = mkBin(ctx, src.Pos(), op, x, other)
 				return ctx.mkErr(src, "conflicting values %v and %v",
@@ -1029,7 +1029,7 @@
 
 	case *durationLit:
 		switch op {
-		case opUnify:
+		case opUnify, opUnifyUnchecked:
 			if x.d != y.d {
 				return ctx.mkIncompatible(src, op, x, other)
 			}
@@ -1090,13 +1090,13 @@
 
 func (x *list) binOp(ctx *context, src source, op op, other evaluated) evaluated {
 	switch op {
-	case opUnify:
+	case opUnify, opUnifyUnchecked:
 		y, ok := other.(*list)
 		if !ok {
 			break
 		}
 
-		n := binOp(ctx, src, opUnify, x.len.(evaluated), y.len.(evaluated))
+		n := binOp(ctx, src, op, x.len.(evaluated), y.len.(evaluated))
 		if isBottom(n) {
 			src = mkBin(ctx, src.Pos(), op, x, other)
 			return ctx.mkErr(src, "conflicting list lengths: %v", n)
@@ -1116,7 +1116,7 @@
 		max, ok := n.(*numLit)
 		if !ok || len(xa) < max.intValue(ctx) {
 			src := mkBin(ctx, src.Pos(), op, x.typ, y.typ)
-			typ = binOp(ctx, src, opUnify, x.typ.(evaluated), y.typ.(evaluated))
+			typ = binOp(ctx, src, op, x.typ.(evaluated), y.typ.(evaluated))
 			if isBottom(typ) {
 				return ctx.mkErr(src, "conflicting list element types: %v", typ)
 			}
@@ -1125,7 +1125,7 @@
 		// TODO: use forwarding instead of this mild hack.
 		x.elem.arcs = xa
 		y.elem.arcs = ya
-		s := binOp(ctx, src, opUnify, x.elem, y.elem).(*structLit)
+		s := binOp(ctx, src, op, x.elem, y.elem).(*structLit)
 		x.elem.arcs = sx
 		y.elem.arcs = sy
 
@@ -1245,7 +1245,7 @@
 }
 
 func (x *builtin) binOp(ctx *context, src source, op op, other evaluated) evaluated {
-	if op == opUnify && evaluated(x) == other {
+	if _, isUnify := op.unifyType(); isUnify && evaluated(x) == other {
 		return x
 	}
 	return ctx.mkIncompatible(src, op, x, other)
diff --git a/cue/copy.go b/cue/copy.go
index a28eaa2..879758a 100644
--- a/cue/copy.go
+++ b/cue/copy.go
@@ -56,9 +56,9 @@
 			arcs[i] = a
 		}
 
-		comp := make([]*fieldComprehension, len(x.comprehensions))
+		comp := make([]value, len(x.comprehensions))
 		for i, c := range x.comprehensions {
-			comp[i] = ctx.copy(c).(*fieldComprehension)
+			comp[i] = ctx.copy(c)
 		}
 		obj.comprehensions = comp
 		return obj, false
diff --git a/cue/export.go b/cue/export.go
index 1a72d4c..ec454b5 100644
--- a/cue/export.go
+++ b/cue/export.go
@@ -656,42 +656,45 @@
 		obj.Elts = append(obj.Elts, f)
 	}
 
-	for _, c := range x.comprehensions {
-		var clauses []ast.Clause
-		next := c.clauses
-		for {
-			if yield, ok := next.(*yield); ok {
-				l := p.expr(yield.key)
-				label, ok := l.(ast.Label)
-				if !ok {
-					// TODO: add an invalid field instead?
-					continue
-				}
-				opt := token.NoPos
-				if yield.opt {
-					opt = token.NoSpace.Pos() // anything but token.NoPos
-				}
-				f := &ast.Field{
-					Label:    label,
-					Optional: opt,
-					Value:    p.expr(yield.value),
-				}
-				var decl ast.Decl = f
-				if len(clauses) > 0 {
-					decl = &ast.Comprehension{
-						Clauses: clauses,
-						Value: &ast.StructLit{
-							Elts: []ast.Decl{f},
-						},
+	for _, v := range x.comprehensions {
+		switch c := v.(type) {
+		case *fieldComprehension:
+			var clauses []ast.Clause
+			next := c.clauses
+			for {
+				if yield, ok := next.(*yield); ok {
+					l := p.expr(yield.key)
+					label, ok := l.(ast.Label)
+					if !ok {
+						// TODO: add an invalid field instead?
+						continue
 					}
+					opt := token.NoPos
+					if yield.opt {
+						opt = token.NoSpace.Pos() // anything but token.NoPos
+					}
+					f := &ast.Field{
+						Label:    label,
+						Optional: opt,
+						Value:    p.expr(yield.value),
+					}
+					var decl ast.Decl = f
+					if len(clauses) > 0 {
+						decl = &ast.Comprehension{
+							Clauses: clauses,
+							Value: &ast.StructLit{
+								Elts: []ast.Decl{f},
+							},
+						}
+					}
+					obj.Elts = append(obj.Elts, decl)
+					break
 				}
-				obj.Elts = append(obj.Elts, decl)
-				break
-			}
 
-			var y ast.Clause
-			y, next = p.clause(next)
-			clauses = append(clauses, y)
+				var y ast.Clause
+				y, next = p.clause(next)
+				clauses = append(clauses, y)
+			}
 		}
 	}
 	return obj, nil
diff --git a/cue/export_test.go b/cue/export_test.go
index a8c7bc3..f968c7a 100644
--- a/cue/export_test.go
+++ b/cue/export_test.go
@@ -229,7 +229,7 @@
 			}`),
 	}, {
 		raw: true,
-		in:  `{ a: [1, 2], b: { "\(k)": v for k, v in a if v > 1 } }`,
+		in:  `{ a: [1, 2], b: { for k, v in a if v > 1 { "\(k)": v } } }`,
 		out: unindent(`
 			{
 				a: [1, 2]
diff --git a/cue/instance_test.go b/cue/instance_test.go
index 2b3405b..0a7ca56 100644
--- a/cue/instance_test.go
+++ b/cue/instance_test.go
@@ -84,12 +84,12 @@
 	}, {
 		desc: "top-level comprehensions",
 		instances: insts(`
-			t: {"\(k)": 10 for k, x in s}
+			t: { for k, x in s {"\(k)": 10} }
 			s <Name>: {}
 			s foo a: 1
 			`,
 			`
-			t: {"\(k)": 10 for k, x in s}
+			t: { for k, x in s {"\(k)": 10 } }
 			s <Name>: {}
 			s bar b: 2
 			`,
diff --git a/cue/resolve_test.go b/cue/resolve_test.go
index 2ab7650..8a9b838 100644
--- a/cue/resolve_test.go
+++ b/cue/resolve_test.go
@@ -1298,24 +1298,36 @@
 		A :: {f1: int, f2: int}
 
 		// Comprehension fields cannot be added like any other.
-		a: A & { "\(k)": v for k, v in {f3 : int}}
+		for k, v in {f3 : int} {
+			a: A & { "\(k)": v }
+		}
 
-		// A closed struct may not generate comprehension values it does not
-		// define explicitly.
-		B :: {"\(k)": v for k, v in {f1: int}}
+		B :: {
+			for k, v in {f1: int} {
+				"\(k)": v
+			}
+		}
 
-		// To fix this, add all allowed fields.
-		C :: {f1: _, "\(k)": v for k, v in {f1: int}}
+		C :: {
+			f1: _
+			for k, v in {f1: int} {
+				"\(k)": v
+			}
+		}
 
-		// Or like this.
-		D :: {"\(k)": v for k, v in {f1: int}, ...}
+		D :: {
+			for k, v in {f1: int} {
+				"\(k)": v
+			}
+			...
+		}
 		`,
 		out: `<0>{` +
 			`A :: <1>C{f1: int, f2: int}, ` +
-			`a: _|_(int:field "f3" not allowed in closed struct), ` +
-			`B :: _|_(int:field "f1" not allowed in closed struct), ` +
-			`C :: <2>C{f1: int}, ` +
-			`D :: <3>{f1: int, ...}}`,
+			`B :: <2>C{f1: int}, ` +
+			`C :: <3>C{f1: int}, ` +
+			`D :: <4>{f1: int, ...}, ` +
+			`a: <5>C{f1: int, f2: int, f3: int}}`,
 	}, {
 		desc: "reference to root",
 		in: `
@@ -1563,7 +1575,9 @@
 			obj foo a: "bar"
 			obj <Name>: {
 				a: *"dummy" | string
-				sub as: a if true
+				if true {
+					sub as: a
+				}
 			}
 		`,
 		out: `<0>{obj: <1>{<>: <2>(Name: string)-><3>{a: (*"dummy" | string) if true yield ("sub"): <4>{as: <3>.a}}, ` +
@@ -1855,7 +1869,13 @@
 	}, {
 		desc: "field comprehension",
 		in: `
-			a: { "\(k)": v for k, v in b if k < "d" if v > b.a }
+			a: {
+				for k, v in b
+				if k < "d"
+				if v > b.a {
+					"\(k)": v
+				}
+			}
 			b: {
 				a: 1
 				b: 2
@@ -1863,25 +1883,32 @@
 				d: 4
 			}
 			c: {
-				"\(k)": v <-
-					for k, v in b
-					if k < "d"
-					if v > b.a
+				for k, v in b
+				if k < "d"
+				if v > b.a {
+					"\(k)": v
+				}
 			}
 			`,
 		out: `<0>{a: <1>{b: 2, c: 3}, b: <2>{a: 1, b: 2, c: 3, d: 4}, c: <3>{b: 2, c: 3}}`,
 	}, {
 		desc: "conditional field",
 		in: `
-			a: "foo" if b
+			if b {
+				a: "foo"
+			}
 			b: true
 			c: {
 				a: 3
-				a: 3 if a > 1
+				if a > 1 {
+					a: 3
+				}
 			}
 			d: {
 				a: int
-				a: 3 if a > 1
+				if a > 1 {
+					a: 3
+				}
 			}
 		`,
 		out: `<0>{b: true, c: <1>{a: 3}, a: "foo", d: <2>{a: int if (<2>.a > 1) yield ("a"): 3}}`,
@@ -1891,7 +1918,9 @@
 		a: { b c: 4 }
 		a: {
 			b d: 5
-			"\(k)": v for k, v in b
+			for k, v in b {
+				"\(k)": v
+			}
 		}
 		`,
 		out: `<0>{a: <1>{b: <2>{c: 4, d: 5}, c: 4, d: 5}}`,
@@ -1941,17 +1970,22 @@
 	}, {
 		desc: "field comprehensions with multiple keys",
 		in: `
-			a "\(x.a)" b "\(x.b)": x for x in [
+			for x in [
 				{a: "A", b: "B" },
 				{a: "C", b: "D" },
 				{a: "E", b: "F" },
-			]
+			] {
+				a "\(x.a)" b "\(x.b)": x
+			}
 
-			"\(x.a)" "\(x.b)": x for x in [
+			for x in [
 				{a: "A", b: "B" },
 				{a: "C", b: "D" },
 				{a: "E", b: "F" },
-			]`,
+			] {
+				"\(x.a)" "\(x.b)": x
+			}
+			`,
 		out: `<0>{E: <1>{F: <2>{a: "E", b: "F"}}, ` +
 			`a: <3>{` +
 			`E: <4>{b: <5>{F: <6>{a: "E", b: "F"}}}, ` +
@@ -1972,10 +2006,12 @@
 		in: `
 			num: 1
 			a: {
-				<A> <B>: {
-					name: A
-					kind: B
-				} if num < 5
+				if num < 5 {
+					<A> <B>: {
+						name: A
+						kind: B
+					}
+				}
 			}
 			a b c d: "bar"
 			`,
@@ -2053,7 +2089,7 @@
 	}, {
 		desc: "resolutions in struct comprehension keys",
 		in: `
-			a: { "\(b + ".")": "a" for _, b in ["c"] }
+			a: { for _, b in ["c"] { "\(b + ".")": "a" } }
 			`,
 		out: `<0>{a: <1>{c.: "a"}}`,
 	}, {
@@ -2151,8 +2187,12 @@
 		fib: {
 			n: int
 
-			out: (fibRec & {nn: n - 2}).out + (fibRec & {nn: n - 1}).out if n >= 2
-			out: n if n < 2
+			if n >= 2 {
+				out: (fibRec & {nn: n - 2}).out + (fibRec & {nn: n - 1}).out
+			}
+			if n < 2 {
+				out: n
+			}
 		}
 		fib2: (fib & {n: 2}).out
 		fib7: (fib & {n: 7}).out
diff --git a/cue/types.go b/cue/types.go
index e71ab71..52be547 100644
--- a/cue/types.go
+++ b/cue/types.go
@@ -776,8 +776,12 @@
 }
 
 // Split returns a list of values from which v originated such that
-// the unification of all these values equals v and for all returned values
+// the unification of all these values equals v and for all returned values.
+// It will also split unchecked unifications (embeddings), so unifying the
+// split values may fail if actually unified.
 // Source returns a non-nil value.
+//
+// Deprecated: use Expr.
 func (v Value) Split() []Value {
 	if v.path == nil {
 		return nil
@@ -795,7 +799,7 @@
 
 func separate(v value) (a []value) {
 	c := v.computed()
-	if c == nil || c.op != opUnify {
+	if c == nil || (c.op != opUnify && c.op != opUnifyUnchecked) {
 		return []value{v}
 	}
 	if c.x != nil {
diff --git a/cue/types_test.go b/cue/types_test.go
index bcfc85e..2503949 100644
--- a/cue/types_test.go
+++ b/cue/types_test.go
@@ -609,10 +609,13 @@
 		value: `{_a:"a"}`,
 		res:   "{}",
 	}, {
-		value: `{"\(k)": v for k, v in y if v > 1}
+		value: `{ for k, v in y if v > 1 {"\(k)": v} }
 		y: {a:1,b:2,c:3}`,
 		res: "{b:2,c:3,}",
 	}, {
+		value: `{ def :: 1, _hidden: 2, opt?: 3, reg: 4 }`,
+		res:   "{reg:4,}",
+	}, {
 		value: `{a:1,b:2,c:int}`,
 		err:   "cannot convert incomplete value",
 	}}
@@ -1021,7 +1024,7 @@
 		dst:   &fields{},
 		want:  fields{A: 1, B: 2, C: 3},
 	}, {
-		value: `{"\(k)": v for k, v in y if v > 1}
+		value: `{for k, v in y if v > 1 {"\(k)": v} }
 		y: {a:1,b:2,c:3}`,
 		dst:  &fields{},
 		want: fields{B: 2, C: 3},
diff --git a/cue/value.go b/cue/value.go
index 524bc1b..c24af45 100644
--- a/cue/value.go
+++ b/cue/value.go
@@ -642,7 +642,7 @@
 	template value
 	isClosed bool
 
-	comprehensions []*fieldComprehension
+	comprehensions []value
 
 	// TODO: consider hoisting the template arc to its own value.
 	arcs     []arc
@@ -783,6 +783,10 @@
 	return x.arcs[i].cache
 }
 
+// expandFields merges in embedded and interpolated fields.
+// Such fields are semantically equivalent to child values, and thus
+// should not be evaluated until the other fields of a struct are
+// fully evaluated.
 func (x *structLit) expandFields(ctx *context) (st *structLit, err *bottom) {
 	switch v := x.expanded.(type) {
 	case nil:
@@ -791,7 +795,7 @@
 	default:
 		return nil, x.expanded.(*bottom)
 	}
-	if x.comprehensions == nil {
+	if len(x.comprehensions) == 0 {
 		x.expanded = x
 		return x, nil
 	}
@@ -803,38 +807,42 @@
 	template := x.template
 	newArcs := []arc{}
 
-	for _, c := range comprehensions {
-		err := c.clauses.yield(ctx, func(k, v evaluated, opt, def bool) *bottom {
-			if !k.kind().isAnyOf(stringKind) {
-				return ctx.mkErr(k, "key must be of type string")
-			}
-			if c.isTemplate {
-				// TODO: disallow altogether or only when it refers to fields.
-				if template == nil {
-					template = v
-				} else {
-					template = mkBin(ctx, c.Pos(), opUnify, template, v)
+	for _, x := range comprehensions {
+		switch c := x.(type) {
+		case *fieldComprehension:
+			err := c.clauses.yield(ctx, func(k, v evaluated, opt, def bool) *bottom {
+				if !k.kind().isAnyOf(stringKind) {
+					return ctx.mkErr(k, "key must be of type string")
 				}
-				return nil
-			}
-			// TODO(perf): improve big O
-			f := ctx.label(k.strValue(), true)
-			for i, a := range newArcs {
-				if a.feature == f {
-					newArcs[i].v = mkBin(ctx, x.Pos(), opUnify, a.v, v)
+
+				if c.isTemplate {
+					// TODO: disallow altogether or only when it refers to fields.
+					if template == nil {
+						template = v
+					} else {
+						template = mkBin(ctx, c.Pos(), opUnify, template, v)
+					}
 					return nil
 				}
-			}
-			newArcs = append(newArcs, arc{
-				feature:    f,
-				optional:   opt,
-				definition: def,
-				v:          v,
+				// TODO(perf): improve big O
+				f := ctx.label(k.strValue(), true)
+				for i, a := range newArcs {
+					if a.feature == f {
+						newArcs[i].v = mkBin(ctx, x.Pos(), opUnify, a.v, v)
+						return nil
+					}
+				}
+				newArcs = append(newArcs, arc{
+					feature:    f,
+					optional:   opt,
+					definition: def,
+					v:          v,
+				})
+				return nil
 			})
-			return nil
-		})
-		if err != nil {
-			return nil, err
+			if err != nil {
+				return nil, err
+			}
 		}
 	}
 
@@ -845,7 +853,7 @@
 	orig := *x
 	orig.comprehensions = nil
 
-	res := orig.binOp(ctx, x, opUnify, &structLit{
+	res := orig.binOp(ctx, x, opUnifyUnchecked, &structLit{
 		x.baseValue, // baseValue
 		emit,        // emit
 		template,    // template