internal/core/adt: finalize earlier for arcs

This introduces some separate logic for if a vertex needs early processing when a subfield has a comprehension: once optional fields are introduced it will no longer be sufficient to know that an arc is not void. It is a noop now and only causes some evaluation to be done early (see some of the test changes that now resolve to a concrete value). Other than that, the main purpose of this change is to isolate this change from the optional field rewrite so that if any errors result from this change they are bisectable to a smaller CL. Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com> Change-Id: Idb2afe08ac63eae7f2e8dad98aea6e9d8eb7f5ea Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/551103 Reviewed-by: Roger Peppe <rogpeppe@gmail.com> Unity-Result: CUEcueckoo <cueckoo@cuelang.org> TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
cue-lang · Mar 16, 2023 · c80a690 · c80a690
1 parent 27b096c
commit c80a690
Show file tree

Hide file tree

Showing 11 changed files with 81 additions and 79 deletions.
diff --git a/cue/testdata/builtins/incomplete.txtar b/cue/testdata/builtins/incomplete.txtar
@@ -101,11 +101,11 @@ multipleErrors: {
 	}
 }
 -- out/eval/stats --
-Leaks:  7
-Freed:  122
-Reused: 117
-Allocs: 12
-Retain: 54
+Leaks:  6
+Freed:  123
+Reused: 118
+Allocs: 11
+Retain: 53
 
 Unifications: 109
 Conjuncts:    267

diff --git a/cue/testdata/comprehensions/pushdown.txtar b/cue/testdata/comprehensions/pushdown.txtar
@@ -746,11 +746,11 @@ Leaks:  17
 Freed:  392
 Reused: 386
 Allocs: 23
-Retain: 134
+Retain: 136
 
 Unifications: 395
 Conjuncts:    646
-Disjuncts:    483
+Disjuncts:    485
 -- out/eval --
 Errors:
 embed.fail1.p: field not allowed:

diff --git a/cue/testdata/cycle/051_resolved_self-reference_cycles_with_disjunction.txtar b/cue/testdata/cycle/051_resolved_self-reference_cycles_with_disjunction.txtar
@@ -168,15 +168,15 @@ z3:  z1-3 & 8
   z3: 8
 }
 -- out/eval/stats --
-Leaks:  5
-Freed:  47
+Leaks:  4
+Freed:  48
 Reused: 40
 Allocs: 12
-Retain: 36
+Retain: 35
 
 Unifications: 31
-Conjuncts:    133
-Disjuncts:    57
+Conjuncts:    136
+Disjuncts:    58
 -- out/eval --
 Errors:
 xe1: 2 errors in empty disjunction:

diff --git a/cue/testdata/cycle/052_resolved_self-reference_cycles_with_disjunction_with_defaults.txtar b/cue/testdata/cycle/052_resolved_self-reference_cycles_with_disjunction_with_defaults.txtar
@@ -125,11 +125,11 @@ z3:  8
   z3: 8
 }
 -- out/eval/stats --
-Leaks:  6
-Freed:  38
+Leaks:  4
+Freed:  40
 Reused: 34
 Allocs: 10
-Retain: 38
+Retain: 36
 
 Unifications: 27
 Conjuncts:    93

diff --git a/cue/testdata/cycle/compbottom2.txtar b/cue/testdata/cycle/compbottom2.txtar
@@ -263,15 +263,15 @@ nestedChain: {
 	}
 }
 -- out/eval/stats --
-Leaks:  3
-Freed:  141
-Reused: 132
-Allocs: 12
-Retain: 66
+Leaks:  1
+Freed:  143
+Reused: 134
+Allocs: 10
+Retain: 72
 
 Unifications: 144
 Conjuncts:    159
-Disjuncts:    186
+Disjuncts:    194
 -- out/eval --
 (struct){
   self: (struct){

diff --git a/cue/testdata/cycle/compbottomnofinal.txtar b/cue/testdata/cycle/compbottomnofinal.txtar
@@ -357,15 +357,15 @@ large: {
 	}
 }
 -- out/eval/stats --
-Leaks:  16
-Freed:  95
-Reused: 84
-Allocs: 27
-Retain: 292
-
-Unifications: 111
-Conjuncts:    227
-Disjuncts:    206
+Leaks:  7
+Freed:  108
+Reused: 97
+Allocs: 18
+Retain: 311
+
+Unifications: 115
+Conjuncts:    238
+Disjuncts:    230
 -- out/eval --
 (struct){
   minimal: (struct){
@@ -529,12 +529,9 @@ Disjuncts:    206
         host: (string){ "mod.test" }
       }
       #Y: (#struct){
-        host: (_|_){// (string){ "mod.test" }
-        }
-        port: (_|_){// (string){ "" }
-        }
-        userinfo: (_|_){// (string){ "user" }
-        }
+        host: (string){ "mod.test" }
+        port: (string){ "" }
+        userinfo: (string){ "user" }
       }
     }
     p3: (struct){
@@ -554,8 +551,7 @@ Disjuncts:    206
       }
       #Y: (#struct){
         host: (string){ "mod.test" }
-        port: (_|_){// (string){ "" }
-        }
+        port: (string){ "" }
         userinfo: (string){ "user" }
       }
     }

