Skip to content

Commit

Permalink
internal/core/adt: fix spurious cycle
Browse files Browse the repository at this point in the history 
These manifested when unmodified fields in modified structs triggered
a cycle. This change adds code to detect new structure and foo cycles.

Fixes #754

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: I60a94b0eae6bab730be6292ebe697930cb01cd6c
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/542249
Reviewed-by: Roger Peppe <[email protected]>
Unity-Result: CUEcueckoo <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
  • Loading branch information
@mpvl
mpvl committed Aug 15, 2022
1 parent 6b3fd65 commit e187f9f
Show file tree
Hide file tree
Showing 2 changed files with 243 additions and 4 deletions.
198 changes: 195 additions & 3 deletions cue/testdata/cycle/structural.txtar
View file
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,45 @@ z1: {
z: x & y
}

// Cross references. Here there is no cycle, but the repeated instantiations
// of T will cause, in turn, cause repeated resolution of `x`. The algorithm
// should not identify these as cyclic references. Instead, it should be
// noted that the reference is each time referring to new content.
// Issue #754
crossRefNoCycle: t1: {
T: {
x: _
y: x
}
C: { x: T } & T
}

crossRefNoCycle: t2: {
T: {
x: _
y: x
}
C: T & { x: T }
}

crossRefNoCycle: t3: {
T: {
x: _
y: x
z: y
}
C: T & { x: T }
}

crossRefNoCycle: t4: {
T: {
x: _
y: x
z: y
}
C: T & { x: T & { x: T } }
}

// Ensure these are NOT treated as structural errors.

