internal/core/adt: fix EVAL counter issue

EVAL is used to signal that a closeContext is not
closed until a node is done. However, this is not
only meaningless if it crosses disjunction boundaries,
but will also cause spurious allocations and
counters that fail to decrement.

This also fixes a bunch of tests in encoding/jsonschema

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: I0839b1b8903252b0e0b6a01171c2ea2b43fff66f
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1207442
Reviewed-by: Daniel Martí <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>

Author: mpvl

encoding/jsonschema/external_teststats.txt

@@ -6,8 +6,8 @@ v2:
 
 v3:
 	schema extract (pass / total): 1054 / 1363 = 77.3%
-	tests (pass / total): 3783 / 4803 = 78.8%
-	tests on extracted schemas (pass / total): 3783 / 3955 = 95.7%
+	tests (pass / total): 3788 / 4803 = 78.9%
+	tests on extracted schemas (pass / total): 3788 / 3955 = 95.8%
 
 Optional tests
 

encoding/jsonschema/testdata/external/tests/draft2019-09/ref.json

@@ -44,10 +44,7 @@
 						"bar": false
 					}
 				},
-				"valid": false,
-				"skip": {
-					"v3": "unexpected success"
-				}
+				"valid": false
 			}
 		]
 	},

encoding/jsonschema/testdata/external/tests/draft2020-12/ref.json

@@ -44,10 +44,7 @@
 						"bar": false
 					}
 				},
-				"valid": false,
-				"skip": {
-					"v3": "unexpected success"
-				}
+				"valid": false
 			}
 		]
 	},

encoding/jsonschema/testdata/external/tests/draft4/ref.json

@@ -43,10 +43,7 @@
 						"bar": false
 					}
 				},
-				"valid": false,
-				"skip": {
-					"v3": "unexpected success"
-				}
+				"valid": false
 			}
 		]
 	},

encoding/jsonschema/testdata/external/tests/draft6/ref.json

@@ -43,10 +43,7 @@
 						"bar": false
 					}
 				},
-				"valid": false,
-				"skip": {
-					"v3": "unexpected success"
-				}
+				"valid": false
 			}
 		]
 	},

encoding/jsonschema/testdata/external/tests/draft7/ref.json

@@ -43,10 +43,7 @@
 						"bar": false
 					}
 				},
-				"valid": false,
-				"skip": {
-					"v3": "unexpected success"
-				}
+				"valid": false
 			}
 		]
 	},

internal/core/adt/eval_test.go

@@ -86,7 +86,7 @@ var skipDebugDepErrors = map[string]int{
 	"cycle/disjunction":        4,
 	"cycle/evaluate":           1,
 	"cycle/issue990":           1,
-	"cycle/structural":         17,
+	"cycle/structural":         13,
 	"disjunctions/edge":        1,
 	"disjunctions/errors":      3,
 	"disjunctions/elimination": 19,

internal/core/adt/overlay.go

@@ -81,13 +81,19 @@ func (ctx *overlayContext) cloneRoot(root *nodeContext) *nodeContext {
 	v := ctx.cloneVertex(root.node)
 	v.IsDisjunct = true
 
+	// At this point we have copied all the mandatory closeContexts. There
+	// may be derivative closeContexts copied as well.
+
 	// TODO: patch notifications to any node that is within the disjunct to
 	// point to the new vertex instead.
 
 	// Initialize closeContexts: at this point, all closeContexts that need to
 	// be cloned have been allocated and stored in closeContexts and can now be
 	// initialized.
-	for _, cc := range ctx.closeContexts {
+	// Use an explicit index as initCloneCC uses allocCC, which MAY allocate a
+	// new closeContext. It probably does not, but we use an index in case.
+	for i := 0; i < len(ctx.closeContexts); i++ {
+		cc := ctx.closeContexts[i]
 		ctx.initCloneCC(cc)
 	}
 
@@ -309,12 +315,6 @@ func (ctx *overlayContext) allocCC(cc *closeContext) *closeContext {
 	// src is set in the root closeContext when cloning a vertex.
 	ctx.closeContexts = append(ctx.closeContexts, cc)
 
-	// needsCloseInSchedule is used as a boolean. The pointer to the original
-	// closeContext is just used for reporting purposes.
-	if cc.needsCloseInSchedule != nil {
-		o.needsCloseInSchedule = ctx.allocCC(cc.needsCloseInSchedule)
-	}
-
 	// We only explicitly tag dependencies of type ARC. Notifications that
 	// point within the disjunct overlay will be tagged elsewhere.
 	for _, a := range cc.arcs {
@@ -351,6 +351,18 @@ func (ctx *overlayContext) initCloneCC(x *closeContext) {
 	o.Expr = x.Expr
 	o.Patterns = append(o.Patterns, x.Patterns...)
 
+	// needsCloseInSchedule is a separate mechanism to signal nodes that have
+	// completed. Do not signal such nodes if they are outside the current
+	// copied graph.
+	if cx := x.needsCloseInSchedule; cx != nil {
+		if cx.src._cc.overlay == nil {
+			o.needsCloseInSchedule = nil
+		} else {
+			// TODO: Debug assert: cx.overlay != nil
+			o.needsCloseInSchedule = ctx.allocCC(cx)
+		}
+	}
+
 	// child and next always point to completed closeContexts. Moreover, only
 	// fields that are immutable, such as Expr, are used. It is therefore not
 	// necessary to use overlays.