cue: Reference handles concrete values for index operations
Also fixes
- some caching and binding issues
related to comprehensions.
- issues with comprehensions introduced with relaxed
closed struct semantics
Change-Id: I55277a6ee9c24b9c9641166bb79942762d44d06c
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/4020
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cue/ast.go b/cue/ast.go
index 178d3be..4b9b193 100644
--- a/cue/ast.go
+++ b/cue/ast.go
@@ -328,7 +328,7 @@
v.errf(n, "comprehension must be struct")
}
yielder.value = v.walk(n.Value)
- v.object.comprehensions = append(v.object.comprehensions, sc)
+ v.object.comprehensions = append(v.object.comprehensions, compValue{comp: sc})
case *ast.Field:
opt := n.Optional != token.NoPos
@@ -371,7 +371,7 @@
doc: leftOverDoc,
attrs: attrs,
}
- v.object.comprehensions = append(v.object.comprehensions, fc)
+ v.object.comprehensions = append(v.object.comprehensions, compValue{comp: fc})
case *ast.ListLit:
if len(x.Elts) != 1 {
diff --git a/cue/binop.go b/cue/binop.go
index 9c7c1fe..b880159 100644
--- a/cue/binop.go
+++ b/cue/binop.go
@@ -646,12 +646,18 @@
// we need to apply the template to all elements.
sz := len(x.comprehensions) + len(y.comprehensions)
- obj.comprehensions = make([]value, sz)
+ obj.comprehensions = make([]compValue, sz)
for i, c := range x.comprehensions {
- obj.comprehensions[i] = ctx.copy(c)
+ obj.comprehensions[i] = compValue{
+ checked: c.checked || (!unchecked && y.isClosed()),
+ comp: ctx.copy(c.comp),
+ }
}
for i, c := range y.comprehensions {
- obj.comprehensions[i+len(x.comprehensions)] = ctx.copy(c)
+ obj.comprehensions[i+len(x.comprehensions)] = compValue{
+ checked: c.checked || (!unchecked && x.isClosed()),
+ comp: ctx.copy(c.comp),
+ }
}
for _, a := range x.arcs {
@@ -666,6 +672,8 @@
if a.optional {
continue
}
+ // TODO: pass position of key, not value. Currently does not have
+ // a position.
return ctx.mkErr(a.v, a.v, "field %q not allowed in closed struct",
ctx.labelStr(a.feature))
}
@@ -709,6 +717,8 @@
if a.optional {
continue
}
+ // TODO: pass position of key, not value. Currently does not have a
+ // position.
return ctx.mkErr(a.v, x, "field %q not allowed in closed struct",
ctx.labelStr(a.feature))
}
diff --git a/cue/copy.go b/cue/copy.go
index fff4342..4698588 100644
--- a/cue/copy.go
+++ b/cue/copy.go
@@ -53,9 +53,9 @@
arcs[i] = a
}
- comp := make([]value, len(x.comprehensions))
+ comp := make([]compValue, len(x.comprehensions))
for i, c := range x.comprehensions {
- comp[i] = ctx.copy(c)
+ comp[i] = compValue{c.checked, ctx.copy(c.comp)}
}
obj.comprehensions = comp
return obj, false
diff --git a/cue/debug.go b/cue/debug.go
index e855f7f..17d8f2f 100644
--- a/cue/debug.go
+++ b/cue/debug.go
@@ -340,7 +340,10 @@
}
p.str(x.arcs)
for i, c := range x.comprehensions {
- p.str(c)
+ if c.checked {
+ p.write("c:")
+ }
+ p.str(c.comp)
if i < len(x.comprehensions)-1 {
p.write(", ")
}
diff --git a/cue/eval.go b/cue/eval.go
index 01051f1..f90aa64 100644
--- a/cue/eval.go
+++ b/cue/eval.go
@@ -18,6 +18,22 @@
"bytes"
)
+type resolver interface {
+ reference(ctx *context) value
+}
+
+var _ resolver = &selectorExpr{}
+var _ resolver = &indexExpr{}
+
+func resolveReference(ctx *context, v value) evaluated {
+ if r, ok := v.(resolver); ok {
+ if st, ok := r.reference(ctx).(*structLit); ok {
+ return st
+ }
+ }
+ return v.evalPartial(ctx)
+}
+
func eval(idx *index, v value) evaluated {
ctx := idx.newContext()
return v.evalPartial(ctx)
@@ -74,8 +90,42 @@
// TODO: mention x.x in error message?
return ctx.mkErr(x, "undefined field %q", field)
}
- // TODO: do we need to evaluate here?
- return n.cache.evalPartial(ctx)
+ return n.cache
+ }
+ return e.err(&selectorExpr{x.baseValue, v, x.feature})
+}
+
+func (x *selectorExpr) reference(ctx *context) (result value) {
+ if ctx.trace {
+ defer uni(indent(ctx, "selectorExpr", x))
+ defer func() { ctx.debugPrint("result:", result) }()
+ }
+
+ e := newEval(ctx, true)
+
+ const msgType = "invalid operation: %[5]s (type %[3]s does not support selection)"
+ v := e.eval(x.x, structKind|lambdaKind, msgType, x)
+
+ if e.is(v, structKind|lambdaKind, "") {
+ sc, ok := v.(scope)
+ if !ok {
+ return ctx.mkErr(x, "invalid subject to selector (found %v)", v.kind())
+ }
+ n := sc.lookup(ctx, x.feature)
+ if n.optional {
+ field := ctx.labelStr(x.feature)
+ return ctx.mkErr(x, codeIncomplete, "field %q is optional", field)
+ }
+ if n.val() == nil {
+ field := ctx.labelStr(x.feature)
+ if st, ok := sc.(*structLit); ok && !st.isClosed() {
+ return ctx.mkErr(x, codeIncomplete, "undefined field %q", field)
+ }
+ // m.foo undefined (type map[string]bool has no field or method foo)
+ // TODO: mention x.x in error message?
+ return ctx.mkErr(x, "undefined field %q", field)
+ }
+ return n.v
}
return e.err(&selectorExpr{x.baseValue, v, x.feature})
}
@@ -129,6 +179,65 @@
return e.err(&indexExpr{x.baseValue, val, index})
}
+func (x *indexExpr) reference(ctx *context) (result value) {
+ if ctx.trace {
+ defer uni(indent(ctx, "indexExpr", x))
+ defer func() { ctx.debugPrint("result:", result) }()
+ }
+
+ e := newEval(ctx, true)
+
+ const msgType = "invalid operation: %[5]s (type %[3]s does not support indexing)"
+ const msgIndexType = "invalid %[5]s index %[1]s (type %[3]s)"
+
+ val := e.eval(x.x, listKind|structKind, msgType, x)
+ k := val.kind()
+ index := e.eval(x.index, stringKind|intKind, msgIndexType, k)
+
+ switch v := val.(type) {
+ case *structLit:
+ if e.is(index, stringKind, msgIndexType, k) {
+ s := index.strValue()
+ // TODO: must lookup
+ n := v.lookup(ctx, ctx.strLabel(s))
+ if n.definition {
+ return ctx.mkErr(x, index,
+ "field %q is a definition", s)
+ }
+ if n.optional {
+ return ctx.mkErr(x, index, codeIncomplete, "field %q is optional", s)
+ }
+ if n.val() == nil {
+ if !v.isClosed() {
+ return ctx.mkErr(x, index, codeIncomplete, "undefined field %q", s)
+ }
+ return ctx.mkErr(x, index, "undefined field %q", s)
+ }
+ return n.v
+ }
+ case *list:
+ if e.is(index, intKind, msgIndexType, k) {
+ i := index.(*numLit).intValue(ctx)
+ if i < 0 {
+ const msg = "invalid %[4]s index %[1]s (index must be non-negative)"
+ return e.mkErr(x.index, index, 0, k, msg)
+ }
+ return v.iterAt(ctx, i).v
+ }
+
+ case atter:
+ if e.is(index, intKind, msgIndexType, k) {
+ i := index.(*numLit).intValue(ctx)
+ if i < 0 {
+ const msg = "invalid %[4]s index %[1]s (index must be non-negative)"
+ return e.mkErr(x.index, index, 0, k, msg)
+ }
+ return v.at(ctx, i)
+ }
+ }
+ return e.err(&indexExpr{x.baseValue, val, index})
+}
+
// Composit
func (x *sliceExpr) evalPartial(ctx *context) (result evaluated) {
@@ -306,7 +415,7 @@
func (x *fieldComprehension) evalPartial(ctx *context) evaluated {
k := x.key.evalPartial(ctx)
- v := x.val.evalPartial(ctx)
+ v := x.val
if err := firstBottom(k, v); err != nil {
return err
}
@@ -321,7 +430,7 @@
func (x *closeIfStruct) evalPartial(ctx *context) evaluated {
v := x.value.evalPartial(ctx)
- updateCloseStatus(ctx, v)
+ v = updateCloseStatus(ctx, v)
return v
}
@@ -450,8 +559,8 @@
}
}
} else {
- left = x.left.evalPartial(ctx)
- right = x.right.evalPartial(ctx)
+ left = resolveReference(ctx, x.left)
+ right = resolveReference(ctx, x.right)
if err := cycleError(left); err != nil && ctx.inSum == 0 && right.kind().isAtom() {
return ctx.delayConstraint(right, x)
diff --git a/cue/export.go b/cue/export.go
index c3d2c84..2e2ca40 100644
--- a/cue/export.go
+++ b/cue/export.go
@@ -809,7 +809,7 @@
}
for _, v := range x.comprehensions {
- switch c := v.(type) {
+ switch c := v.comp.(type) {
case *fieldComprehension:
l := p.expr(c.key)
label, _ := l.(ast.Label)
diff --git a/cue/resolve_test.go b/cue/resolve_test.go
index 2a1ece3..da385ee 100644
--- a/cue/resolve_test.go
+++ b/cue/resolve_test.go
@@ -1484,12 +1484,12 @@
}
`,
out: `<0>{` +
- `E :: _|_(int:field "f3" not allowed in closed struct), ` +
- `A :: <1>C{f1: int, f2: int}, ` +
- `a: _|_(int:field "f3" not allowed in closed struct), ` +
- `B :: <2>C{f1: int}, ` +
- `C :: <3>C{f1: int}, ` +
- `D :: <4>{f1: int, ...}` +
+ `E :: _|_(<1>.v:field "f3" not allowed in closed struct), ` +
+ `A :: <2>C{f1: int, f2: int}, ` +
+ `a: _|_(<3>.v:field "f3" not allowed in closed struct), ` +
+ `B :: <4>C{f1: int}, ` +
+ `C :: <5>C{f1: int}, ` +
+ `D :: <6>{f1: int, ...}` +
`}`,
}, {
desc: "incomplete comprehensions",
@@ -2603,7 +2603,7 @@
}]
}
`,
- out: `<0>{<1>{listOfCloseds: [_|_(2:field "b" not allowed in closed struct)]}, Foo: <2>{listOfCloseds: []}, Closed :: <3>C{a: 0}, Junk: <4>{b: 2}}`,
+ out: `<0>{<1>{listOfCloseds: [_|_(<2>.v:field "b" not allowed in closed struct)]}, Foo: <3>{listOfCloseds: []}, Closed :: <4>C{a: 0}, Junk: <5>{b: 2}}`,
}, {
desc: "label and field aliases",
in: `
@@ -2750,6 +2750,37 @@
foo: Task & {"op": "pull"}
`,
out: `<0>{Task :: (<1>C{op: "pull", tag: (*"latest" | string), refToTag: <1>.tag, tagExpr: (<1>.tag + "dd"), tagInString: ""+<1>.tag+""} | <2>C{op: "scratch"}), foo: <3>C{op: "pull", tag: "latest", refToTag: "latest", tagExpr: "latestdd", tagInString: "latest"}}`,
+ }, {
+ in: `
+ t: {
+ ok :: *true | bool
+ if ok {
+ x: int
+ }
+ }
+ s: t & {
+ ok :: false
+ }`,
+ out: `<0>{t: <1>{ok :: true, x: int}, s: <2>{ok :: false}}`,
+ }, {
+ // TODO(eval): fix: c should ultimately be allowed the struct. Current
+ // semantics require, however, that generated fields are not available
+ // for evaluation. This, however, does not have to hold, for closedness
+ // checks and allowing this would be more intuitive.
+ // Until that time, ensure that the behavior is at commutative.
+ in: `
+ a :: {
+ if b {
+ c: 4
+ }
+ b: bool
+ }
+ x: (a & { b: true}) & {c: 4 }
+ y: x
+ `,
+ // c should not be allowed, as it would break commutativiy.
+ // See comments above.
+ out: `<0>{a :: <1>C{b: bool if <2>.b yield <3>C{c: 4}}, x: _|_(4:field "c" not allowed in closed struct), y: _|_(4:field "c" not allowed in closed struct)}`,
}}
rewriteHelper(t, testCases, evalFull)
}
diff --git a/cue/types.go b/cue/types.go
index bb2a2ee..ad16023 100644
--- a/cue/types.go
+++ b/cue/types.go
@@ -1349,27 +1349,56 @@
// Reference returns the instance and path referred to by this value such that
// inst.Lookup(path) resolves to the same value, or no path if this value is not
-// a reference,
+// a reference. If a reference contains index selection (foo[bar]), it will
+// only return a reference if the index resolves to a concrete value.
func (v Value) Reference() (inst *Instance, path []string) {
// TODO: don't include references to hidden fields.
if v.path == nil {
return nil, nil
}
- sel, ok := v.path.v.(*selectorExpr)
- if !ok {
+ ctx := v.ctx()
+ var x value
+ var feature string
+ switch sel := v.path.v.(type) {
+ case *selectorExpr:
+ x = sel.x
+ feature = ctx.labelStr(sel.feature)
+
+ case *indexExpr:
+ e := sel.index.evalPartial(ctx)
+ s, ok := e.(*stringLit)
+ if !ok {
+ return nil, nil
+ }
+ x = sel.x
+ feature = s.str
+
+ default:
return nil, nil
}
- imp, a := mkPath(v.ctx(), v.path, sel, 0)
+ imp, a := mkPath(ctx, v.path, x, feature, 0)
return imp, a
}
-func mkPath(c *context, up *valueData, sel *selectorExpr, d int) (imp *Instance, a []string) {
- switch x := sel.x.(type) {
+func mkPath(c *context, up *valueData, x value, feature string, d int) (imp *Instance, a []string) {
+ switch x := x.(type) {
case *selectorExpr:
- imp, a = mkPath(c, up.parent, x, d+1)
+ imp, a = mkPath(c, up, x.x, c.labelStr(x.feature), d+1)
if imp == nil {
return nil, nil
}
+
+ case *indexExpr:
+ e := x.index.evalPartial(c)
+ s, ok := e.(*stringLit)
+ if !ok {
+ return nil, nil
+ }
+ imp, a = mkPath(c, up, x.x, s.str, d+1)
+ if imp == nil {
+ return nil, nil
+ }
+
case *nodeRef:
// the parent must exist.
for ; up != nil && up.cache != x.node.(value); up = up.parent {
@@ -1379,11 +1408,11 @@
if v == nil {
v = x.node
}
- imp = c.getImportFromNode(x.node)
+ imp = c.getImportFromNode(v)
default:
return nil, nil
}
- return imp, append(a, c.labelStr(sel.feature))
+ return imp, append(a, feature)
}
func mkFromRoot(c *context, up *valueData, d int) (root value, a []string) {
diff --git a/cue/types_test.go b/cue/types_test.go
index 31eb4a6..b088a7b 100644
--- a/cue/types_test.go
+++ b/cue/types_test.go
@@ -1998,21 +1998,41 @@
input string
want string
}{{
- input: "v: _|_",
+ input: "v: w: x: _|_",
want: "",
}, {
- input: "v: 2",
+ input: "v: w: x: 2",
want: "",
}, {
- input: "v: a, a: 1",
+ input: "v: w: x: a, a: 1",
want: "a",
}, {
- input: "v: a.b.c, a b c: 1",
+ input: "v: w: x: a.b.c, a: b: c: 1",
want: "a b c",
+ }, {
+ input: "v: w: x: w.a.b.c, v: w: a: b: c: 1",
+ want: "v w a b c",
+ }, {
+ input: `v: w: x: w.a.b.c, v: w: a: b: c: 1, D :: 3, opt?: 3, "v\(D)": 3, X: {a: 3}, X`,
+ want: "v w a b c",
+ }, {
+ input: `v: w: x: w.a[bb]["c"], v: w: a: b: c: 1, bb: "b"`,
+ want: "v w a b c",
+ }, {
+ input: `v: {
+ for t in src {
+ w: "t\(t)": 1
+ w: "\(t)": w["t\(t)"]
+ }
+ },
+ src: ["x", "y"]`,
+ want: "v w tx",
}}
for _, tc := range testCases {
t.Run("", func(t *testing.T) {
- v := getInstance(t, tc.input).Lookup("v")
+ var r Runtime
+ inst, _ := r.Compile("in", tc.input) // getInstance(t, tc.input)
+ v := inst.Lookup("v", "w", "x")
inst, a := v.Reference()
if got := strings.Join(a, " "); got != tc.want {
t.Errorf("\n got %v;\nwant %v", got, tc.want)
diff --git a/cue/value.go b/cue/value.go
index 7ad98e7..858fa44 100644
--- a/cue/value.go
+++ b/cue/value.go
@@ -642,13 +642,26 @@
optionals *optionals
closeStatus closeMode
- comprehensions []value
+ comprehensions []compValue
// TODO: consider hoisting the template arc to its own value.
arcs []arc
expanded evaluated
}
+// compValue is a temporary stop-gap until the correct unification algorithm is
+// implemented. This implementation is more strict than should be. When two
+// structs, of which at least one is closed, are unified, the fields resolving
+// later from unresolved comprehensions should match the closedness constraints.
+// To relax this constraint, unification could follow the lines of
+// traditional unification with bookkeeping of which fields are
+// allowed, to be applied as constraints after full unification.
+
+type compValue struct {
+ checked bool
+ comp value
+}
+
// optionals holds a set of key pattern-constraint pairs, where constraints are
// to be applied to concrete fields of which the label matches the key pattern.
//
@@ -1045,7 +1058,13 @@
// it is safe to cache the result.
ctx.cycleErr = false
- updateCloseStatus(ctx, v)
+ v = updateCloseStatus(ctx, v)
+ if st, ok := v.(*structLit); ok {
+ v, err = st.expandFields(ctx)
+ if err != nil {
+ v = err
+ }
+ }
x.arcs[i].cache = v
if doc != nil {
x.arcs[i].docs = &docNode{left: doc, right: x.arcs[i].docs}
@@ -1083,15 +1102,17 @@
comprehensions := x.comprehensions
- var incomplete []value
+ var incomplete []compValue
var n evaluated = &top{x.baseValue}
if x.emit != nil {
n = x.emit.evalPartial(ctx)
}
+ var checked evaluated = &top{x.baseValue}
+
for _, x := range comprehensions {
- v := x.evalPartial(ctx)
+ v := x.comp.evalPartial(ctx)
if v, ok := v.(*bottom); ok {
if isIncomplete(v) {
incomplete = append(incomplete, x)
@@ -1100,27 +1121,41 @@
return nil, v
}
- src := binSrc(x.Pos(), opUnify, x, v)
- n = binOp(ctx, src, opUnifyUnchecked, n, v)
+ src := binSrc(x.comp.Pos(), opUnify, x.comp, v)
+ _ = checked
+ if x.checked {
+ checked = binOp(ctx, src, opUnifyUnchecked, checked, v)
+ } else {
+ n = binOp(ctx, src, opUnifyUnchecked, n, v)
+ }
+ }
+ if len(comprehensions) == len(incomplete) {
+ return x, nil
}
- src := binSrc(x.Pos(), opUnify, x, n)
switch n.(type) {
- case *bottom:
+ case *bottom, *top:
case *structLit:
- orig := *x
- orig.comprehensions = incomplete
- orig.emit = nil
- n = binOp(ctx, src, opUnify, &orig, n)
+ orig := x.comprehensions
+ x.comprehensions = incomplete
+ src := binSrc(x.Pos(), opUnify, x, n)
+ n = binOp(ctx, src, opUnifyUnchecked, x, n)
+ x.comprehensions = orig
default:
- if len(comprehensions) == len(incomplete) {
- return x, nil
- }
- if x.emit != nil {
- v := x.emit.evalPartial(ctx)
- n = binOp(ctx, src, opUnifyUnchecked, v, n)
- }
+ return nil, ctx.mkErr(x, n, "invalid embedding")
+ }
+
+ switch checked.(type) {
+ case *bottom, *top:
+ case *structLit:
+ orig := x.comprehensions
+ x.comprehensions = incomplete
+ src := binSrc(x.Pos(), opUnify, n, checked)
+ n = binOp(ctx, src, opUnify, x, checked)
+ x.comprehensions = orig
+
+ default:
return nil, ctx.mkErr(x, n, "invalid embedding")
}
@@ -1152,7 +1187,7 @@
}
if x.closeStatus != 0 {
- updateCloseStatus(ctx, v)
+ v = updateCloseStatus(ctx, v)
}
return v, doc
}
@@ -1240,8 +1275,12 @@
switch x := v.(type) {
case *top:
return v
- case *structLit, *list, *disjunction:
- updateCloseStatus(ctx, v)
+ case *structLit:
+ v = updateCloseStatus(ctx, x)
+ case *list:
+ v = updateCloseStatus(ctx, x)
+ case *disjunction:
+ v = updateCloseStatus(ctx, x)
case *closeIfStruct:
return x
}
@@ -1250,18 +1289,15 @@
return v
}
-func updateCloseStatus(ctx *context, v value) {
+func updateCloseStatus(ctx *context, v evaluated) evaluated {
switch x := v.(type) {
case *structLit:
- y, err := x.expandFields(ctx)
- if err == nil {
- if x.closeStatus.shouldClose() {
- x.closeStatus = isClosed
- y.optionals = y.optionals.close()
- }
- x.closeStatus |= shouldFinalize
- y.closeStatus = x.closeStatus
+ if x.closeStatus.shouldClose() {
+ x.closeStatus = isClosed
+ x.optionals = x.optionals.close()
}
+ x.closeStatus |= shouldFinalize
+ return x
case *disjunction:
for _, d := range x.values {
@@ -1274,6 +1310,7 @@
wrapFinalize(ctx, x.typ)
}
}
+ return v
}
// insertValue is used during initialization but never during evaluation.
