tools/trim: implement using adt
The data structures (adt) of the new evaluator make
implementing trim a lot easier than it was before.
The new evaluator tracks all conjuncts in a Vertex.
It now also marks whether a value originated from
a comprehension, definition, embedding, etc. This
means that the trim implemenation no longer needs to
maintain parallel data structures, which is hard and
error prone.
It also allows the right level of pre-evaluation,
so that the right values can be used to determine
whether a field is subsumed. This was hard in the
old implementation.
Ultimately it would be good if these kind of tools
could be build on a public API, but let's first try
to understand, building tools like trim, what would
be a good API.
Fixes #643
Fixes #626
Fixes #558
Closes #548
This fixes several other issues that were noted in
the tests and issues that were not explicitly marked
as issues.
Change-Id: I2a8cf6933288fbfa427792e536fbca030a04b3b2
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8238
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/trim.txt b/cmd/cue/cmd/testdata/script/trim.txt
index b5f0cd5..78468eb 100644
--- a/cmd/cue/cmd/testdata/script/trim.txt
+++ b/cmd/cue/cmd/testdata/script/trim.txt
@@ -44,11 +44,9 @@
}
t: u: {
- x: 5
}
}
-// TODO: top-level fields are currently not removed.
group: {
for k, v in foo {
comp: "\(k)": v
@@ -58,7 +56,7 @@
aa: 8 // new value
}
- comp: baz: {} // removed: fully implied by comprehension above
+ comp: {} // TODO: remove: implied by comprehension above
}
-- trim/trim.cue --
package trim
@@ -93,7 +91,7 @@
b: "foo"
c: 45
e: string
- f: ">> here <<" // TODO: remove
+ f: ">> here <<"
// 5 is an integer, so this can be removed.
n: int
@@ -121,7 +119,6 @@
}
}
-// TODO: top-level fields are currently not removed.
group: {
for k, v in foo {
comp: "\(k)": v
@@ -132,6 +129,6 @@
aa: 8 // new value
}
- comp: baz: {} // removed: fully implied by comprehension above
+ comp: baz: {} // TODO: remove: implied by comprehension above
}
-- cue.mod --
diff --git a/doc/tutorial/kubernetes/quick/services/frontend/bartender/kube.cue b/doc/tutorial/kubernetes/quick/services/frontend/bartender/kube.cue
index 2173264..8570398 100644
--- a/doc/tutorial/kubernetes/quick/services/frontend/bartender/kube.cue
+++ b/doc/tutorial/kubernetes/quick/services/frontend/bartender/kube.cue
@@ -1,6 +1,5 @@
package kube
-service: {}
deployment: bartender: spec: template: spec: containers: [{
image: "gcr.io/myproj/bartender:v0.1.34"
args: [
diff --git a/doc/tutorial/kubernetes/quick/services/frontend/breaddispatcher/kube.cue b/doc/tutorial/kubernetes/quick/services/frontend/breaddispatcher/kube.cue
index 18bbcfd..6e06a6a 100644
--- a/doc/tutorial/kubernetes/quick/services/frontend/breaddispatcher/kube.cue
+++ b/doc/tutorial/kubernetes/quick/services/frontend/breaddispatcher/kube.cue
@@ -1,6 +1,5 @@
package kube
-service: {}
deployment: breaddispatcher: spec: template: spec: containers: [{
image: "gcr.io/myproj/breaddispatcher:v0.3.24"
args: [
diff --git a/doc/tutorial/kubernetes/quick/services/frontend/host/kube.cue b/doc/tutorial/kubernetes/quick/services/frontend/host/kube.cue
index 727349d..e58c822 100644
--- a/doc/tutorial/kubernetes/quick/services/frontend/host/kube.cue
+++ b/doc/tutorial/kubernetes/quick/services/frontend/host/kube.cue
@@ -1,6 +1,5 @@
package kube
-service: {}
deployment: host: spec: {
replicas: 2
template: spec: containers: [{
diff --git a/doc/tutorial/kubernetes/quick/services/frontend/maitred/kube.cue b/doc/tutorial/kubernetes/quick/services/frontend/maitred/kube.cue
index 8659935..4829e95 100644
--- a/doc/tutorial/kubernetes/quick/services/frontend/maitred/kube.cue
+++ b/doc/tutorial/kubernetes/quick/services/frontend/maitred/kube.cue
@@ -1,6 +1,5 @@
package kube
-service: {}
deployment: maitred: spec: template: spec: containers: [{
image: "gcr.io/myproj/maitred:v0.0.4"
args: [
diff --git a/doc/tutorial/kubernetes/quick/services/frontend/waiter/kube.cue b/doc/tutorial/kubernetes/quick/services/frontend/waiter/kube.cue
index 5c30c2e..7a2c1de 100644
--- a/doc/tutorial/kubernetes/quick/services/frontend/waiter/kube.cue
+++ b/doc/tutorial/kubernetes/quick/services/frontend/waiter/kube.cue
@@ -1,6 +1,5 @@
package kube
-service: {}
deployment: waiter: spec: {
replicas: 5
template: spec: containers: [{
diff --git a/doc/tutorial/kubernetes/quick/services/infra/download/kube.cue b/doc/tutorial/kubernetes/quick/services/infra/download/kube.cue
index a54a89b..a7860ae 100644
--- a/doc/tutorial/kubernetes/quick/services/infra/download/kube.cue
+++ b/doc/tutorial/kubernetes/quick/services/infra/download/kube.cue
@@ -1,6 +1,5 @@
package kube
-service: {}
deployment: download: spec: template: spec: containers: [{
image: "gcr.io/myproj/download:v0.0.2"
ports: [{
diff --git a/doc/tutorial/kubernetes/quick/services/infra/updater/kube.cue b/doc/tutorial/kubernetes/quick/services/infra/updater/kube.cue
index e72b31e..ca7b848 100644
--- a/doc/tutorial/kubernetes/quick/services/infra/updater/kube.cue
+++ b/doc/tutorial/kubernetes/quick/services/infra/updater/kube.cue
@@ -1,6 +1,5 @@
package kube
-service: {}
deployment: updater: spec: template: spec: {
volumes: [{
name: "secret-updater"
diff --git a/doc/tutorial/kubernetes/quick/services/kitchen/caller/kube.cue b/doc/tutorial/kubernetes/quick/services/kitchen/caller/kube.cue
index 779f801..ddd728b 100644
--- a/doc/tutorial/kubernetes/quick/services/kitchen/caller/kube.cue
+++ b/doc/tutorial/kubernetes/quick/services/kitchen/caller/kube.cue
@@ -1,6 +1,5 @@
package kube
-service: {}
deployment: caller: spec: {
replicas: 3
template: spec: {
diff --git a/doc/tutorial/kubernetes/quick/services/kitchen/dishwasher/kube.cue b/doc/tutorial/kubernetes/quick/services/kitchen/dishwasher/kube.cue
index 8b37f43..49430a4 100644
--- a/doc/tutorial/kubernetes/quick/services/kitchen/dishwasher/kube.cue
+++ b/doc/tutorial/kubernetes/quick/services/kitchen/dishwasher/kube.cue
@@ -1,6 +1,5 @@
package kube
-service: {}
deployment: dishwasher: spec: {
replicas: 5
template: spec: {
diff --git a/doc/tutorial/kubernetes/quick/services/kitchen/expiditer/kube.cue b/doc/tutorial/kubernetes/quick/services/kitchen/expiditer/kube.cue
index 689e480..c152000 100644
--- a/doc/tutorial/kubernetes/quick/services/kitchen/expiditer/kube.cue
+++ b/doc/tutorial/kubernetes/quick/services/kitchen/expiditer/kube.cue
@@ -1,6 +1,5 @@
package kube
-service: {}
deployment: expiditer: spec: template: spec: containers: [{
image: "gcr.io/myproj/expiditer:v0.5.34"
args: [
diff --git a/doc/tutorial/kubernetes/quick/services/kitchen/headchef/kube.cue b/doc/tutorial/kubernetes/quick/services/kitchen/headchef/kube.cue
index fd00407..983d689 100644
--- a/doc/tutorial/kubernetes/quick/services/kitchen/headchef/kube.cue
+++ b/doc/tutorial/kubernetes/quick/services/kitchen/headchef/kube.cue
@@ -1,6 +1,5 @@
package kube
-service: {}
deployment: headchef: spec: template: spec: containers: [{
image: "gcr.io/myproj/headchef:v0.2.16"
volumeMounts: [{
diff --git a/doc/tutorial/kubernetes/quick/services/kitchen/linecook/kube.cue b/doc/tutorial/kubernetes/quick/services/kitchen/linecook/kube.cue
index 862e094..179b993 100644
--- a/doc/tutorial/kubernetes/quick/services/kitchen/linecook/kube.cue
+++ b/doc/tutorial/kubernetes/quick/services/kitchen/linecook/kube.cue
@@ -1,6 +1,5 @@
package kube
-service: {}
deployment: linecook: spec: template: spec: {
volumes: [{
}, {
diff --git a/doc/tutorial/kubernetes/quick/services/kitchen/pastrychef/kube.cue b/doc/tutorial/kubernetes/quick/services/kitchen/pastrychef/kube.cue
index 612e679..3810eda 100644
--- a/doc/tutorial/kubernetes/quick/services/kitchen/pastrychef/kube.cue
+++ b/doc/tutorial/kubernetes/quick/services/kitchen/pastrychef/kube.cue
@@ -1,6 +1,5 @@
package kube
-service: {}
deployment: pastrychef: spec: template: spec: {
volumes: [{
}, {
diff --git a/doc/tutorial/kubernetes/quick/services/kitchen/souschef/kube.cue b/doc/tutorial/kubernetes/quick/services/kitchen/souschef/kube.cue
index d39e055..3d035f4 100644
--- a/doc/tutorial/kubernetes/quick/services/kitchen/souschef/kube.cue
+++ b/doc/tutorial/kubernetes/quick/services/kitchen/souschef/kube.cue
@@ -1,6 +1,5 @@
package kube
-service: {}
deployment: souschef: spec: template: spec: containers: [{
image: "gcr.io/myproj/souschef:v0.5.3"
}]
diff --git a/doc/tutorial/kubernetes/quick/services/proxy/authproxy/service.cue b/doc/tutorial/kubernetes/quick/services/proxy/authproxy/service.cue
index eb6a6e0..f84e1e4 100644
--- a/doc/tutorial/kubernetes/quick/services/proxy/authproxy/service.cue
+++ b/doc/tutorial/kubernetes/quick/services/proxy/authproxy/service.cue
@@ -1,3 +1 @@
package kube
-
-service: {}
diff --git a/internal/core/adt/closed.go b/internal/core/adt/closed.go
index 6e978f3..acf50ac 100644
--- a/internal/core/adt/closed.go
+++ b/internal/core/adt/closed.go
@@ -96,6 +96,34 @@
IsClosed bool
}
+func (c CloseInfo) Location() Node {
+ if c.closeInfo == nil {
+ return nil
+ }
+ return c.closeInfo.location
+}
+
+func (c CloseInfo) SpanMask() SpanType {
+ if c.closeInfo == nil {
+ return 0
+ }
+ return c.span
+}
+
+func (c CloseInfo) RootSpanType() SpanType {
+ if c.closeInfo == nil {
+ return 0
+ }
+ return c.root
+}
+
+func (c CloseInfo) IsInOneOf(t SpanType) bool {
+ if c.closeInfo == nil {
+ return false
+ }
+ return c.span&t != 0
+}
+
// TODO(perf): remove: error positions should always be computed on demand
// in dedicated error types.
func (c *CloseInfo) AddPositions(ctx *OpContext) {
@@ -118,6 +146,7 @@
parent: c.closeInfo,
location: x,
mode: closeEmbed,
+ root: EmbeddingSpan,
span: span | EmbeddingSpan,
}
return c
@@ -152,6 +181,7 @@
c.closeInfo = &closeInfo{
parent: c.closeInfo,
location: x,
+ root: t,
span: span | t,
}
return c
@@ -169,6 +199,7 @@
}
if isDef {
c.mode = closeDef
+ c.closeInfo.root = DefinitionSpan
c.closeInfo.span |= DefinitionSpan
}
return c
@@ -228,6 +259,7 @@
// - it is a sibling of a new definition.
noCheck bool // don't process for inclusion info
+ root SpanType
span SpanType
}
diff --git a/tools/trim/testdata/definitions.txtar b/tools/trim/testdata/definitions.txtar
new file mode 100644
index 0000000..d376569
--- /dev/null
+++ b/tools/trim/testdata/definitions.txtar
@@ -0,0 +1,20 @@
+#Issue #558
+
+-- in.cue --
+#Foo: {
+ bar: int
+ baz: 2
+}
+foo: #Foo & {
+ bar: 1
+ baz: 2
+}
+-- out/trim --
+== in.cue
+#Foo: {
+ bar: int
+ baz: 2
+}
+foo: #Foo & {
+ bar: 1
+}
diff --git a/tools/trim/trim.go b/tools/trim/trim.go
index 4af47a4..17e164c 100644
--- a/tools/trim/trim.go
+++ b/tools/trim/trim.go
@@ -12,21 +12,18 @@
// See the License for the specific language governing permissions and
// limitations under the License.
-// Package trim removes definitions that may be inferred from
-// templates.
+// Package trim removes fields that may be inferred from another mixed in value
+// that "dominates" it. For instance, a value that is merged in from a
+// definition is considered to dominate a value from a regular struct that
+// mixes in this definition. Values derived from constraints and comprehensions
+// can also dominate other fields.
//
-// A field, struct, or list is removed if it is implied by a constraint, such
-// as from an optional field matching a required field, a list type value,
-// a comprehension or any other implied content. It will modify the files in place.
+// A value A is considered to be implied by a value B if A subsumes the default
+// value of B. For instance, if a definition defines a field `a: *1 | int` and
+// mixed in with a struct that defines a field `a: 1 | 2`, then the latter can
+// be removed because a definition field dominates a regular field and because
+// the latter subsumes the default value of the former.
//
-// Limitations
-//
-// Removal is on a best effort basis. Some caveats:
-// - Fields in implied content may refer to fields within the struct in which
-// they are included, but are only resolved on a best-effort basis.
-// - Disjunctions that contain structs in implied content cannot be used to
-// remove fields.
-// - There is currently no verification step: manual verification is required.
//
// Examples:
//
@@ -44,11 +41,7 @@
//
// Results in:
//
-// light: [string]: {
-// room: string
-// brightnessOff: *0.0 | >=0 & <=100.0
-// brightnessOn: *100.0 | >=0 & <=100.0
-// }
+// // Unmodified: light: [string]: { ... }
//
// light: ceiling50: {
// room: "MasterBedroom"
@@ -57,25 +50,15 @@
package trim
import (
- "bytes"
- "fmt"
- "strconv"
- "strings"
-
- "cuelang.org/go/cue/ast"
- "cuelang.org/go/cue/token"
- "cuelang.org/go/internal"
-
- // "cuelang.org/go/cue"
"cuelang.org/go/cue"
+ "cuelang.org/go/cue/ast"
+ "cuelang.org/go/cue/ast/astutil"
+ "cuelang.org/go/internal"
+ "cuelang.org/go/internal/core/adt"
+ "cuelang.org/go/internal/core/runtime"
+ "cuelang.org/go/internal/core/subsume"
)
-type Runtime = cue.Runtime
-
-// TODO:
-// - remove the limitations mentioned in the documentation
-// - implement verification post-processing as extra safety
-
// Config configures trim options.
type Config struct {
Trace bool
@@ -86,443 +69,141 @@
// Trimming is done on a best-effort basis and only when the removed field
// is clearly implied by another field, rather than equal sibling fields.
func Files(files []*ast.File, inst *cue.Instance, cfg *Config) error {
- gen := newTrimSet(cfg)
- gen.files = files
- for _, f := range files {
- gen.markNodes(f)
+ rx, vx := internal.CoreValue(inst.Value())
+ r := rx.(*runtime.Runtime)
+ v := vx.(*adt.Vertex)
+
+ t := &trimmer{
+ Config: *cfg,
+ ctx: adt.NewContext(r, v),
+ remove: map[ast.Node]bool{},
}
- root := inst.Lookup()
- rm := gen.trim("root", root, cue.Value{}, root)
+ t.findSubordinates(v)
+ // Remove subordinate values from files.
for _, f := range files {
- f.Decls = gen.trimDecls(f.Decls, rm, cue.Value{}, true)
+ astutil.Apply(f, func(c astutil.Cursor) bool {
+ if f, ok := c.Node().(*ast.Field); ok && t.remove[f.Value] {
+ c.Delete()
+ }
+ return true
+ }, nil)
}
+
return nil
}
-type trimSet struct {
- cfg Config
- files []*ast.File
- stack []string
- exclude map[ast.Node]bool // don't remove fields marked here
- alwaysGen map[ast.Node]bool // node is always from a generated source
- fromComp map[ast.Node]bool // node originated from a comprehension
+type trimmer struct {
+ Config
+
+ ctx *adt.OpContext
+ remove map[ast.Node]bool
}
-func newTrimSet(cfg *Config) *trimSet {
- if cfg == nil {
- cfg = &Config{}
- }
- return &trimSet{
- cfg: *cfg,
- exclude: map[ast.Node]bool{},
- alwaysGen: map[ast.Node]bool{},
- fromComp: map[ast.Node]bool{},
+func (t *trimmer) markRemove(c adt.Conjunct) {
+ if src := c.Expr().Source(); src != nil {
+ t.remove[src] = true
}
}
-func (t *trimSet) path() string {
- return strings.Join(t.stack[1:], " ")
+const dominatorNode = adt.ComprehensionSpan | adt.DefinitionSpan | adt.ConstraintSpan
+
+func isDominator(c adt.Conjunct) bool {
+ return c.CloseInfo.IsInOneOf(dominatorNode)
}
-func (t *trimSet) traceMsg(msg string) {
- if t.cfg.Trace {
- fmt.Print(t.path())
- msg = strings.TrimRight(msg, "\n")
- msg = strings.Replace(msg, "\n", "\n ", -1)
- fmt.Printf(": %s\n", msg)
- }
-}
-
-func (t *trimSet) markNodes(n ast.Node) {
- ast.Walk(n, nil, func(n ast.Node) {
- switch x := n.(type) {
- case *ast.Ident:
- if x.Node != nil {
- t.exclude[x.Node] = true
- }
- if x.Scope != nil {
- t.exclude[x.Scope] = true
- }
-
- case *ast.ListLit:
- _, e := internal.ListEllipsis(x)
- if e != nil && e.Type != nil {
- t.markAlwaysGen(e.Type, false)
- }
-
- case *ast.Field:
- switch x.Label.(type) {
- case *ast.TemplateLabel, *ast.ListLit:
- t.markAlwaysGen(x.Value, false)
- }
-
- case *ast.ListComprehension, *ast.Comprehension:
- t.markAlwaysGen(x, true)
- }
- })
-}
-
-func (t *trimSet) markAlwaysGen(first ast.Node, isComp bool) {
- ast.Walk(first, func(n ast.Node) bool {
- if t.alwaysGen[n] {
- return false
- }
- t.alwaysGen[n] = true
- if isComp {
- t.fromComp[n] = true
- }
- if x, ok := n.(*ast.Ident); ok && n != first {
- // Also mark any value used within a constraint from an optional field.
- if x.Node != nil {
- // fmt.Println("MARKED", internal.DebugStr(x.Node),
- // "by", internal.DebugStr(first))
- // t.markAlwaysGen(x.Node)
- }
- }
- return true
- }, nil)
-}
-
-func (t *trimSet) canRemove(n ast.Node) bool {
- return !t.exclude[n] && !t.alwaysGen[n]
-}
-
-func isDisjunctionOfStruct(n ast.Node) bool {
- switch x := n.(type) {
- case *ast.BinaryExpr:
- if x.Op == token.OR {
- return hasStruct(x.X) || hasStruct(x.Y)
- }
- }
- return false
-}
-
-func hasStruct(n ast.Node) bool {
- hasStruct := false
- ast.Walk(n, func(n ast.Node) bool {
- if _, ok := n.(*ast.StructLit); ok {
- hasStruct = true
- }
- return !hasStruct
- }, nil)
- return hasStruct
-}
-
-// trim strips fields from structs that would otherwise be generated by implied
-// content, such as optional fields turned required, comprehensions, and list types.
-//
-// The algorithm walks the tree with two values in parallel: one for the full
-// configuration, and one for implied content. For each node in the tree it
-// determines the value of the implied content and that of the full value
-// and strips any of the non-implied fields if it subsumes the implied ones.
-//
-// There are a few gotchas:
-// - Fields in the implied content may refer to fields in the complete config.
-// To support this, incomplete fields are detected and evaluated within the
-// configuration.
-// - Values of optional fields are instantiated as a result of the declaration
-// of concrete sibling fields. Such fields should not be removed even if the
-// instantiated constraint completely subsumes such fields as the reason to
-// apply the optional constraint will disappear with it.
-// - As the parallel structure is different, it may resolve to different
-// default values. There is no support yet for selecting defaults of a value
-// based on another value without doing a full unification. So for now we
-// skip any disjunction containing structs.
-//
-// v the current value
-// m the "mixed-in" values
-// scope in which to evaluate expressions
-// rmSet nodes in v that may be removed by the caller
-func (t *trimSet) trim(label string, v, m, scope cue.Value) (rmSet []ast.Node) {
- saved := t.stack
- t.stack = append(t.stack, label)
- defer func() { t.stack = saved }()
-
- vSplit := v.Split()
-
- // At the moment disjunctions of structs are not supported. Detect them and
- // punt.
- // TODO: support disjunctions.
- mSplit := m.Split()
- for _, v := range mSplit {
- if isDisjunctionOfStruct(v.Source()) {
- return
- }
- }
-
- // Collect generated nodes.
- // Only keep the good parts of the optional constraint.
- // Incoming structs may be incomplete resulting in errors. It is safe
- // to ignore these. If there is an actual error, it will manifest in
- // the evaluation of v.
- in := cue.Value{}
- gen := []ast.Node{}
- for _, v := range mSplit {
- // TODO: consider resolving incomplete values within the current
- // scope, as we do for fields.
- if v.Exists() {
- in = in.UnifyAccept(v, m)
- }
- gen = append(gen, v.Source())
- }
-
- accept := v
- comp := in
-
- // Identify generated components and unify them with the mixin value.
- exists := false
- for _, v := range vSplit {
- src := v.Source()
- alwaysGen := t.alwaysGen[src]
- inNodes := inNodes(gen, src)
- fromComp := t.fromComp[src]
- if fromComp {
- gen = append(gen, src)
- comp = comp.UnifyAccept(v, accept)
- continue
- }
- if !(alwaysGen || inNodes) {
- continue
- }
-
- if !v.IsConcrete() {
- // The template has an expression that cannot be fully
- // resolved. Attempt to complete the expression by
- // evaluting it within the struct to which the template
- // is applied.
- expr := internal.ToExpr(v.Syntax())
-
- // TODO: this only resolves references contained in scope.
- v = internal.EvalExpr(scope, expr).(cue.Value)
- }
-
- if w := in.UnifyAccept(v, accept); w.Err() == nil {
- in = w
- }
- // One of the sources of this struct is generated. That means
- // we can safely delete a non-generated version.
- exists = true
- gen = append(gen, src)
- }
-
- switch v.Kind() {
- case cue.StructKind:
- // Build map of mixin fields.
- valueMap := map[key]cue.Value{}
- for mIter, _ := in.Fields(cue.All(), cue.Optional(false)); mIter.Next(); {
- valueMap[iterKey(mIter)] = mIter.Value()
- }
-
- fn := v.Template()
-
- // Process fields.
- rm := []ast.Node{}
- for iter, _ := v.Fields(cue.All()); iter.Next(); {
- mSub := valueMap[iterKey(iter)]
- if fn != nil {
- label := iter.Label()
- w := fn(label)
- mSub = mSub.Unify(w)
- }
-
- label := iter.Label()
- removed := t.trim(label, iter.Value(), mSub, v)
- rm = append(rm, removed...)
- }
-
- canRemove := fn == nil
- for _, v := range vSplit {
- src := v.Source()
- if t.fromComp[src] {
- canRemove = true
- }
- }
-
- // Remove fields from source.
- for _, v := range vSplit {
- if src := v.Source(); !t.alwaysGen[src] {
- switch x := src.(type) {
- case *ast.File:
- // TODO: use in instead?
- x.Decls = t.trimDecls(x.Decls, rm, m, canRemove)
-
- case *ast.StructLit:
- x.Elts = t.trimDecls(x.Elts, rm, m, canRemove)
- exists = exists || m.Exists()
- if len(x.Elts) == 0 && exists && t.canRemove(src) && !inNodes(gen, src) {
- rmSet = append(rmSet, src)
- }
-
- default:
- // Currently the ast.Files are not associated with the
- // nodes. We detect this case here and iterate over all
- // files.
- // TODO: fix this. See cue/instance.go.
- if len(t.stack) == 1 {
- for _, x := range t.files {
- x.Decls = t.trimDecls(x.Decls, rm, m, canRemove)
- }
- }
- }
- }
- }
-
- if t.cfg.Trace {
- w := &bytes.Buffer{}
- fmt.Fprintln(w)
- fmt.Fprintln(w, "value: ", v)
- if in.Exists() {
- fmt.Fprintln(w, "mixed in: ", in)
- }
- for _, v := range vSplit {
- status := "[]"
- src := v.Source()
- if inNodes(rmSet, src) {
- status = "[re]"
- } else if t.alwaysGen[src] {
- status = "[i]"
- }
- fmt.Fprintf(w, " %4s %v: %v %T\n", status, v.Pos(), internal.DebugStr(src), src)
- }
-
- t.traceMsg(w.String())
- }
-
- case cue.ListKind:
- mIter, _ := m.List()
- elem, hasElem := m.Elem()
- i := 0
- rmElem := []ast.Node{}
- allowRemove := true
- for iter, _ := v.List(); iter.Next(); i++ {
- var m cue.Value
- if mIter.Next() {
- m = mIter.Value()
- } else if hasElem {
- m = elem
- allowRemove = false
- } else {
- allowRemove = false
- break
- }
- rm := t.trim(strconv.Itoa(i), iter.Value(), m, scope)
- rmElem = append(rmElem, rm...)
- }
-
- // Signal the removal of lists of which all elements have been marked
- // for removal.
- if allowRemove {
- for _, v := range vSplit {
- if src := v.Source(); !t.alwaysGen[src] {
- l, ok := src.(*ast.ListLit)
- if !ok {
- break
- }
- rmList := true
- iter, _ := v.List()
- for i := 0; i < len(l.Elts) && iter.Next(); i++ {
- if !inNodes(rmElem, l.Elts[i]) {
- rmList = false
- break
- }
- }
- if rmList && m.Exists() && t.canRemove(src) && !inNodes(gen, src) {
- rmSet = append(rmSet, src)
- }
- }
- }
- }
- fallthrough
-
- default:
- // Mark any subsumed part that is covered by generated config.
- in, _ := in.Default()
- if in.Err() == nil && v.Subsume(in) == nil {
- for _, v := range vSplit {
- src := v.Source()
- if t.canRemove(src) && !inNodes(gen, src) &&
- exists {
- rmSet = append(rmSet, src)
- }
- }
- }
-
- if t.cfg.Trace {
- w := &bytes.Buffer{}
- if len(rmSet) > 0 {
- fmt.Fprint(w, "field: SUBSUMED\n")
- } else {
- fmt.Fprint(w, "field: \n")
- }
- fmt.Fprintln(w, "value: ", v)
- if in.Exists() {
- fmt.Fprintln(w, "mixed in: ", in)
- }
- for _, v := range vSplit {
- status := "["
- if inNodes(gen, v.Source()) {
- status += "i"
- }
- if inNodes(rmSet, v.Source()) {
- status += "r"
- }
- status += "]"
- src := v.Source()
- fmt.Fprintf(w, " %4s %v: %v\n", status, v.Pos(), internal.DebugStr(src))
- }
-
- t.traceMsg(w.String())
- }
- }
-
- if comp.Exists() {
- if v.Subsume(comp) == nil {
- for _, v := range vSplit {
- src := v.Source()
- if !inNodes(gen, src) {
- rmSet = append(rmSet, src)
- }
- }
- }
- }
-
- return rmSet
-}
-
-func (t *trimSet) trimDecls(decls []ast.Decl, rm []ast.Node, m cue.Value, allow bool) []ast.Decl {
- a := make([]ast.Decl, 0, len(decls))
-
- for _, d := range decls {
- if f, ok := d.(*ast.Field); ok {
- label, _, err := ast.LabelName(f.Label)
- v := m.Lookup(label)
- inNodes := inNodes(rm, f.Value)
- ok := allow || v.Exists()
- if err == nil && inNodes && ok {
- continue
- }
- }
- a = append(a, d)
- }
- return a
-}
-
-func inNodes(a []ast.Node, n ast.Node) bool {
- for _, e := range a {
- if e == n {
+// Roots of constraints are not allowed to strip conjuncts by
+// themselves as it will eliminate the reason for the trigger.
+func allowRemove(v *adt.Vertex) bool {
+ for _, c := range v.Conjuncts {
+ if isDominator(c) &&
+ (c.CloseInfo.Location() != c.Expr() ||
+ c.CloseInfo.RootSpanType() != adt.ConstraintSpan) {
return true
}
}
return false
}
-type key struct {
- label string
- hidden bool
-}
+// A parent may be removed if there is not a `no` and there is at least one
+// `yes`. A `yes` is proves that there is at least one node that is not a
+// dominator node and that we are not removing nodes from a declaration of a
+// dominator itself.
+const (
+ no = 1 << iota
+ maybe
+ yes
+)
-func iterKey(v *cue.Iterator) key {
- return key{v.Label(), v.IsHidden()}
+func (t *trimmer) findSubordinates(v *adt.Vertex) int {
+ // TODO(structure sharing): do not descend into vertices whose parent is not
+ // equal to the parent. This is not relevant at this time, but may be so in
+ // the future.
+
+ if len(v.Arcs) > 0 {
+ var match int
+ for _, a := range v.Arcs {
+ match |= t.findSubordinates(a)
+ }
+
+ // This also skips embedded scalars if not all fields are removed. In
+ // this case we need to preserve the scalar to keep the type of the
+ // struct intact, which might as well be done by not removing the scalar
+ // type.
+ if match&yes == 0 || match&no != 0 {
+ return match
+ }
+ }
+
+ if !allowRemove(v) {
+ return no
+ }
+
+ switch v.BaseValue.(type) {
+ case *adt.StructMarker, *adt.ListMarker:
+ // Rely on previous processing of the Arcs and the fact that we take the
+ // default value to check dominator subsumption, meaning that we don't
+ // have to check additional optional constraints to pass subsumption.
+
+ default:
+ doms := &adt.Vertex{}
+ hasSubs := false
+
+ for _, c := range v.Conjuncts {
+ if isDominator(c) {
+ doms.AddConjunct(c)
+ } else {
+ hasSubs = true
+ }
+ }
+
+ if !hasSubs {
+ return maybe // only if there are siblings to be removed.
+ }
+
+ doms.Finalize(t.ctx)
+ doms = doms.Default()
+
+ // This is not necessary, but seems like it may result in more
+ // user-friendly semantics.
+ if doms.IsErr() || v.IsErr() {
+ return no
+ }
+
+ // TODO: since we take v, instead of the unification of subordinate
+ // values, it should suffice to take equality here:
+ // doms ⊑ subs ==> doms == subs&doms
+ if err := subsume.Value(t.ctx, v, doms); err != nil {
+ return no
+ }
+ }
+
+ for _, c := range v.Conjuncts {
+ if !isDominator(c) {
+ t.markRemove(c)
+ }
+ }
+
+ return yes
}
diff --git a/tools/trim/trim_test.go b/tools/trim/trim_test.go
index 04de0c2..167192a 100644
--- a/tools/trim/trim_test.go
+++ b/tools/trim/trim_test.go
@@ -79,6 +79,7 @@
c: b: 3
z: {
+
a: b: 3
for k, v in a {
c: "\(k)": v
@@ -92,6 +93,7 @@
}
z: {
+
a: b: 3
for k, v in a {
c: "\(k)": v
@@ -135,7 +137,7 @@
x: >=5 & <=8 & int
}
- t: u: { x: 5 } // TODO: should be t: u: {}
+ t: u: { x: 5 }
}
group: {
@@ -157,7 +159,7 @@
x: >=5 & <=8 & int
}
- t: u: {x: 5}
+ t: u: {}
}
group: {
@@ -184,6 +186,22 @@
}
`,
}, {
+ name: "list removal",
+ in: `
+ service: [string]: {
+ ports: [{a: 1}, {a: 1}, ...{ extra: 3 }]
+ }
+ service: a: {
+ ports: [{a: 1}, {a: 1,}]
+ }
+ `,
+ out: `service: [string]: {
+ ports: [{a: 1}, {a: 1}, ...{extra: 3}]
+}
+service: a: {
+}
+`,
+ }, {
name: "do not overmark comprehension",
in: `
foo: multipath: {
@@ -213,36 +231,25 @@
}
`,
}, {
- name: "list removal",
+ name: "remove implied interpolations",
in: `
- service: [string]: {
- ports: [{a: 1}, {a: 1}, ...{ extra: 3 }]
- }
- service: a: {
- ports: [{a: 1}, {a: 1, extra: 3}, {}, { extra: 3 }]
- }
-`,
- out: `service: [string]: {
- ports: [{a: 1}, {a: 1}, ...{extra: 3}]
+ foo: [string]: {
+ a: string
+ b: "--\(a)--"
+ }
+ foo: entry: {
+ a: "insert"
+ b: "--insert--"
+ }
+ `,
+ out: `foo: [string]: {
+ a: string
+ b: "--\(a)--"
}
-service: a: {
- ports: [{}, {extra: 3}, {}, {}]
+foo: entry: {
+ a: "insert"
}
`,
- // }, {
- // TODO: This used to work.
- // name: "remove implied interpolations",
- // in: `
- // foo: [string]: {
- // a: string
- // b: "--\(a)--"
- // }
- // foo: entry: {
- // a: "insert"
- // b: "--insert--"
- // }
- // `,
- // out: ``,
}}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
@@ -250,7 +257,7 @@
if err != nil {
t.Fatal(err)
}
- r := Runtime{}
+ r := cue.Runtime{}
inst, err := r.CompileFile(f)
if err != nil {
t.Fatal(err)