diff --git a/cue/testdata/cycle/evaluate.txtar b/cue/testdata/cycle/evaluate.txtar
@@ -107,11 +107,11 @@ closeFail: {
 }
 
 -- out/eval/stats --
-Leaks:  67
-Freed:  94
-Reused: 90
-Allocs: 71
-Retain: 137
+Leaks:  66
+Freed:  95
+Reused: 91
+Allocs: 70
+Retain: 136
 
 Unifications: 149
 Conjuncts:    289

diff --git a/cue/testdata/cycle/structural.txtar b/cue/testdata/cycle/structural.txtar
@@ -532,11 +532,11 @@ n2: n1 & {a: n1}
 n3: n1 & {n1}
 n4: n1 & {x: n1 & {y: n1 & {z: int}}}
 -- out/eval/stats --
-Leaks:  17
-Freed:  791
-Reused: 779
-Allocs: 29
-Retain: 66
+Leaks:  16
+Freed:  792
+Reused: 780
+Allocs: 28
+Retain: 65
 
 Unifications: 622
 Conjuncts:    1219

diff --git a/internal/core/adt/composite.go b/internal/core/adt/composite.go
@@ -196,6 +196,9 @@ type Vertex struct {
 	// Used for cycle detection.
 	IsDynamic bool
 
+	// hasVoidArc is set if this Vertex has a void arc (e.g. for comprehensions)
+	hasVoidArc bool
+
 	// ArcType indicates the level of optionality of this arc.
 	ArcType ArcType
 
@@ -764,6 +767,9 @@ func (v *Vertex) GetArc(c *OpContext, f Feature, t ArcType) (arc *Vertex, isNew
 	}
 	arc = &Vertex{Parent: v, Label: f, ArcType: t}
 	v.Arcs = append(v.Arcs, arc)
+	if t == ArcVoid {
+		v.hasVoidArc = true
+	}
 	return arc, true
 }
 

diff --git a/internal/core/adt/eval.go b/internal/core/adt/eval.go
@@ -732,7 +732,7 @@ func (n *nodeContext) completeArcs(state VertexStatus) {
 	if state <= Conjuncts &&
 		// Is allowed to go one step back. See Vertex.UpdateStatus.
 		n.node.status <= state+1 &&
-		n.node.ArcType != ArcVoid {
+		(!n.node.hasVoidArc || n.node.ArcType == ArcMember) {
 
 		n.node.UpdateStatus(Conjuncts)
 		return
@@ -752,7 +752,7 @@ func (n *nodeContext) completeArcs(state VertexStatus) {
 
 			wasVoid := !a.isDefined()
 
-			ctx.Unify(a, state)
+			ctx.Unify(a, Finalized)
 
 			if !a.isDefined() {
 				continue

diff --git a/tools/flow/testdata/template.txtar b/tools/flow/testdata/template.txtar
@@ -46,6 +46,16 @@ graph TD
 		body: ""
 	}
 }
+-- out/run/t1/stats --
+Leaks:  0
+Freed:  41
+Reused: 34
+Allocs: 7
+Retain: 0
+
+Unifications: 24
+Conjuncts:    64
+Disjuncts:    41
 -- out/run/t2 --
 graph TD
   t0("root.get [Terminated]")
@@ -78,34 +88,6 @@ graph TD
 		stdout: "foo"
 	}
 }
--- out/run/t3 --
-graph TD
-  t0("root.get [Terminated]")
-  t0-->t2
-  t1("root.convert [Terminated]")
-  t1-->t0
-  t2("foo [Terminated]")
-
--- out/run/t3/value --
-{
-	$id: "tool/exec.Run"
-	cmd: "go run cuelang.org/go/cmd/cue import -f -p json -l #Workflow: jsonschema: - --outfile pkg/github.com/SchemaStore/schemastore/src/schemas/json/github-workflow.cue"
-	env: {}
-	stdout:  "foo"
-	stderr:  null
-	stdin:   (*null | string | bytes) & get.response.body
-	success: bool
-}
--- out/run/t1/stats --
-Leaks:  0
-Freed:  41
-Reused: 34
-Allocs: 7
-Retain: 0
-
-Unifications: 24
-Conjuncts:    64
-Disjuncts:    41
 -- out/run/t2/stats --
 Leaks:  0
 Freed:  41
@@ -125,4 +107,22 @@ Retain: 0
 
 Unifications: 48
 Conjuncts:    132
-Disjuncts:    82
+Disjuncts:    82
+-- out/run/t3 --
+graph TD
+  t0("root.get [Terminated]")
+  t0-->t2
+  t1("root.convert [Terminated]")
+  t1-->t0
+  t2("foo [Terminated]")
+
+-- out/run/t3/value --
+{
+	$id: "tool/exec.Run"
+	cmd: "go run cuelang.org/go/cmd/cue import -f -p json -l #Workflow: jsonschema: - --outfile pkg/github.com/SchemaStore/schemastore/src/schemas/json/github-workflow.cue"
+	env: {}
+	stdout:  "foo"
+	stderr:  null
+	stdin:   (*null | string | bytes) & get.response.body
+	success: bool
+}