this repo has no description
internal/core/adt: do not finalize before arcs are processed
Before this change, a node with a shared value might
be marked as finalized before all checks were completed.
With the new closedness algorithm this had the
consequence that, for such nodes, finalizeShared would
not be called, which, in turn, meant that that node
would be missing conjunct markers relevant for the
closedness algorithm.
This bug could be the cause of other related issues that
had yet to be exposed.
Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: Ibcd2218c4048009bac37485dcc5d4085140f8756
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1211563
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Reviewed-by: Daniel Martí <mvdan@mvdan.cc>
+50
-61
+7
-22
cue/testdata/definitions/defembed.txtar
+7
-22
cue/testdata/definitions/defembed.txtar
···
27
27
Errors:
28
28
a.c: field not allowed:
29
29
./in.cue:5:4
30
-
b.leaf_ref: field not allowed:
31
-
./in.cue:13:5
32
30
33
31
Result:
34
32
(_|_){
···
44
42
#A: (#struct){
45
43
b: (int){ 1 }
46
44
}
47
-
b: (_|_){
48
-
// [eval]
45
+
b: (#struct){
49
46
leaf: (string){ "leaf value" }
50
47
interpolation: (string){ "more leaf value" }
51
-
leaf_ref: (_|_){
52
-
// [eval] b.leaf_ref: field not allowed:
53
-
// ./in.cue:13:5
54
-
}
48
+
leaf_ref: (string){ "leaf value" }
55
49
b: (int){ 1 }
56
50
}
57
51
}
···
59
53
diff old new
60
54
--- old
61
55
+++ new
62
-
@@ -1,9 +1,8 @@
56
+
@@ -1,9 +1,6 @@
63
57
Errors:
64
58
a.c: field not allowed:
65
59
- ./in.cue:1:4
66
60
- ./in.cue:2:2
67
61
./in.cue:5:4
68
62
- ./in.cue:7:5
69
-
+b.leaf_ref: field not allowed:
70
-
+ ./in.cue:13:5
71
63
72
64
Result:
73
65
(_|_){
74
-
@@ -10,22 +9,23 @@
66
+
@@ -10,22 +7,19 @@
75
67
// [eval]
76
68
a: (_|_){
77
69
// [eval]
···
82
74
- // ./in.cue:2:2
83
75
// ./in.cue:5:4
84
76
- // ./in.cue:7:5
85
-
- }
86
-
+ }
77
+
}
87
78
+ b: (int){ 1 }
88
79
}
89
80
#A: (#struct){
90
81
b: (int){ 1 }
91
82
}
92
-
- b: (#struct){
83
+
b: (#struct){
93
84
- b: (int){ 1 }
94
-
+ b: (_|_){
95
-
+ // [eval]
96
85
leaf: (string){ "leaf value" }
97
86
interpolation: (string){ "more leaf value" }
98
-
- leaf_ref: (string){ "leaf value" }
99
-
+ leaf_ref: (_|_){
100
-
+ // [eval] b.leaf_ref: field not allowed:
101
-
+ // ./in.cue:13:5
102
-
+ }
87
+
leaf_ref: (string){ "leaf value" }
103
88
+ b: (int){ 1 }
104
89
}
105
90
}
+43
-39
internal/core/adt/unify.go
+43
-39
internal/core/adt/unify.go
···
343
343
344
344
n.finalizeDisjunctions()
345
345
346
+
if n.completed&(subFieldsProcessed) == 0 {
347
+
return false
348
+
}
349
+
346
350
w = v.DerefValue() // Dereference anything, including shared nodes.
347
351
if w != v {
348
352
// Clear value fields that are now referred to in the dereferenced
···
360
364
v.HasEllipsis = true
361
365
}
362
366
v.status = w.status
367
+
368
+
n.finalizeSharing()
363
369
364
370
// TODO: find a more principled way to catch this cycle and avoid this
365
371
// check.
···
389
395
// }
390
396
391
397
// validationCompleted
392
-
if n.completed&(subFieldsProcessed) != 0 {
393
-
// The next piece of code used to address the following case
394
-
// (order matters)
395
-
//
396
-
// c1: c: [string]: f2
397
-
// f2: c1
398
-
// Also: cycle/issue990
399
-
//
400
-
// However, with recent changes, it no longer matters. Simultaneously,
401
-
// this causes a hang in the following case:
402
-
//
403
-
// _self: x: [...and(x)]
404
-
// _self
405
-
// x: [1]
406
-
//
407
-
// For this reason we disable it now. It may be the case that we need
408
-
// to enable it for computing disjunctions.
409
-
//
410
-
n.incDepth()
411
-
defer n.decDepth()
398
+
// The next piece of code used to address the following case
399
+
// (order matters)
400
+
//
401
+
// c1: c: [string]: f2
402
+
// f2: c1
403
+
// Also: cycle/issue990
404
+
//
405
+
// However, with recent changes, it no longer matters. Simultaneously,
406
+
// this causes a hang in the following case:
407
+
//
408
+
// _self: x: [...and(x)]
409
+
// _self
410
+
// x: [1]
411
+
//
412
+
// For this reason we disable it now. It may be the case that we need
413
+
// to enable it for computing disjunctions.
414
+
//
415
+
n.incDepth()
416
+
defer n.decDepth()
412
417
413
-
if pc := n.node.PatternConstraints; pc != nil {
414
-
for _, c := range pc.Pairs {
415
-
c.Constraint.unify(n.ctx, allKnown, attemptOnly)
416
-
}
418
+
if pc := n.node.PatternConstraints; pc != nil {
419
+
for _, c := range pc.Pairs {
420
+
c.Constraint.unify(n.ctx, allKnown, attemptOnly)
417
421
}
422
+
}
418
423
419
-
// TODO: find more strategic place to set ClosedRecursive and get rid
420
-
// of helper fields.
421
-
blockClose := n.hasTop
422
-
if n.hasStruct {
423
-
blockClose = false
424
-
}
425
-
if n.hasOpenValidator {
426
-
blockClose = true
427
-
}
428
-
if n.isDef && !blockClose {
429
-
n.node.ClosedRecursive = true
430
-
}
424
+
// TODO: find more strategic place to set ClosedRecursive and get rid
425
+
// of helper fields.
426
+
blockClose := n.hasTop
427
+
if n.hasStruct {
428
+
blockClose = false
429
+
}
430
+
if n.hasOpenValidator {
431
+
blockClose = true
432
+
}
433
+
if n.isDef && !blockClose {
434
+
n.node.ClosedRecursive = true
435
+
}
431
436
432
-
n.node.updateStatus(finalized)
437
+
n.node.updateStatus(finalized)
433
438
434
-
n.checkTypos()
435
-
}
439
+
n.checkTypos()
436
440
437
441
return n.meets(needs)
438
442
}