From a8de61c6d5ede0ac2009371ac310a41fd374cd00 Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Fri, 22 Jan 2021 16:05:03 +0100 Subject: [PATCH] cue/testdata/cycle: fix another cycle bug The special case here is that the cycle passes over a reference into a builtin. Fixes #655 Change-Id: Ibc650932ccdf63aa3163a81bf6af101c7d942271 Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8283 Reviewed-by: CUE cueckoo Reviewed-by: Marcel van Lohuizen Reviewed-by: Paul Jolly --- cue/testdata/cycle/builtins.txtar | 179 ++++++++++++++++++++++++++++++ internal/core/adt/eval.go | 59 +++++----- 2 files changed, 213 insertions(+), 25 deletions(-) create mode 100644 cue/testdata/cycle/builtins.txtar diff --git a/cue/testdata/cycle/builtins.txtar b/cue/testdata/cycle/builtins.txtar new file mode 100644 index 000000000..0e4876682 --- /dev/null +++ b/cue/testdata/cycle/builtins.txtar @@ -0,0 +1,179 @@ +-- in.cue -- +import "regexp" + +// Issue #655 +// When evaluating a value into a struct, and then back into a value, the +// evaluation mode flips from Partial to AllArcs to Back. This is typically +// not an issue, but if a referred field is within a struct generated by a +// builtin, effectively the entire struct needs to be evaluated and special care +// should be taking to not evaluate too early. +builtinCyclePerm0: { + X: "example.com" + + Y: { + #components: regexp.FindNamedSubmatch(#"^(?P[[:alnum:].]+)$"#, X) + host: #components.host + } + + X: Y.host +} + +builtinCyclePerm1: { + X: Y.host + + Y: { + #components: regexp.FindNamedSubmatch(#"^(?P[[:alnum:].]+)$"#, X) + host: #components.host + } + + X: "example.com" +} + +builtinCyclePerm2: { + Y: { + #components: regexp.FindNamedSubmatch(#"^(?P[[:alnum:].]+)$"#, X) + host: #components.host + } + + X: Y.host + X: "example.com" +} + +builtinCyclePerm3: { + Y: { + #components: regexp.FindNamedSubmatch(#"^(?P[[:alnum:].]+)$"#, X) + host: #components.host + } + + X: "example.com" + X: Y.host +} + +builtinCyclePerm4: { + X: "example.com" + X: Y.host + + Y: { + #components: regexp.FindNamedSubmatch(#"^(?P[[:alnum:].]+)$"#, X) + host: #components.host + } +} + +builtinCyclePerm5: { + X: Y.host + X: "example.com" + + Y: { + #components: regexp.FindNamedSubmatch(#"^(?P[[:alnum:].]+)$"#, X) + host: #components.host + } +} +-- out/eval -- +(struct){ + builtinCyclePerm0: (struct){ + X: (string){ "example.com" } + Y: (struct){ + #components: (#struct){ + host: (string){ "example.com" } + } + host: (string){ "example.com" } + } + } + builtinCyclePerm1: (struct){ + X: (string){ "example.com" } + Y: (struct){ + #components: (#struct){ + host: (string){ "example.com" } + } + host: (string){ "example.com" } + } + } + builtinCyclePerm2: (struct){ + Y: (struct){ + #components: (#struct){ + host: (string){ "example.com" } + } + host: (string){ "example.com" } + } + X: (string){ "example.com" } + } + builtinCyclePerm3: (struct){ + Y: (struct){ + #components: (#struct){ + host: (string){ "example.com" } + } + host: (string){ "example.com" } + } + X: (string){ "example.com" } + } + builtinCyclePerm4: (struct){ + X: (string){ "example.com" } + Y: (struct){ + #components: (#struct){ + host: (string){ "example.com" } + } + host: (string){ "example.com" } + } + } + builtinCyclePerm5: (struct){ + X: (string){ "example.com" } + Y: (struct){ + #components: (#struct){ + host: (string){ "example.com" } + } + host: (string){ "example.com" } + } + } +} +-- out/compile -- +--- in.cue +{ + builtinCyclePerm0: { + X: "example.com" + Y: { + #components: 〈import;regexp〉.FindNamedSubmatch("^(?P[[:alnum:].]+)$", 〈1;X〉) + host: 〈0;#components〉.host + } + X: 〈0;Y〉.host + } + builtinCyclePerm1: { + X: 〈0;Y〉.host + Y: { + #components: 〈import;regexp〉.FindNamedSubmatch("^(?P[[:alnum:].]+)$", 〈1;X〉) + host: 〈0;#components〉.host + } + X: "example.com" + } + builtinCyclePerm2: { + Y: { + #components: 〈import;regexp〉.FindNamedSubmatch("^(?P[[:alnum:].]+)$", 〈1;X〉) + host: 〈0;#components〉.host + } + X: 〈0;Y〉.host + X: "example.com" + } + builtinCyclePerm3: { + Y: { + #components: 〈import;regexp〉.FindNamedSubmatch("^(?P[[:alnum:].]+)$", 〈1;X〉) + host: 〈0;#components〉.host + } + X: "example.com" + X: 〈0;Y〉.host + } + builtinCyclePerm4: { + X: "example.com" + X: 〈0;Y〉.host + Y: { + #components: 〈import;regexp〉.FindNamedSubmatch("^(?P[[:alnum:].]+)$", 〈1;X〉) + host: 〈0;#components〉.host + } + } + builtinCyclePerm5: { + X: 〈0;Y〉.host + X: "example.com" + Y: { + #components: 〈import;regexp〉.FindNamedSubmatch("^(?P[[:alnum:].]+)$", 〈1;X〉) + host: 〈0;#components〉.host + } + } +} diff --git a/internal/core/adt/eval.go b/internal/core/adt/eval.go index a194e9eb3..9f7d23328 100644 --- a/internal/core/adt/eval.go +++ b/internal/core/adt/eval.go @@ -264,7 +264,8 @@ func (e *Unifier) Unify(c *OpContext, v *Vertex, state VertexStatus) { n.doNotify() if !n.done() { - if len(n.disjunctions) > 0 && v.BaseValue == cycle { + switch { + case len(n.disjunctions) > 0 && v.BaseValue == cycle: // We disallow entering computations of disjunctions with // incomplete data. if state == Finalized { @@ -276,12 +277,16 @@ func (e *Unifier) Unify(c *OpContext, v *Vertex, state VertexStatus) { n.node.UpdateStatus(Partial) } return - } - } - if !n.done() && state <= Partial { - n.node.UpdateStatus(Partial) - return + case state <= Partial: + n.node.UpdateStatus(Partial) + return + + case state <= AllArcs: + c.AddBottom(n.incompleteErrors()) + n.node.UpdateStatus(Partial) + return + } } if s := v.Status(); state <= s { @@ -416,25 +421,7 @@ func (n *nodeContext) postDisjunct(state VertexStatus) { default: if n.node.BaseValue == cycle { if !n.done() { - // collect incomplete errors. - var err *Bottom // n.incomplete - for _, d := range n.dynamicFields { - err = CombineErrors(nil, err, d.err) - } - for _, c := range n.forClauses { - err = CombineErrors(nil, err, c.err) - } - for _, c := range n.ifClauses { - err = CombineErrors(nil, err, c.err) - } - for _, x := range n.exprs { - err = CombineErrors(nil, err, x.err) - } - if err == nil { - // safeguard. - err = incompleteSentinel - } - n.node.BaseValue = err + n.node.BaseValue = n.incompleteErrors() } else { n.node.BaseValue = nil } @@ -538,6 +525,28 @@ func (n *nodeContext) postDisjunct(state VertexStatus) { n.completeArcs(state) } +func (n *nodeContext) incompleteErrors() *Bottom { + // collect incomplete errors. + var err *Bottom // n.incomplete + for _, d := range n.dynamicFields { + err = CombineErrors(nil, err, d.err) + } + for _, c := range n.forClauses { + err = CombineErrors(nil, err, c.err) + } + for _, c := range n.ifClauses { + err = CombineErrors(nil, err, c.err) + } + for _, x := range n.exprs { + err = CombineErrors(nil, err, x.err) + } + if err == nil { + // safeguard. + err = incompleteSentinel + } + return err +} + func (n *nodeContext) completeArcs(state VertexStatus) { if state <= AllArcs {