cue: changes ahead of unchecked unification

Introducing unchecked unification introduces some
boilerplate-y changes. Changes this ahead of time
to make the actual changes stand out.

- Function to determine whether an operation is
  a unification op or not.
- passing the type of unification to various functions
  where previously it was assumed to be opUnify.

Issue #40

Change-Id: I1136a7341cf41b7a5ac47133996fd469d39a700f
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/2869
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cue/binop.go b/cue/binop.go
index d83fb16..b52150c 100644
--- a/cue/binop.go
+++ b/cue/binop.go
@@ -34,15 +34,16 @@
 }
 
 func binOp(ctx *context, src source, op op, left, right evaluated) (result evaluated) {
+	_, isUnify := op.unifyType()
 	if b, ok := left.(*bottom); ok {
-		if op == opUnify && b.exprDepth == 0 && cycleError(b) != nil {
+		if isUnify && b.exprDepth == 0 && cycleError(b) != nil {
 			ctx.cycleErr = true
 			return right
 		}
 		return left
 	}
 	if b, ok := right.(*bottom); ok {
-		if op == opUnify && b.exprDepth == 0 && cycleError(b) != nil {
+		if isUnify && b.exprDepth == 0 && cycleError(b) != nil {
 			ctx.cycleErr = true
 			return left
 		}
@@ -82,7 +83,7 @@
 	if invert {
 		left, right = right, left
 	}
-	if op != opUnify {
+	if !isUnify {
 		// Any operation other than unification or disjunction must be on
 		// concrete types. Disjunction is handled separately.
 		if !leftKind.isGround() || !rightKind.isGround() {
@@ -94,7 +95,7 @@
 		return v
 	}
 
-	// op == opUnify
+	// isUnify
 
 	// TODO: unify type masks.
 	if left == right {
@@ -108,13 +109,13 @@
 	}
 
 	if dl, ok := left.(*disjunction); ok {
-		return distribute(ctx, src, dl, right)
+		return distribute(ctx, src, op, dl, right)
 	} else if dr, ok := right.(*disjunction); ok {
-		return distribute(ctx, src, dr, left)
+		return distribute(ctx, src, op, dr, left)
 	}
 
 	if _, ok := right.(*unification); ok {
-		return right.binOp(ctx, src, opUnify, left)
+		return right.binOp(ctx, src, op, left)
 	}
 
 	// TODO: value may be incomplete if there is a cycle. Instead of an error
@@ -141,13 +142,13 @@
 // TODO: this is an exponential algorithm. There is no reason to have to
 // resolve this early. Revise this to only do early pruning but not a full
 // evaluation.
-func distribute(ctx *context, src source, x, y evaluated) evaluated {
+func distribute(ctx *context, src source, op op, x, y evaluated) evaluated {
 	dn := &disjunction{baseValue: src.base()}
-	dist(ctx, dn, false, mVal{x, true}, mVal{y, true})
+	dist(ctx, dn, false, op, mVal{x, true}, mVal{y, true})
 	return dn.normalize(ctx, src).val
 }
 
-func dist(ctx *context, d *disjunction, mark bool, x, y mVal) {
+func dist(ctx *context, d *disjunction, mark bool, op op, x, y mVal) {
 	if dx, ok := x.val.(*disjunction); ok {
 		if dx.hasDefaults {
 			mark = true
@@ -155,7 +156,7 @@
 		}
 		for _, dxv := range dx.values {
 			m := dxv.marked || !dx.hasDefaults
-			dist(ctx, d, mark, mVal{dxv.val.evalPartial(ctx), m}, y)
+			dist(ctx, d, mark, op, mVal{dxv.val.evalPartial(ctx), m}, y)
 		}
 		return
 	}
@@ -166,12 +167,12 @@
 		}
 		for _, dxy := range dy.values {
 			m := dxy.marked || !dy.hasDefaults
-			dist(ctx, d, mark, x, mVal{dxy.val.evalPartial(ctx), m})
+			dist(ctx, d, mark, op, x, mVal{dxy.val.evalPartial(ctx), m})
 		}
 		return
 	}
-	src := binSrc(token.NoPos, opUnify, x.val, y.val)
-	d.add(ctx, binOp(ctx, src, opUnify, x.val, y.val), mark && x.mark && y.mark)
+	src := binSrc(token.NoPos, op, x.val, y.val)
+	d.add(ctx, binOp(ctx, src, op, x.val, y.val), mark && x.mark && y.mark)
 }
 
 func (x *disjunction) binOp(ctx *context, src source, op op, other evaluated) evaluated {
@@ -331,7 +332,6 @@
 		k, _, msg := matchBinOpKind(opUnify, x.kind(), other.kind())
 		if k == bottomKind {
 			return ctx.mkErr(src, msg, opUnify, ctx.str(x), ctx.str(other), x.kind(), other.kind())
-			break
 		}
 		switch y := other.(type) {
 		case *basicType:
@@ -582,7 +582,8 @@
 
 func (x *structLit) binOp(ctx *context, src source, op op, other evaluated) evaluated {
 	y, ok := other.(*structLit)
-	if !ok || op != opUnify {
+	_, isUnify := op.unifyType()
+	if !ok || !isUnify {
 		return ctx.mkIncompatible(src, op, x, other)
 	}
 
@@ -594,7 +595,14 @@
 		return x
 	}
 	arcs := make(arcs, 0, len(x.arcs)+len(y.arcs))
-	obj := &structLit{binSrc(src.Pos(), op, x, other), x.emit, nil, nil, arcs, nil}
+	obj := &structLit{
+		binSrc(src.Pos(), op, x, other), // baseValue
+		x.emit,                          // emit
+		nil,                             // template
+		nil,                             // comprehensions
+		arcs,                            // arcs
+		nil,                             // attributes
+	}
 	defer ctx.pushForwards(x, obj, y, obj).popForwards()
 
 	tx, ex := evalLambda(ctx, x.template)
diff --git a/cue/errors.go b/cue/errors.go
index 12342c9..b8f8436 100644
--- a/cue/errors.go
+++ b/cue/errors.go
@@ -147,7 +147,7 @@
 func appendPositions(pos []token.Pos, src source) []token.Pos {
 	if src != nil {
 		if b, ok := src.(*binaryExpr); ok {
-			if b.op == opUnify {
+			if _, isUnify := b.op.unifyType(); isUnify {
 				pos = appendPositions(pos, b.left)
 				pos = appendPositions(pos, b.right)
 			}
diff --git a/cue/eval.go b/cue/eval.go
index 1aa390f..f0f5cfb 100644
--- a/cue/eval.go
+++ b/cue/eval.go
@@ -365,7 +365,7 @@
 	}
 	var left, right evaluated
 
-	if x.op != opUnify {
+	if _, isUnify := x.op.unifyType(); !isUnify {
 		ctx.incEvalDepth()
 		left = ctx.manifest(x.left)
 		right = ctx.manifest(x.right)
diff --git a/cue/export.go b/cue/export.go
index 3e804a3..8f5a9c6 100644
--- a/cue/export.go
+++ b/cue/export.go
@@ -352,104 +352,7 @@
 		return bin
 
 	case *structLit:
-		obj := &ast.StructLit{}
-		if doEval(p.mode) {
-			x = x.expandFields(p.ctx)
-		}
-		for _, a := range x.arcs {
-			p.stack = append(p.stack, remap{
-				key:  x,
-				from: a.feature,
-				to:   nil,
-				syn:  obj,
-			})
-		}
-		if x.emit != nil {
-			obj.Elts = append(obj.Elts, &ast.EmbedDecl{Expr: p.expr(x.emit)})
-		}
-		if !doEval(p.mode) && x.template != nil {
-			l, ok := x.template.evalPartial(p.ctx).(*lambdaExpr)
-			if ok {
-				obj.Elts = append(obj.Elts, &ast.Field{
-					Label: &ast.TemplateLabel{
-						Ident: p.identifier(l.params.arcs[0].feature),
-					},
-					Value: p.expr(l.value),
-				})
-			} // TODO: else record error
-		}
-		for i, a := range x.arcs {
-			f := &ast.Field{
-				Label: p.label(a.feature),
-			}
-			// TODO: allow the removal of hidden fields. However, hidden fields
-			// that still used in incomplete expressions should not be removed
-			// (unless RequireConcrete is requested).
-			if a.optional {
-				// Optional fields are almost never concrete. We omit them in
-				// concrete mode to allow the user to use the -a option in eval
-				// without getting many errors.
-				if p.mode.omitOptional || p.mode.concrete {
-					continue
-				}
-				f.Optional = token.NoSpace.Pos()
-			}
-			if a.feature&hidden != 0 && p.mode.concrete && p.mode.omitHidden {
-				continue
-			}
-			if !doEval(p.mode) {
-				f.Value = p.expr(a.v)
-			} else {
-				e := x.at(p.ctx, i)
-				if v := p.ctx.manifest(e); isIncomplete(v) && !p.mode.concrete && isBottom(e) {
-					p := &exporter{p.ctx, options{raw: true}, p.stack, p.top, p.imports}
-					f.Value = p.expr(a.v)
-				} else {
-					f.Value = p.expr(e)
-				}
-			}
-			if a.attrs != nil && !p.mode.omitAttrs {
-				for _, at := range a.attrs.attr {
-					f.Attrs = append(f.Attrs, &ast.Attribute{Text: at.text})
-				}
-			}
-			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.ComprehensionDecl{Field: f, Clauses: clauses}
-					}
-					obj.Elts = append(obj.Elts, decl)
-					break
-				}
-
-				var y ast.Clause
-				y, next = p.clause(next)
-				clauses = append(clauses, y)
-			}
-		}
-		return obj
+		return p.structure(x)
 
 	case *fieldComprehension:
 		panic("should be handled in structLit")
@@ -607,6 +510,107 @@
 	}
 }
 
+func (p *exporter) structure(x *structLit) *ast.StructLit {
+	obj := &ast.StructLit{}
+	if doEval(p.mode) {
+		x = x.expandFields(p.ctx)
+	}
+	for _, a := range x.arcs {
+		p.stack = append(p.stack, remap{
+			key:  x,
+			from: a.feature,
+			to:   nil,
+			syn:  obj,
+		})
+	}
+	if x.emit != nil {
+		obj.Elts = append(obj.Elts, &ast.EmbedDecl{Expr: p.expr(x.emit)})
+	}
+	if !doEval(p.mode) && x.template != nil {
+		l, ok := x.template.evalPartial(p.ctx).(*lambdaExpr)
+		if ok {
+			obj.Elts = append(obj.Elts, &ast.Field{
+				Label: &ast.TemplateLabel{
+					Ident: p.identifier(l.params.arcs[0].feature),
+				},
+				Value: p.expr(l.value),
+			})
+		} // TODO: else record error
+	}
+	for i, a := range x.arcs {
+		f := &ast.Field{
+			Label: p.label(a.feature),
+		}
+		// TODO: allow the removal of hidden fields. However, hidden fields
+		// that still used in incomplete expressions should not be removed
+		// (unless RequireConcrete is requested).
+		if a.optional {
+			// Optional fields are almost never concrete. We omit them in
+			// concrete mode to allow the user to use the -a option in eval
+			// without getting many errors.
+			if p.mode.omitOptional || p.mode.concrete {
+				continue
+			}
+			f.Optional = token.NoSpace.Pos()
+		}
+		if a.feature&hidden != 0 && p.mode.concrete && p.mode.omitHidden {
+			continue
+		}
+		if !doEval(p.mode) {
+			f.Value = p.expr(a.v)
+		} else {
+			e := x.at(p.ctx, i)
+			if v := p.ctx.manifest(e); isIncomplete(v) && !p.mode.concrete && isBottom(e) {
+				p := &exporter{p.ctx, options{raw: true}, p.stack, p.top, p.imports}
+				f.Value = p.expr(a.v)
+			} else {
+				f.Value = p.expr(e)
+			}
+		}
+		if a.attrs != nil && !p.mode.omitAttrs {
+			for _, at := range a.attrs.attr {
+				f.Attrs = append(f.Attrs, &ast.Attribute{Text: at.text})
+			}
+		}
+		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.ComprehensionDecl{Field: f, Clauses: clauses}
+				}
+				obj.Elts = append(obj.Elts, decl)
+				break
+			}
+
+			var y ast.Clause
+			y, next = p.clause(next)
+			clauses = append(clauses, y)
+		}
+	}
+	return obj
+}
+
 // quote quotes the given string.
 func quote(str string, quote byte) string {
 	if strings.IndexByte(str, '\n') < 0 {
diff --git a/cue/op.go b/cue/op.go
index 8becafc..e13f965 100644
--- a/cue/op.go
+++ b/cue/op.go
@@ -133,6 +133,10 @@
 	return opEql <= op && op <= opGeq
 }
 
+func (op op) unifyType() (unchecked, ok bool) {
+	return false, op == opUnify
+}
+
 type op uint16
 
 const (
diff --git a/cue/value.go b/cue/value.go
index f8b6479..cfcee96 100644
--- a/cue/value.go
+++ b/cue/value.go
@@ -742,11 +742,9 @@
 	emit := x.emit
 	template := x.template
 	newArcs := []arc{}
-	optional := false
 
 	for _, c := range comprehensions {
 		result := c.clauses.yield(ctx, func(k, v evaluated, opt bool) *bottom {
-			optional = opt
 			if !k.kind().isAnyOf(stringKind) {
 				return ctx.mkErr(k, "key must be of type string")
 			}
@@ -767,7 +765,11 @@
 					return nil
 				}
 			}
-			newArcs = append(newArcs, arc{feature: f, v: v})
+			newArcs = append(newArcs, arc{
+				feature:  f,
+				optional: opt,
+				v:        v,
+			})
 			return nil
 		})
 		switch {
@@ -785,7 +787,14 @@
 	arcs := make([]arc, 0, len(x.arcs)+len(newArcs))
 	arcs = append(arcs, x.arcs...)
 	orig := x
-	x = &structLit{x.baseValue, emit, template, nil, arcs, nil}
+	x = &structLit{
+		x.baseValue, // baseValue
+		emit,        // emit
+		template,    // template
+		nil,         // comprehensions
+		arcs,        // arcs
+		nil,         // attributes
+	}
 	x.expanded = x
 	orig.expanded = x
 
@@ -799,13 +808,17 @@
 						ctx.labelStr(f))
 				} else {
 					x.arcs[i].v = mkBin(ctx, x.Pos(), opUnify, a.v, na.v)
-					x.arcs[i].optional = x.arcs[i].optional && optional
+					x.arcs[i].optional = x.arcs[i].optional && na.optional
 					x.arcs[i].docs = mergeDocs(na.docs, a.docs)
 				}
 				continue outer
 			}
 		}
-		x.arcs = append(x.arcs, arc{feature: f, optional: optional, v: na.v})
+		x.arcs = append(x.arcs, arc{
+			feature:  f,
+			optional: na.optional,
+			v:        na.v,
+		})
 	}
 	sort.Stable(x)
 	return x