internal/core/adt: add support info for trim

This change adds different span types to track
in a closeInfo struct.

A SpanType keeps track how nodes were introduced,
for instance by comprehension, definition, or
constraint.

The added closeInfo nodes also improve error messages.
Previously, only referred structs were adding
position information, but indirectly added structs,
for instance those added by a constraint, did not
result in additional position information.

The information introduced here is intended for
trim, but could very well be useful for other
algorithms.

It is kept as a separate CL, mostly to allow tracking
down performance issues, if such were to arise.
So far the trim refactoring seems to improve
performance, though, because some of the hot
paths have been improved.

Change-Id: I656bad2ced723a1bc54971efbd130711c47a75be
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8237
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Paul Jolly <paul@myitcv.org.uk>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>

cmd/cue/cmd/testdata/script/vet_file.txt

@@ -8,11 +8,13 @@ cmp stderr expect-stderr2
 translations.hello.lang: incomplete value string
 translations.hello.lang: conflicting values false and string (mismatched types bool and string):
     ./data.yaml:13:11
+    ./vet.cue:3:25
     ./vet.cue:3:31
 -- expect-stderr2 --
 translations.hello.lang: incomplete value string
 translations.hello.lang: conflicting values false and string (mismatched types bool and string):
     ./data.yaml:13:11
+    ./vet.cue:3:25
     ./vet.cue:3:31
 -- vet.cue --
 package foo

cue/testdata/benchmarks/dedupelem.txtar

@@ -19,7 +19,7 @@ foo.0: 2 errors in empty disjunction:
 foo.0.type: conflicting values "int" and "float":
     ./in.cue:3:16
     ./in.cue:5:14
-    ./in.cue:5:30
+    ./in.cue:6:30
 foo.0.type: conflicting values "string" and "float":
     ./in.cue:5:14
     ./in.cue:10:14
@@ -41,7 +41,7 @@ Result:
       // foo.0.type: conflicting values "int" and "float":
       //     ./in.cue:3:16
       //     ./in.cue:5:14
-      //     ./in.cue:5:30
+      //     ./in.cue:6:30
       // foo.0.type: conflicting values "string" and "float":
       //     ./in.cue:5:14
       //     ./in.cue:10:14

cue/testdata/comprehensions/issue293.txtar

@@ -27,6 +27,7 @@ t: p: "foo"
 -- out/eval --
 Errors:
 z.x: field `f2` not allowed:
+    ./in.cue:2:2
     ./in.cue:5:12
     ./in.cue:11:4
     ./in.cue:14:3
@@ -46,6 +47,7 @@ Result:
       f1: (int){ 99 }
       f2: (_|_){
         // [eval] z.x: field `f2` not allowed:
+        //     ./in.cue:2:2
         //     ./in.cue:5:12
         //     ./in.cue:11:4
         //     ./in.cue:14:3

cue/testdata/definitions/033_Issue_#153.txtar

@@ -67,6 +67,7 @@ listOfCloseds.0: field `b` not allowed:
     ./in.cue:5:10
     ./in.cue:13:1
     ./in.cue:14:18
+    ./in.cue:15:3
     ./in.cue:16:4
 
 Result:
@@ -83,6 +84,7 @@ Result:
         //     ./in.cue:5:10
         //     ./in.cue:13:1
         //     ./in.cue:14:18
+        //     ./in.cue:15:3
         //     ./in.cue:16:4
       }
     }

cue/testdata/definitions/037_closing_with_comprehensions.txtar

@@ -108,9 +108,11 @@ Errors:
     ./in.cue:1:5
     ./in.cue:27:5
     ./in.cue:27:10
+    ./in.cue:28:2
     ./in.cue:29:3
 a: field `f3` not allowed:
     ./in.cue:1:5
+    ./in.cue:3:1
     ./in.cue:4:5
     ./in.cue:4:11
 
@@ -139,6 +141,7 @@ Result:
       //     ./in.cue:1:5
       //     ./in.cue:27:5
       //     ./in.cue:27:10
+      //     ./in.cue:28:2
       //     ./in.cue:29:3
     }
   }
@@ -149,6 +152,7 @@ Result:
     f3: (_|_){
       // [eval] a: field `f3` not allowed:
       //     ./in.cue:1:5
+      //     ./in.cue:3:1
       //     ./in.cue:4:5
       //     ./in.cue:4:11
     }

cue/testdata/resolve/029_non-closed_definition_carries_over_closedness_to_enclosed_template.txtar

@@ -114,6 +114,7 @@ b.w: field `c` not allowed:
     ./in.cue:10:4
     ./in.cue:11:6
 c.w.0: field `d` not allowed:
+    ./in.cue:14:12
     ./in.cue:14:13
     ./in.cue:16:4
     ./in.cue:17:7
