Skip to content

Commit

Permalink
internal/core/adt: avoid loops in debug printing
Browse files Browse the repository at this point in the history
Printing adt values can cause infinite loops with structure
sharing enabled. This is especially the case for partially
evaluated Vertices, if a shared Vertex is printed for which
the structural cycle is not yet detected.

We use the recently added cycle detection mechanism to
whether a node is cyclic and/or shared in a Vertex.
This is a rather conservative method and may mark false
positives. This is okay as it serves the purpose of
avoiding printing cycles only and is not otherwise
involved in operations.

We could enabled printing with structure sharing in more
tests, but this would produce rather large diffs. We could
still consider this later, especially once the old
evaluator has been removed and the output from the new
evaluator become the new golden files.

Issue #2884

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: Icc5641446c8b5b9a4d4f67b584ccaa7114939890
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1193845
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
  • Loading branch information
mpvl committed Apr 27, 2024
1 parent 831374c commit bae8bbf
Show file tree
Hide file tree
Showing 12 changed files with 319 additions and 295 deletions.
74 changes: 58 additions & 16 deletions cue/testdata/benchmarks/listdisj.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -64,26 +64,16 @@ Retain: 0
Unifications: 15
Conjuncts: 43
Disjuncts: 4
-- out/eval --
-- out/evalalpha --
(struct){
#T: (list){
0: (string){ "d" }
}
x: (list){
0: (string){ "d" }
}
y: (list){
0: (string){ "d" }
}
z: (list){
0: (string){ "d" }
}
v: (list){
0: (string){ "d" }
}
#X: (list){
0: (string){ "d" }
}
x: ~(#T)
y: ~(#T)
z: ~(#T)
v: ~(#T)
#X: ~(#T)
}
-- diff/-out/evalalpha/stats<==>+out/eval/stats --
diff old new
Expand All @@ -106,6 +96,58 @@ diff old new
+Unifications: 15
+Conjuncts: 43
+Disjuncts: 4
-- diff/-out/evalalpha<==>+out/eval --
diff old new
--- old
+++ new
@@ -2,19 +2,9 @@
#T: (list){
0: (string){ "d" }
}
- x: (list){
- 0: (string){ "d" }
- }
- y: (list){
- 0: (string){ "d" }
- }
- z: (list){
- 0: (string){ "d" }
- }
- v: (list){
- 0: (string){ "d" }
- }
- #X: (list){
- 0: (string){ "d" }
- }
+ x: ~(#T)
+ y: ~(#T)
+ z: ~(#T)
+ v: ~(#T)
+ #X: ~(#T)
}
-- out/eval --
(struct){
#T: (list){
0: (string){ "d" }
}
x: (list){
0: (string){ "d" }
}
y: (list){
0: (string){ "d" }
}
z: (list){
0: (string){ "d" }
}
v: (list){
0: (string){ "d" }
}
#X: (list){
0: (string){ "d" }
}
}
-- diff/explanation --
Differences fully due to rendered structure sharing.
-- out/eval/stats --
Leaks: 0
Freed: 447
Expand Down
116 changes: 26 additions & 90 deletions cue/testdata/cycle/constraints.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -664,15 +664,7 @@ Result:
// [structural cycle]
t1: (_|_){
// [structural cycle]
a: (_|_){
// [structural cycle]
b: (_|_){
// [structural cycle]
b: (_|_){
// [structural cycle] selfTriggerCycle.t1.#T.b.b: structural cycle
}
}
}
a: ~(selfTriggerCycle.t1.#T)
#T: (_|_){
// [structural cycle]
b: (_|_){
Expand Down Expand Up @@ -735,49 +727,16 @@ Result:
link: (#struct){
a: (#struct){
two: (#struct){
link: (_|_){
// [structural cycle]
a: (_|_){
// [structural cycle]
b: (_|_){
// [structural cycle]
link: (_|_){
// [structural cycle] selfTriggerCycle.issue1503.#T.a.b.link: structural cycle
}
}
}
}
link: ~(selfTriggerCycle.issue1503.#T)
}
b: (#struct){
link: (_|_){
// [structural cycle]
a: (_|_){
// [structural cycle]
b: (_|_){
// [structural cycle]
link: (_|_){
// [structural cycle] selfTriggerCycle.issue1503.#T.a.b.link: structural cycle
}
}
}
}
link: ~(selfTriggerCycle.issue1503.#T)
}
}
}
}
b: (#struct){
link: (_|_){
// [structural cycle]
a: (_|_){
// [structural cycle]
b: (_|_){
// [structural cycle]
link: (_|_){
// [structural cycle] selfTriggerCycle.issue1503.#T.a.b.link: structural cycle
}
}
}
}
link: ~(selfTriggerCycle.issue1503.#T)
}
}
}
Expand Down Expand Up @@ -854,17 +813,21 @@ diff old new
}
}
}
@@ -222,6 +222,9 @@
@@ -218,12 +218,7 @@
// [structural cycle]
t1: (_|_){
// [structural cycle]
- a: (_|_){
- // [structural cycle]
- b: (_|_){
- // [structural cycle]
- }
- }
+ a: ~(selfTriggerCycle.t1.#T)
#T: (_|_){
// [structural cycle]
b: (_|_){
// [structural cycle]
+ b: (_|_){
+ // [structural cycle] selfTriggerCycle.t1.#T.b.b: structural cycle
+ }
}
}
#T: (_|_){
@@ -280,22 +283,52 @@
@@ -280,25 +275,22 @@
}
issue1503: (_|_){
// [structural cycle]
Expand All @@ -884,55 +847,28 @@ diff old new
- // [structural cycle]
- a: (struct){
- two: (struct){
- }
- }
- }
+ a: (#struct){
+ a: (#struct){
+ one: (#struct){
+ link: (#struct){
+ a: (#struct){
+ two: (#struct){
+ link: (_|_){
+ // [structural cycle]
+ a: (_|_){
+ // [structural cycle]
+ b: (_|_){
+ // [structural cycle]
+ link: (_|_){
+ // [structural cycle] selfTriggerCycle.issue1503.#T.a.b.link: structural cycle
+ }
+ }
+ }
+ }
+ link: ~(selfTriggerCycle.issue1503.#T)
+ }
+ b: (#struct){
+ link: (_|_){
+ // [structural cycle]
+ a: (_|_){
+ // [structural cycle]
+ b: (_|_){
+ // [structural cycle]
+ link: (_|_){
+ // [structural cycle] selfTriggerCycle.issue1503.#T.a.b.link: structural cycle
+ }
+ }
+ }
+ }
+ link: ~(selfTriggerCycle.issue1503.#T)
+ }
+ }
+ }
+ }
+ b: (#struct){
+ link: (_|_){
+ // [structural cycle]
+ a: (_|_){
+ // [structural cycle]
+ b: (_|_){
+ // [structural cycle]
+ link: (_|_){
+ // [structural cycle] selfTriggerCycle.issue1503.#T.a.b.link: structural cycle
+ }
}
}
}
+ link: ~(selfTriggerCycle.issue1503.#T)
}
}
}
-- diff/todo/p2 --
issue1503: cycle detected too late (but not super late)
selfTriggerCycle.t1.a.b.b: cycle detected slightly too late
Expand Down

0 comments on commit bae8bbf

Please sign in to comment.