n1: a: b: int
Expand All @@ -460,7 +499,6 @@ Errors:
a1.f.0: structural cycle
a3.f.g: structural cycle
b13.root.a.0.0: structural cycle
b14.root.b.1.0: structural cycle
b14.root.b.1.1: structural cycle
b4.b.0: conflicting values 1 and [y] (mismatched types int and list):
./in.cue:53:5
Expand All @@ -484,6 +522,9 @@ b7.b.0.0: conflicting values 1 and [a] (mismatched types int and list):
./in.cue:69:6
b7.b.0.0.0: structural cycle
c1.a.c.c: structural cycle
crossRefNoCycle.t4.C.y.x: structural cycle
crossRefNoCycle.t4.C.y.y: structural cycle
crossRefNoCycle.t4.C.y.z: structural cycle
d1.a.b.c.d.t: structural cycle
d2.r.c.d.t: structural cycle
d2.x.d.t.c.d.t: structural cycle
Expand Down Expand Up @@ -861,8 +902,7 @@ Result:
}
1: (_|_){
// [structural cycle]
0: (_|_){
// [structural cycle] b14.root.b.1.0: structural cycle
0: (list){
}
1: (_|_){
// [structural cycle] b14.root.b.1.1: structural cycle
Expand Down Expand Up @@ -1501,6 +1541,110 @@ Result:
}
}
}
crossRefNoCycle: (_|_){
// [structural cycle]
t1: (struct){
T: (struct){
x: (_){ _ }
y: (_){ _ }
}
C: (struct){
x: (struct){
x: (_){ _ }
y: (_){ _ }
}
y: (struct){
x: (_){ _ }
y: (_){ _ }
}
}
}
t2: (struct){
T: (struct){
x: (_){ _ }
y: (_){ _ }
}
C: (struct){
x: (struct){
x: (_){ _ }
y: (_){ _ }
}
y: (struct){
x: (_){ _ }
y: (_){ _ }
}
}
}
t3: (struct){
T: (struct){
x: (_){ _ }
y: (_){ _ }
z: (_){ _ }
}
C: (struct){
x: (struct){
x: (_){ _ }
y: (_){ _ }
z: (_){ _ }
}
y: (struct){
x: (_){ _ }
y: (_){ _ }
z: (_){ _ }
}
z: (struct){
x: (_){ _ }
y: (_){ _ }
z: (_){ _ }
}
}
}
t4: (_|_){
// [structural cycle]
T: (struct){
x: (_){ _ }
y: (_){ _ }
z: (_){ _ }
}
C: (_|_){
// [structural cycle]
x: (struct){
x: (struct){
x: (_){ _ }
y: (_){ _ }
z: (_){ _ }
}
y: (struct){
x: (_){ _ }
y: (_){ _ }
z: (_){ _ }
}
z: (struct){
x: (_){ _ }
y: (_){ _ }
z: (_){ _ }
}
}
y: (_|_){
// [structural cycle]
x: (_|_){
// [structural cycle] crossRefNoCycle.t4.C.y.x: structural cycle
}
y: (_|_){
// [structural cycle] crossRefNoCycle.t4.C.y.y: structural cycle
}
z: (_|_){
// [structural cycle] crossRefNoCycle.t4.C.y.z: structural cycle
}
}
z: (_|_){
// [structural cycle] crossRefNoCycle.t4.C.y.x: structural cycle
// crossRefNoCycle.t4.C.y.y: structural cycle
// crossRefNoCycle.t4.C.y.z: structural cycle
}
}
}
}
n1: (struct){
a: (struct){
b: (int){ int }
Expand Down Expand Up @@ -2197,6 +2341,54 @@ Result:
}
z: (〈0;x〉 & 〈0;y〉)
}
crossRefNoCycle: {
t1: {
T: {
x: _
y: 〈0;x〉
}
C: ({
x: 〈1;T〉
} & 〈0;T〉)
}
}
crossRefNoCycle: {
t2: {
T: {
x: _
y: 〈0;x〉
}
C: (〈0;T〉 & {
x: 〈1;T〉
})
}
}
crossRefNoCycle: {
t3: {
T: {
x: _
y: 〈0;x〉
z: 〈0;y〉
}
C: (〈0;T〉 & {
x: 〈1;T〉
})
}
}
crossRefNoCycle: {
t4: {
T: {
x: _
y: 〈0;x〉
z: 〈0;y〉
}
C: (〈0;T〉 & {
x: (〈1;T〉 & {
x: 〈2;T〉
})
})
}
}
n1: {
a: {
b: int
Expand Down
49 changes: 48 additions & 1 deletion internal/core/adt/cycle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,21 @@ type CycleInfo struct {
// TODO(perf): pack this in with CloseInfo. Make an uint32 pointing into
// a buffer maintained in OpContext, using a mark-release mechanism.
Refs *RefNode

// Values is used to track the introduction of new values to invalidate
// potential cycles in case repeated uses of a reference are triggered
// by something that is not a cycle. For instance, consider
//
// T: {
// x: _
// y: x
// }
// C: { x: T } & T
//
// Here the repeated mixing in of T causes reference x to be repeatedly
// added to the same conjunct. However, as it keeps refering to fresh
// content, it is not a cycle.
Values *ValueNode
}

// A RefNode is a linked list of associated references.
Expand All @@ -229,6 +244,13 @@ type RefNode struct {
Depth int32
}

// ValueNode is a node of a linked list of expressions. See the Values field
// in CycleInfo for more info.
type ValueNode struct {
Expr Expr
Next *ValueNode
}

// cyclicConjunct is used in nodeContext to postpone the computation of
// cyclic conjuncts until a non-cyclic conjunct permits it to be processed.
type cyclicConjunct struct {
Expand Down Expand Up @@ -267,6 +289,31 @@ func (n *nodeContext) markCycle(arc *Vertex, v Conjunct, x Resolver) (_ Conjunct
}
depth = r.Depth
found = true

// We do not exclude references pointing to the same nodes as these
// are always cycles, and because excluding them would degrade
// performance too much.
if r.Arc == arc {
break
}

// This loop checks that the detected cycle is not postponed or
// invalidated by new values. See the comment in CloseInfo.Values.
outer:
for _, c := range arc.Conjuncts {
x := c.Expr()
for v := v.CloseInfo.Values; v != nil; v = v.Next {
if v.Expr == x {
continue outer
}
}
v.CloseInfo.Values = &ValueNode{
Next: v.CloseInfo.Values,
Expr: x,
}
found = false
}

break
}
}
Expand Down Expand Up @@ -297,8 +344,8 @@ func (n *nodeContext) markCycle(arc *Vertex, v Conjunct, x Resolver) (_ Conjunct
// Fix this by ensuring the root vertex starts with a depth of 1, for
// instance.
foundCycle := n.node.Parent == nil || depth == 0

foundNonCycle := false

if !foundCycle {
// Look for evidence of "new structure" to invalidate the cycle.
// This is done by checking for non-cyclic conjuncts between the
Expand Down

0 comments on commit e187f9f

Please sign in to comment.