@@ -148,6 +149,7 @@ Result:
       //     ./in.cue:11:6
       c: (_|_){
         // [eval] b.w: field `c` not allowed:
+        //     ./in.cue:8:12
         //     ./in.cue:8:23
         //     ./in.cue:10:4
         //     ./in.cue:11:6
@@ -165,6 +167,7 @@ Result:
         // [eval]
         d: (_|_){
           // [eval] c.w.0: field `d` not allowed:
+          //     ./in.cue:14:12
           //     ./in.cue:14:13
           //     ./in.cue:16:4
           //     ./in.cue:17:7

internal/core/adt/closed.go

@@ -91,9 +91,9 @@ const (
 
 // TODO: merge with closeInfo: this is a leftover of the refactoring.
 type CloseInfo struct {
-	IsClosed bool
-
 	*closeInfo
+
+	IsClosed bool
 }
 
 // TODO(perf): remove: error positions should always be computed on demand
@@ -141,6 +141,22 @@ func (c CloseInfo) SpawnGroup(x Expr) CloseInfo {
 	return c
 }
 
+// SpawnSpan is used to track that a value is introduced by a comprehension
+// or constraint. Definition and embedding spans are introduced with SpawnRef
+// and SpawnEmbed, respectively.
+func (c CloseInfo) SpawnSpan(x Node, t SpanType) CloseInfo {
+	var span SpanType
+	if c.closeInfo != nil {
+		span = c.span
+	}
+	c.closeInfo = &closeInfo{
+		parent:   c.closeInfo,
+		location: x,
+		span:     span | t,
+	}
+	return c
+}
+
 func (c CloseInfo) SpawnRef(arc *Vertex, isDef bool, x Expr) CloseInfo {
 	var span SpanType
 	if c.closeInfo != nil {
@@ -153,6 +169,7 @@ func (c CloseInfo) SpawnRef(arc *Vertex, isDef bool, x Expr) CloseInfo {
 	}
 	if isDef {
 		c.mode = closeDef
+		c.closeInfo.span |= DefinitionSpan
 	}
 	return c
 }
@@ -187,12 +204,9 @@ const (
 	// EmbeddingSpan means that this value was embedded at some point and should
 	// not be included as a possible root node in the todo field of OpContext.
 	EmbeddingSpan SpanType = 1 << iota
-
-	// TODO:
-	// from comprehension
-	// from template.
-	// from definition
-
+	ConstraintSpan
+	ComprehensionSpan
+	DefinitionSpan
 )
 
 type closeInfo struct {

internal/core/adt/eval.go

@@ -1787,8 +1787,12 @@ func (n *nodeContext) injectEmbedded(all *[]envYield) (progress bool) {
 			continue
 		}
 
+		if len(sa) == 0 {
+			continue
+		}
+		id := d.id.SpawnSpan(d.yield, ComprehensionSpan)
 		for _, st := range sa {
-			n.addStruct(st.env, st.s, d.id)
+			n.addStruct(st.env, st.s, id)
 		}
 	}
 

internal/core/adt/expr.go

@@ -127,6 +127,7 @@ func (o *StructLit) Init() {
 			if x.Value == nil {
 				o.IsOpen = true
 				o.types |= IsOpen
+				// TODO(perf): encode more efficiently.
 				expr = &Top{}
 			} else {
 				o.types |= HasAdditional

internal/core/adt/optional.go

@@ -40,22 +40,26 @@ outer:
 		return
 	}
 
-	bulkEnv := *env
-	bulkEnv.DynamicLabel = arc.Label
-	bulkEnv.Deref = nil
-	bulkEnv.Cycles = nil
-
-	// match bulk optional fields / pattern properties
-	for _, b := range o.Bulk {
-		// if matched && f.additional {
-		// 	continue
-		// }
-		if matchBulk(c, env, b, arc.Label) {
-			matched = true
-			arc.AddConjunct(MakeConjunct(&bulkEnv, b, closeInfo))
+	if len(o.Bulk) > 0 {
+		bulkEnv := *env
+		bulkEnv.DynamicLabel = arc.Label
+		bulkEnv.Deref = nil
+		bulkEnv.Cycles = nil
+
+		// match bulk optional fields / pattern properties
+		for _, b := range o.Bulk {
+			// if matched && f.additional {
+			// 	continue
+			// }
+			if matchBulk(c, env, b, arc.Label) {
+				matched = true
+				info := closeInfo.SpawnSpan(b.Value, ConstraintSpan)
+				arc.AddConjunct(MakeConjunct(&bulkEnv, b, info))
+			}
 		}
 	}
-	if matched {
+
+	if matched || len(o.Additional) == 0 {
 		return
 	}
 
@@ -65,7 +69,11 @@ outer:
 
 	// match others
 	for _, x := range o.Additional {
-		arc.AddConjunct(MakeConjunct(&addEnv, x, closeInfo))
+		info := closeInfo
+		if _, ok := x.(*Top); !ok {
+			info = info.SpawnSpan(x, ConstraintSpan)
+		}
+		arc.AddConjunct(MakeConjunct(&addEnv, x, info))
 	}
 }