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>
diff --git a/cmd/cue/cmd/testdata/script/vet_file.txt b/cmd/cue/cmd/testdata/script/vet_file.txt
index 3797681..d5e5fcd 100644
--- a/cmd/cue/cmd/testdata/script/vet_file.txt
+++ b/cmd/cue/cmd/testdata/script/vet_file.txt
@@ -8,11 +8,13 @@
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
diff --git a/cue/testdata/benchmarks/dedupelem.txtar b/cue/testdata/benchmarks/dedupelem.txtar
index ca1ed5b..b83f06c 100644
--- a/cue/testdata/benchmarks/dedupelem.txtar
+++ b/cue/testdata/benchmarks/dedupelem.txtar
@@ -19,7 +19,7 @@
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 @@
// 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
diff --git a/cue/testdata/comprehensions/issue293.txtar b/cue/testdata/comprehensions/issue293.txtar
index 3bd38ef..30a8166 100644
--- a/cue/testdata/comprehensions/issue293.txtar
+++ b/cue/testdata/comprehensions/issue293.txtar
@@ -27,6 +27,7 @@
-- 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 @@
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
diff --git "a/cue/testdata/definitions/033_Issue_\043153.txtar" "b/cue/testdata/definitions/033_Issue_\043153.txtar"
index 1eaf9f5..1524dd4 100644
--- "a/cue/testdata/definitions/033_Issue_\043153.txtar"
+++ "b/cue/testdata/definitions/033_Issue_\043153.txtar"
@@ -67,6 +67,7 @@
./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 @@
// ./in.cue:5:10
// ./in.cue:13:1
// ./in.cue:14:18
+ // ./in.cue:15:3
// ./in.cue:16:4
}
}
diff --git a/cue/testdata/definitions/037_closing_with_comprehensions.txtar b/cue/testdata/definitions/037_closing_with_comprehensions.txtar
index 289eda3..95024d2 100644
--- a/cue/testdata/definitions/037_closing_with_comprehensions.txtar
+++ b/cue/testdata/definitions/037_closing_with_comprehensions.txtar
@@ -108,9 +108,11 @@
./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 @@
// ./in.cue:1:5
// ./in.cue:27:5
// ./in.cue:27:10
+ // ./in.cue:28:2
// ./in.cue:29:3
}
}
@@ -149,6 +152,7 @@
f3: (_|_){
// [eval] a: field `f3` not allowed:
// ./in.cue:1:5
+ // ./in.cue:3:1
// ./in.cue:4:5
// ./in.cue:4:11
}
diff --git a/cue/testdata/resolve/029_non-closed_definition_carries_over_closedness_to_enclosed_template.txtar b/cue/testdata/resolve/029_non-closed_definition_carries_over_closedness_to_enclosed_template.txtar
index f94d398..c184206 100644
--- a/cue/testdata/resolve/029_non-closed_definition_carries_over_closedness_to_enclosed_template.txtar
+++ b/cue/testdata/resolve/029_non-closed_definition_carries_over_closedness_to_enclosed_template.txtar
@@ -114,6 +114,7 @@
./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 @@
// ./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 @@
// [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
diff --git a/internal/core/adt/closed.go b/internal/core/adt/closed.go
index 327d7a5..6e978f3 100644
--- a/internal/core/adt/closed.go
+++ b/internal/core/adt/closed.go
@@ -91,9 +91,9 @@
// 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 @@
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 @@
}
if isDef {
c.mode = closeDef
+ c.closeInfo.span |= DefinitionSpan
}
return c
}
@@ -187,12 +204,9 @@
// 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 {
diff --git a/internal/core/adt/eval.go b/internal/core/adt/eval.go
index 54058de..766f339 100644
--- a/internal/core/adt/eval.go
+++ b/internal/core/adt/eval.go
@@ -1787,8 +1787,12 @@
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)
}
}
diff --git a/internal/core/adt/expr.go b/internal/core/adt/expr.go
index 2a6e1aa..ba5aabd 100644
--- a/internal/core/adt/expr.go
+++ b/internal/core/adt/expr.go
@@ -127,6 +127,7 @@
if x.Value == nil {
o.IsOpen = true
o.types |= IsOpen
+ // TODO(perf): encode more efficiently.
expr = &Top{}
} else {
o.types |= HasAdditional
diff --git a/internal/core/adt/optional.go b/internal/core/adt/optional.go
index 504d0f6..c648431 100644
--- a/internal/core/adt/optional.go
+++ b/internal/core/adt/optional.go
@@ -40,22 +40,26 @@
return
}
- bulkEnv := *env
- bulkEnv.DynamicLabel = arc.Label
- bulkEnv.Deref = nil
- bulkEnv.Cycles = nil
+ 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
- arc.AddConjunct(MakeConjunct(&bulkEnv, b, closeInfo))
+ // 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 @@
// 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))
}
}
