From 9938e1c9dddaa29df5f3326aeb2143d3bb89da85 Mon Sep 17 00:00:00 2001 From: Matthew Sackman Date: Thu, 26 Dec 2024 08:16:47 +0000 Subject: [PATCH] tools/trim: implement trim for evalv3 Trim now works on the result of evaluation for evalv3. This new implementation only works for evalv3, and does not alter any code paths for evalv2 and the existing trim. The new implementation addresses all known bugs with trim. Signed-off-by: Matthew Sackman Change-Id: If814b5f66ce2ea3ce37ddcbec71eb38f1bc97216 Dispatch-Trailer: {"type":"trybot","CL":1208658,"patchset":5,"ref":"refs/changes/58/1208658/5","targetBranch":"master"} --- .../cmd/testdata/script/trim_package.txtar | 63 + internal/core/subsume/vertex.go | 3 + tools/trim/testdata/1.txtar | 10 + tools/trim/testdata/10.txtar | 38 + tools/trim/testdata/11.txtar | 19 + tools/trim/testdata/12.txtar | 19 + tools/trim/testdata/13.txtar | 19 + tools/trim/testdata/14.txtar | 19 + tools/trim/testdata/15.txtar | 19 + tools/trim/testdata/16.txtar | 25 + tools/trim/testdata/17.txtar | 42 + tools/trim/testdata/18.txtar | 6 + tools/trim/testdata/19.txtar | 6 + tools/trim/testdata/2.txtar | 10 + tools/trim/testdata/20.txtar | 29 + tools/trim/testdata/21.txtar | 10 + tools/trim/testdata/22.txtar | 27 + tools/trim/testdata/23.txtar | 5 + tools/trim/testdata/24.txtar | 11 + tools/trim/testdata/25.txtar | 26 + tools/trim/testdata/26.txtar | 38 + tools/trim/testdata/27.txtar | 14 + tools/trim/testdata/28.txtar | 23 + tools/trim/testdata/29.txtar | 19 + tools/trim/testdata/3.txtar | 21 + tools/trim/testdata/30.txtar | 30 + tools/trim/testdata/31.txtar | 12 + tools/trim/testdata/32.txtar | 23 + tools/trim/testdata/33.txtar | 32 + tools/trim/testdata/34.txtar | 7 + tools/trim/testdata/35.txtar | 53 + tools/trim/testdata/36.txtar | 59 + tools/trim/testdata/37.txtar | 70 ++ tools/trim/testdata/38.txtar | 61 + tools/trim/testdata/39.txtar | 40 + tools/trim/testdata/4.txtar | 8 + tools/trim/testdata/40.txtar | 21 + tools/trim/testdata/41.txtar | 90 ++ tools/trim/testdata/42.txtar | 37 + tools/trim/testdata/43.txtar | 31 + tools/trim/testdata/44.txtar | 55 + tools/trim/testdata/45.txtar | 30 + tools/trim/testdata/46.txtar | 35 + tools/trim/testdata/47.txtar | 28 + tools/trim/testdata/48.txtar | 29 + tools/trim/testdata/49.txtar | 77 ++ tools/trim/testdata/5.txtar | 17 + tools/trim/testdata/50.txtar | 33 + tools/trim/testdata/51.txtar | 24 + tools/trim/testdata/52.txtar | 45 + tools/trim/testdata/53.txtar | 19 + tools/trim/testdata/54.txtar | 22 + tools/trim/testdata/55.txtar | 20 + tools/trim/testdata/56.txtar | 17 + tools/trim/testdata/57.txtar | 45 + tools/trim/testdata/58.txtar | 27 + tools/trim/testdata/59.txtar | 15 + tools/trim/testdata/6.txtar | 20 + tools/trim/testdata/60.txtar | 17 + tools/trim/testdata/7.txtar | 29 + tools/trim/testdata/8.txtar | 38 + tools/trim/testdata/9.txtar | 49 + tools/trim/testdata/constraintroots.txtar | 151 ++- tools/trim/testdata/defaults.txtar | 81 ++ .../defaults_can_remove_non-defaults.txtar | 13 + .../do_not_overmark_comprehension.txtar | 27 + tools/trim/testdata/do_not_remove_field.txtar | 17 + .../trim/testdata/do_not_remove_field_2.txtar | 21 + tools/trim/testdata/kube1.txtar | 38 + tools/trim/testdata/list_removal_1.txtar | 19 + tools/trim/testdata/list_removal_2.txtar | 28 + tools/trim/testdata/optional.txtar | 28 + .../remove_due_to_simplification.txtar | 36 + tools/trim/testdata/rmimport.txtar | 23 + tools/trim/testdata/shared.txtar | 16 +- tools/trim/trim.go | 9 +- tools/trim/trim_test.go | 2 +- tools/trim/trimv3.go | 1108 +++++++++++++++++ 78 files changed, 3372 insertions(+), 31 deletions(-) create mode 100644 cmd/cue/cmd/testdata/script/trim_package.txtar create mode 100644 tools/trim/testdata/1.txtar create mode 100644 tools/trim/testdata/10.txtar create mode 100644 tools/trim/testdata/11.txtar create mode 100644 tools/trim/testdata/12.txtar create mode 100644 tools/trim/testdata/13.txtar create mode 100644 tools/trim/testdata/14.txtar create mode 100644 tools/trim/testdata/15.txtar create mode 100644 tools/trim/testdata/16.txtar create mode 100644 tools/trim/testdata/17.txtar create mode 100644 tools/trim/testdata/18.txtar create mode 100644 tools/trim/testdata/19.txtar create mode 100644 tools/trim/testdata/2.txtar create mode 100644 tools/trim/testdata/20.txtar create mode 100644 tools/trim/testdata/21.txtar create mode 100644 tools/trim/testdata/22.txtar create mode 100644 tools/trim/testdata/23.txtar create mode 100644 tools/trim/testdata/24.txtar create mode 100644 tools/trim/testdata/25.txtar create mode 100644 tools/trim/testdata/26.txtar create mode 100644 tools/trim/testdata/27.txtar create mode 100644 tools/trim/testdata/28.txtar create mode 100644 tools/trim/testdata/29.txtar create mode 100644 tools/trim/testdata/3.txtar create mode 100644 tools/trim/testdata/30.txtar create mode 100644 tools/trim/testdata/31.txtar create mode 100644 tools/trim/testdata/32.txtar create mode 100644 tools/trim/testdata/33.txtar create mode 100644 tools/trim/testdata/34.txtar create mode 100644 tools/trim/testdata/35.txtar create mode 100644 tools/trim/testdata/36.txtar create mode 100644 tools/trim/testdata/37.txtar create mode 100644 tools/trim/testdata/38.txtar create mode 100644 tools/trim/testdata/39.txtar create mode 100644 tools/trim/testdata/4.txtar create mode 100644 tools/trim/testdata/40.txtar create mode 100644 tools/trim/testdata/41.txtar create mode 100644 tools/trim/testdata/42.txtar create mode 100644 tools/trim/testdata/43.txtar create mode 100644 tools/trim/testdata/44.txtar create mode 100644 tools/trim/testdata/45.txtar create mode 100644 tools/trim/testdata/46.txtar create mode 100644 tools/trim/testdata/47.txtar create mode 100644 tools/trim/testdata/48.txtar create mode 100644 tools/trim/testdata/49.txtar create mode 100644 tools/trim/testdata/5.txtar create mode 100644 tools/trim/testdata/50.txtar create mode 100644 tools/trim/testdata/51.txtar create mode 100644 tools/trim/testdata/52.txtar create mode 100644 tools/trim/testdata/53.txtar create mode 100644 tools/trim/testdata/54.txtar create mode 100644 tools/trim/testdata/55.txtar create mode 100644 tools/trim/testdata/56.txtar create mode 100644 tools/trim/testdata/57.txtar create mode 100644 tools/trim/testdata/58.txtar create mode 100644 tools/trim/testdata/59.txtar create mode 100644 tools/trim/testdata/6.txtar create mode 100644 tools/trim/testdata/60.txtar create mode 100644 tools/trim/testdata/7.txtar create mode 100644 tools/trim/testdata/8.txtar create mode 100644 tools/trim/testdata/9.txtar create mode 100644 tools/trim/testdata/do_not_remove_field_2.txtar create mode 100644 tools/trim/trimv3.go diff --git a/cmd/cue/cmd/testdata/script/trim_package.txtar b/cmd/cue/cmd/testdata/script/trim_package.txtar new file mode 100644 index 000000000..c8f191bd7 --- /dev/null +++ b/cmd/cue/cmd/testdata/script/trim_package.txtar @@ -0,0 +1,63 @@ +exec cue mod init + +# -- evalv3 -- +env CUE_EXPERIMENT=evalv3=1 +exec cue trim -o - ./... +cmp stdout expect-stdout-v3 + +# -- evalv2 -- +env CUE_EXPERIMENT=evalv3=0 +exec cue trim -o - ./... +cmp stdout expect-stdout-v2 + +-- a.cue -- +package p + +x: y: int +x: z: int +-- b/b.cue -- +package p + +x: y: 7 +-- c/c.cue -- +package p + +x: z: 6 +-- expect-stdout-v3 -- +package p + +x: y: int +x: z: int +package p + +x: y: int +x: z: int +package p + +x: y: 7 +package p + +x: y: int +x: z: int +package p + +x: z: 6 +-- expect-stdout-v2 -- +package p + +x: y: int +x: z: int +package p + +x: y: int +x: z: int +package p + +x: y: 7 +package p + +x: y: int +x: z: int +package p + +x: z: 6 diff --git a/internal/core/subsume/vertex.go b/internal/core/subsume/vertex.go index adb164296..855ce6fbc 100644 --- a/internal/core/subsume/vertex.go +++ b/internal/core/subsume/vertex.go @@ -291,6 +291,9 @@ func (s *subsumer) verticesDev(x, y *adt.Vertex) bool { return true } + case nil: + return false + default: panic(fmt.Sprintf("unexpected type %T", v)) } diff --git a/tools/trim/testdata/1.txtar b/tools/trim/testdata/1.txtar new file mode 100644 index 000000000..e4d45cdc5 --- /dev/null +++ b/tools/trim/testdata/1.txtar @@ -0,0 +1,10 @@ +TODO. This should get simplified in the same way as 6, but +doesn't. Possible bug in evaluator. + +-- a.cue -- +<10 +7 +-- out/trim -- +== a.cue +<10 +7 diff --git a/tools/trim/testdata/10.txtar b/tools/trim/testdata/10.txtar new file mode 100644 index 000000000..47d2dc071 --- /dev/null +++ b/tools/trim/testdata/10.txtar @@ -0,0 +1,38 @@ +Similar to 9, but with list instead of struct. Note the simplification +of the pattern too. + +See also 60. + +-- a.cue -- +d: [ + {name: "jack", age: 5}, + {name: "gill", age: >4}, + {name: "hilbert", age: int}, +] +d: [...{name: string, age: 5, age: number}] +-- out/trim-v3 -- +== a.cue +d: [ + {name: "jack"}, + {name: "gill"}, + {name: "hilbert"}, +] +d: [...{name: string, age: 5}] +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -4,4 +4,4 @@ + {name: "gill"}, + {name: "hilbert"}, + ] +-d: [...{name: string, age: 5, age: number}] ++d: [...{name: string, age: 5}] +-- out/trim -- +== a.cue +d: [ + {name: "jack"}, + {name: "gill"}, + {name: "hilbert"}, +] +d: [...{name: string, age: 5, age: number}] diff --git a/tools/trim/testdata/11.txtar b/tools/trim/testdata/11.txtar new file mode 100644 index 000000000..f849cc5ba --- /dev/null +++ b/tools/trim/testdata/11.txtar @@ -0,0 +1,19 @@ +Explicit unification of structs can lead to simplification. +Contrast with 4. + +-- a.cue -- +x: {a: bool, b: "hi"} & {a: true, b: string} +-- out/trim-v3 -- +== a.cue +x: {b: "hi"} & {a: true} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,2 +1,2 @@ + == a.cue +-x: {a: bool, b: "hi"} & {a: true, b: string} ++x: {b: "hi"} & {a: true} +-- out/trim -- +== a.cue +x: {a: bool, b: "hi"} & {a: true, b: string} diff --git a/tools/trim/testdata/12.txtar b/tools/trim/testdata/12.txtar new file mode 100644 index 000000000..dc3ce8df5 --- /dev/null +++ b/tools/trim/testdata/12.txtar @@ -0,0 +1,19 @@ +A variant of 11, with fields slightly rearranged. We do not currently +simplify explicit unification where one side is top. + +-- a.cue -- +x: {a: bool, b: string} & {a: true, b: "hi"} +-- out/trim-v3 -- +== a.cue +x: _ & {a: true, b: "hi"} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,2 +1,2 @@ + == a.cue +-x: {a: bool, b: string} & {a: true, b: "hi"} ++x: _ & {a: true, b: "hi"} +-- out/trim -- +== a.cue +x: {a: bool, b: string} & {a: true, b: "hi"} diff --git a/tools/trim/testdata/13.txtar b/tools/trim/testdata/13.txtar new file mode 100644 index 000000000..763481e56 --- /dev/null +++ b/tools/trim/testdata/13.txtar @@ -0,0 +1,19 @@ +A variant of 12, to ensure the ellipsis does not prevent +simplification of the struct to top. + +-- a.cue -- +x: {a: bool, b: string, ...} & {a: true, b: "hi"} +-- out/trim-v3 -- +== a.cue +x: _ & {a: true, b: "hi"} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,2 +1,2 @@ + == a.cue +-x: {a: bool, b: string, ...} & {a: true, b: "hi"} ++x: _ & {a: true, b: "hi"} +-- out/trim -- +== a.cue +x: {a: bool, b: string, ...} & {a: true, b: "hi"} diff --git a/tools/trim/testdata/14.txtar b/tools/trim/testdata/14.txtar new file mode 100644 index 000000000..be0d4d976 --- /dev/null +++ b/tools/trim/testdata/14.txtar @@ -0,0 +1,19 @@ +Further variation on 11: make sure patterns are kept, even if they're +not used. + +-- a.cue -- +x: {a: bool, b: string, [>"b"]: int} & {a: true, b: "hi"} +-- out/trim-v3 -- +== a.cue +x: {[>"b"]: int} & {a: true, b: "hi"} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,2 +1,2 @@ + == a.cue +-x: {a: bool, b: string, [>"b"]: int} & {a: true, b: "hi"} ++x: {[>"b"]: int} & {a: true, b: "hi"} +-- out/trim -- +== a.cue +x: {a: bool, b: string, [>"b"]: int} & {a: true, b: "hi"} diff --git a/tools/trim/testdata/15.txtar b/tools/trim/testdata/15.txtar new file mode 100644 index 000000000..73c83d01d --- /dev/null +++ b/tools/trim/testdata/15.txtar @@ -0,0 +1,19 @@ +Similar to 11, but with lists: explicit unification of lists can lead +to simplification. + +-- a.cue -- +x: [int, int, int] & [9, 8, 7] +-- out/trim-v3 -- +== a.cue +x: [_, _, _] & [9, 8, 7] +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,2 +1,2 @@ + == a.cue +-x: [int, int, int] & [9, 8, 7] ++x: [_, _, _] & [9, 8, 7] +-- out/trim -- +== a.cue +x: [int, int, int] & [9, 8, 7] diff --git a/tools/trim/testdata/16.txtar b/tools/trim/testdata/16.txtar new file mode 100644 index 000000000..1a0e77712 --- /dev/null +++ b/tools/trim/testdata/16.txtar @@ -0,0 +1,25 @@ +Like 11, but using a let comprehension. For trim, the let +comprehension is treated the same as a field. I.e. in this case, it's +no different to s: {a: bool, b: "hi"}, and so no simplification can +occur. + +-- a.cue -- +let s = {a: bool, b: "hi"} +x: s & {a: true, b: string} +-- out/trim-v3 -- +== a.cue +let s = {a: bool, b: "hi"} +x: s & {a: true} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,3 +1,3 @@ + == a.cue + let s = {a: bool, b: "hi"} +-x: s & {a: true, b: string} ++x: s & {a: true} +-- out/trim -- +== a.cue +let s = {a: bool, b: "hi"} +x: s & {a: true, b: string} diff --git a/tools/trim/testdata/17.txtar b/tools/trim/testdata/17.txtar new file mode 100644 index 000000000..cbae89846 --- /dev/null +++ b/tools/trim/testdata/17.txtar @@ -0,0 +1,42 @@ +Variant of 9, but with more complex field expressions. As before, we +must ignore the conjuncts from the pattern when simplifying the +fields, so that we don't accidentally remove the entire field. + +-- a.cue -- +d: { + jack: {age: 5, age: 5} + gill: {age: 5} + hilbert: {age: 5} & {age: 5} + [string]: {age: 5} +} +-- out/trim-v3 -- +== a.cue +d: { + jack: _ + gill: _ + hilbert: _ & _ + [string]: {age: 5} +} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,7 +1,7 @@ + == a.cue + d: { +- jack: {} +- gill: {} +- hilbert: {} & {} ++ jack: _ ++ gill: _ ++ hilbert: _ & _ + [string]: {age: 5} + } +-- out/trim -- +== a.cue +d: { + jack: {} + gill: {} + hilbert: {} & {} + [string]: {age: 5} +} diff --git a/tools/trim/testdata/18.txtar b/tools/trim/testdata/18.txtar new file mode 100644 index 000000000..e8af955f8 --- /dev/null +++ b/tools/trim/testdata/18.txtar @@ -0,0 +1,6 @@ +Make sure we keep patterns for lists too. +-- a.cue -- +x: [...string] +-- out/trim -- +== a.cue +x: [...string] diff --git a/tools/trim/testdata/19.txtar b/tools/trim/testdata/19.txtar new file mode 100644 index 000000000..ce8bac3af --- /dev/null +++ b/tools/trim/testdata/19.txtar @@ -0,0 +1,6 @@ +Make sure we keep *all* of a pattern, not just its root. +-- a.cue -- +x: [string]: a: b: c: d: _ +-- out/trim -- +== a.cue +x: [string]: a: b: c: d: _ diff --git a/tools/trim/testdata/2.txtar b/tools/trim/testdata/2.txtar new file mode 100644 index 000000000..8fadb40ae --- /dev/null +++ b/tools/trim/testdata/2.txtar @@ -0,0 +1,10 @@ +TODO: This should get simplified in the same way as 3, but +doesn't. Possible bug in evaluator. + +-- a.cue -- +string +"hi" +-- out/trim -- +== a.cue +string +"hi" diff --git a/tools/trim/testdata/20.txtar b/tools/trim/testdata/20.txtar new file mode 100644 index 000000000..c032926a9 --- /dev/null +++ b/tools/trim/testdata/20.txtar @@ -0,0 +1,29 @@ +Simplifications can occur within definitions, but we must be sure to +keep the ellipsis. + +-- a.cue -- +let #x = {a: int, b: {c: 5, c: >4, ...}} +y: #x & {a: 4, b: {c: int}} +z: #x & {a: 4, b: {c: int, d: "hi"}} +-- out/trim-v3 -- +== a.cue +let #x = {a: int, b: {c: 5, ...}} +y: #x & {a: 4} +z: #x & {a: 4, b: {d: "hi"}} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,4 +1,4 @@ + == a.cue +-let #x = {a: int, b: {c: 5, c: >4, ...}} +-y: #x & {a: 4, b: {c: int}} +-z: #x & {a: 4, b: {c: int, d: "hi"}} ++let #x = {a: int, b: {c: 5, ...}} ++y: #x & {a: 4} ++z: #x & {a: 4, b: {d: "hi"}} +-- out/trim -- +== a.cue +let #x = {a: int, b: {c: 5, c: >4, ...}} +y: #x & {a: 4, b: {c: int}} +z: #x & {a: 4, b: {c: int, d: "hi"}} diff --git a/tools/trim/testdata/21.txtar b/tools/trim/testdata/21.txtar new file mode 100644 index 000000000..087845dc6 --- /dev/null +++ b/tools/trim/testdata/21.txtar @@ -0,0 +1,10 @@ +We do no simplification of arguments to function calls. This includes +close, which appears in the AST as a normal CallExpr. + +-- a.cue -- +y: close({a: int, b: {c: string}}) & {a: 4, b: {c: "hi"}} +z: close({a: int, b: {c: string}} & {a: 4, b: {c: "hi"}}) +-- out/trim -- +== a.cue +y: close({a: int, b: {c: string}}) & {a: 4, b: {c: "hi"}} +z: close({a: int, b: {c: string}} & {a: 4, b: {c: "hi"}}) diff --git a/tools/trim/testdata/22.txtar b/tools/trim/testdata/22.txtar new file mode 100644 index 000000000..44d9a6751 --- /dev/null +++ b/tools/trim/testdata/22.txtar @@ -0,0 +1,27 @@ +As with 21, make sure we don't do simplification of arguments to +function calls - we've not just special-cased "close". + +-- a.cue -- +let x = close +y: x({a: 5}) +z: y & {a: int} +-- out/trim-v3 -- +== a.cue +let x = close +y: x({a: 5}) +z: y & _ +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,4 +1,4 @@ + == a.cue + let x = close + y: x({a: 5}) +-z: y & {a: int} ++z: y & _ +-- out/trim -- +== a.cue +let x = close +y: x({a: 5}) +z: y & {a: int} diff --git a/tools/trim/testdata/23.txtar b/tools/trim/testdata/23.txtar new file mode 100644 index 000000000..2c98cd583 --- /dev/null +++ b/tools/trim/testdata/23.txtar @@ -0,0 +1,5 @@ +-- a.cue -- +x: (2 + 3) & (1 + 4) +-- out/trim -- +== a.cue +x: (2 + 3) & (1 + 4) diff --git a/tools/trim/testdata/24.txtar b/tools/trim/testdata/24.txtar new file mode 100644 index 000000000..2b44b4051 --- /dev/null +++ b/tools/trim/testdata/24.txtar @@ -0,0 +1,11 @@ +Variant of 22. + +-- a.cue -- +let x = close +let y = x({a: int}) +z: y & {a: 5} +-- out/trim -- +== a.cue +let x = close +let y = x({a: int}) +z: y & {a: 5} diff --git a/tools/trim/testdata/25.txtar b/tools/trim/testdata/25.txtar new file mode 100644 index 000000000..67985cf05 --- /dev/null +++ b/tools/trim/testdata/25.txtar @@ -0,0 +1,26 @@ +Make sure we do not remove the outer `x: 5` (line 6), even though it +is a tie with the inner `x: 5` (and the inner `x: 5` occurs earlier in +the file (line 2) so it would win in a tie-break). + +What happens here is that we establish that line 1 requires (depends +on) line 6. Even though within x we have a tie between line 2 and line +6, we find that line 1 cannot be removed (because y only has 1 winner +in line 4, and line 4 requires line 1), and so therefore line 6 is +required, and so line 2 is redundant. + +Contrast with 57. + +#skip-v2 +-- a.cue -- +if x == 5 { + x: 5 + x: int + y: "hi" +} +x: 5 +-- out/trim-v3 -- +== a.cue +if x == 5 { + y: "hi" +} +x: 5 diff --git a/tools/trim/testdata/26.txtar b/tools/trim/testdata/26.txtar new file mode 100644 index 000000000..140758847 --- /dev/null +++ b/tools/trim/testdata/26.txtar @@ -0,0 +1,38 @@ +This test demonstrates that it is only the "root" conjuncts within a +pattern that need to be ignored for the purpose of identifying +redundant conjuncts, and not any of their children. + +Thus `b` can be simplified, provided `b.w` survives, which is enough +to ensure it matches the pattern. + +-- a.cue -- +a: [string]: { + x: y: z: 5 +} +b: w: x: y: {} +b: a +-- out/trim-v3 -- +== a.cue +a: [string]: { + x: y: z: 5 +} +b: w: _ +b: a +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -2,5 +2,5 @@ + a: [string]: { + x: y: z: 5 + } +-b: w: x: y: {} ++b: w: _ + b: a +-- out/trim -- +== a.cue +a: [string]: { + x: y: z: 5 +} +b: w: x: y: {} +b: a diff --git a/tools/trim/testdata/27.txtar b/tools/trim/testdata/27.txtar new file mode 100644 index 000000000..067ba365a --- /dev/null +++ b/tools/trim/testdata/27.txtar @@ -0,0 +1,14 @@ +Again, we don't simplify anything within literal arguments to function +calls. + +-- a.cue -- +y: len({ + let x = {a: 5, a: int} + x +}) +-- out/trim -- +== a.cue +y: len({ + let x = {a: 5, a: int} + x +}) diff --git a/tools/trim/testdata/28.txtar b/tools/trim/testdata/28.txtar new file mode 100644 index 000000000..2658de554 --- /dev/null +++ b/tools/trim/testdata/28.txtar @@ -0,0 +1,23 @@ +If we bring out the arguments to a function call, then we can simplify them. +Compare to 27. + +-- a.cue -- +let x = {a: 5, a: int} +y: len(x) +-- out/trim-v3 -- +== a.cue +let x = {a: 5} +y: len(x) +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,3 +1,3 @@ + == a.cue +-let x = {a: 5, a: int} ++let x = {a: 5} + y: len(x) +-- out/trim -- +== a.cue +let x = {a: 5, a: int} +y: len(x) diff --git a/tools/trim/testdata/29.txtar b/tools/trim/testdata/29.txtar new file mode 100644 index 000000000..bf31991f4 --- /dev/null +++ b/tools/trim/testdata/29.txtar @@ -0,0 +1,19 @@ +A real tie. The final tiebreaker is file position, so the first one we +see wins. + +-- a.cue -- +x: {a: 5} & {a: 5} +-- out/trim-v3 -- +== a.cue +x: {a: 5} & _ +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,2 +1,2 @@ + == a.cue +-x: {a: 5} & {a: 5} ++x: {a: 5} & _ +-- out/trim -- +== a.cue +x: {a: 5} & {a: 5} diff --git a/tools/trim/testdata/3.txtar b/tools/trim/testdata/3.txtar new file mode 100644 index 000000000..161deabc3 --- /dev/null +++ b/tools/trim/testdata/3.txtar @@ -0,0 +1,21 @@ +We do simplify implicit unification. +2 should get simplified in the same way as this. + +-- a.cue -- +x: string +x: "hi" +-- out/trim-v3 -- +== a.cue +x: "hi" +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,3 +1,2 @@ + == a.cue +-x: string + x: "hi" +-- out/trim -- +== a.cue +x: string +x: "hi" diff --git a/tools/trim/testdata/30.txtar b/tools/trim/testdata/30.txtar new file mode 100644 index 000000000..5a3088e42 --- /dev/null +++ b/tools/trim/testdata/30.txtar @@ -0,0 +1,30 @@ +`x` and `y` cannot be eliminated. This also means uses of `x` and `y` +can't be eliminated. So the simplification of `z` can only remove the +literal `{a: 5}`. + +Similar vein to 5. + +-- a.cue -- +x: {a: 5} +y: {a: 5} +z: x & y & {a: 5} +-- out/trim-v3 -- +== a.cue +x: {a: 5} +y: {a: 5} +z: x & y & _ +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,4 +1,4 @@ + == a.cue + x: {a: 5} + y: {a: 5} +-z: x & y & {a: 5} ++z: x & y & _ +-- out/trim -- +== a.cue +x: {a: 5} +y: {a: 5} +z: x & y & {a: 5} diff --git a/tools/trim/testdata/31.txtar b/tools/trim/testdata/31.txtar new file mode 100644 index 000000000..73f23af0e --- /dev/null +++ b/tools/trim/testdata/31.txtar @@ -0,0 +1,12 @@ +Make sure we don't remove conjuncts that discriminate between +disjunction branches. + +issue 2544 + +-- a.cue -- +#A: value?: {x: number, y: number} | *{x: 0, y: 0} | 5 +a: #A & {value: {}} +-- out/trim -- +== a.cue +#A: value?: {x: number, y: number} | *{x: 0, y: 0} | 5 +a: #A & {value: {}} diff --git a/tools/trim/testdata/32.txtar b/tools/trim/testdata/32.txtar new file mode 100644 index 000000000..35ef4b88c --- /dev/null +++ b/tools/trim/testdata/32.txtar @@ -0,0 +1,23 @@ +Variation of 31. + +issue 2544 +-- a.cue -- +#A: value?: {x: number, y: number} | *{x: 0, y: 0} +a: #A & {value: {x: 0, y: 1}} +-- out/trim-v3 -- +== a.cue +#A: value?: {x: number, y: number} | *{x: 0, y: 0} +a: #A & {value: {x: 0, y: 1}} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,3 +1,3 @@ + == a.cue + #A: value?: {x: number, y: number} | *{x: 0, y: 0} +-a: #A & {value: {y: 1}} ++a: #A & {value: {x: 0, y: 1}} +-- out/trim -- +== a.cue +#A: value?: {x: number, y: number} | *{x: 0, y: 0} +a: #A & {value: {y: 1}} diff --git a/tools/trim/testdata/33.txtar b/tools/trim/testdata/33.txtar new file mode 100644 index 000000000..05097e86c --- /dev/null +++ b/tools/trim/testdata/33.txtar @@ -0,0 +1,32 @@ +Do not remove conjuncts that are essential for selecting branches of a +disjunction. + +-- a.cue -- +g: #x & {a: 5} +#x: {a: 5, a: int} | {b: 5} | {c: >4} +h: #x & {b: 5} +i: #x & {c: 5} +-- out/trim-v3 -- +== a.cue +g: #x & {a: 5} +#x: {a: 5, a: int} | {b: 5} | {c: >4} +h: #x & {b: 5} +i: #x & {c: 5} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,5 +1,5 @@ + == a.cue +-g: #x & {} ++g: #x & {a: 5} + #x: {a: 5, a: int} | {b: 5} | {c: >4} +-h: #x & {} ++h: #x & {b: 5} + i: #x & {c: 5} +-- out/trim -- +== a.cue +g: #x & {} +#x: {a: 5, a: int} | {b: 5} | {c: >4} +h: #x & {} +i: #x & {c: 5} diff --git a/tools/trim/testdata/34.txtar b/tools/trim/testdata/34.txtar new file mode 100644 index 000000000..587e007f6 --- /dev/null +++ b/tools/trim/testdata/34.txtar @@ -0,0 +1,7 @@ +-- a.cue -- +x: a: string +x: {b: 5} | {b: 6} +-- out/trim -- +== a.cue +x: a: string +x: {b: 5} | {b: 6} diff --git a/tools/trim/testdata/35.txtar b/tools/trim/testdata/35.txtar new file mode 100644 index 000000000..3a5a33f72 --- /dev/null +++ b/tools/trim/testdata/35.txtar @@ -0,0 +1,53 @@ +issue 3140 +-- a.cue -- +#Foo: { + id: string + ... +} + +foo: [ID=_]: #Foo & { + id: ID +} + +foo: X1: { + id: "X1" + alist: [] +} +-- out/trim-v3 -- +== a.cue +#Foo: { + id: string + ... +} + +foo: [ID=_]: #Foo & { + id: ID +} + +foo: X1: { + alist: [] +} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -8,4 +8,6 @@ + id: ID + } + +-foo: X1: {} ++foo: X1: { ++ alist: [] ++} +-- out/trim -- +== a.cue +#Foo: { + id: string + ... +} + +foo: [ID=_]: #Foo & { + id: ID +} + +foo: X1: {} diff --git a/tools/trim/testdata/36.txtar b/tools/trim/testdata/36.txtar new file mode 100644 index 000000000..535e65fb2 --- /dev/null +++ b/tools/trim/testdata/36.txtar @@ -0,0 +1,59 @@ +issue 3009 +-- a.cue -- +X: { + a: 1 + b: true +} +#X: X + +A: #X & { + a: 1 + b: true +} + +B: X & { + a: 1 + b: true +} +-- out/trim-v3 -- +== a.cue +X: { + a: 1 + b: true +} +#X: X + +A: #X & _ + +B: X & _ +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -5,9 +5,6 @@ + } + #X: X + +-A: #X & {} +- +-B: X & { +- a: 1 +- b: true +-} ++A: #X & _ ++ ++B: X & _ +-- out/trim -- +== a.cue +X: { + a: 1 + b: true +} +#X: X + +A: #X & {} + +B: X & { + a: 1 + b: true +} diff --git a/tools/trim/testdata/37.txtar b/tools/trim/testdata/37.txtar new file mode 100644 index 000000000..421c00e62 --- /dev/null +++ b/tools/trim/testdata/37.txtar @@ -0,0 +1,70 @@ +issue 2357 +-- a.cue -- +things: [_]: { + resources: *{ + requests: { + memory: "100M" + cpu: "100m" + } + limits: requests + } | {...} +} + +things: vagrant: { + resources: { + requests: { + cpu: "1m" + } + limits: close({}) + } +} +-- out/trim-v3 -- +== a.cue +things: [_]: { + resources: *{ + requests: { + memory: "100M" + cpu: "100m" + } + limits: requests + } | {...} +} + +things: vagrant: { + resources: { + requests: { + cpu: "1m" + } + limits: close({}) + } +} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -14,5 +14,6 @@ + requests: { + cpu: "1m" + } ++ limits: close({}) + } + } +-- out/trim -- +== a.cue +things: [_]: { + resources: *{ + requests: { + memory: "100M" + cpu: "100m" + } + limits: requests + } | {...} +} + +things: vagrant: { + resources: { + requests: { + cpu: "1m" + } + } +} diff --git a/tools/trim/testdata/38.txtar b/tools/trim/testdata/38.txtar new file mode 100644 index 000000000..11182a725 --- /dev/null +++ b/tools/trim/testdata/38.txtar @@ -0,0 +1,61 @@ +issue 2356 +-- a.cue -- +#IPPool: { + cidr: string +} + +#cluster: { + podCIDR: string + IPPool: #IPPool & { + cidr: podCIDR + } +} + +clusters: [_]: #cluster + +clusters: vagrant: podCIDR: "abc" +-- out/trim-v3 -- +== a.cue +#IPPool: { + cidr: string +} + +#cluster: { + podCIDR: string + IPPool: #IPPool & { + cidr: podCIDR + } +} + +clusters: [_]: #cluster + +clusters: vagrant: podCIDR: "abc" +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -5,7 +5,9 @@ + + #cluster: { + podCIDR: string +- IPPool: #IPPool & {} ++ IPPool: #IPPool & { ++ cidr: podCIDR ++ } + } + + clusters: [_]: #cluster +-- out/trim -- +== a.cue +#IPPool: { + cidr: string +} + +#cluster: { + podCIDR: string + IPPool: #IPPool & {} +} + +clusters: [_]: #cluster + +clusters: vagrant: podCIDR: "abc" diff --git a/tools/trim/testdata/39.txtar b/tools/trim/testdata/39.txtar new file mode 100644 index 000000000..455629fc1 --- /dev/null +++ b/tools/trim/testdata/39.txtar @@ -0,0 +1,40 @@ +issue 2333 +-- a.cue -- +#D: inner: { + type: "A" | "B" + if type == "A" { + value: number | *0 + } +} + +a: {inner: {type: "A", value: 0}} & #D +-- out/trim-v3 -- +== a.cue +#D: inner: { + type: "A" | "B" + if type == "A" { + value: number | *0 + } +} + +a: {inner: {type: "A"}} & #D +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -6,4 +6,4 @@ + } + } + +-a: {inner: {type: "A", value: 0}} & #D ++a: {inner: {type: "A"}} & #D +-- out/trim -- +== a.cue +#D: inner: { + type: "A" | "B" + if type == "A" { + value: number | *0 + } +} + +a: {inner: {type: "A", value: 0}} & #D diff --git a/tools/trim/testdata/4.txtar b/tools/trim/testdata/4.txtar new file mode 100644 index 000000000..410055b84 --- /dev/null +++ b/tools/trim/testdata/4.txtar @@ -0,0 +1,8 @@ +We do not simplify explicit unification. +Compare and contrast with 3 and 11. + +-- a.cue -- +z: "hi" & string +-- out/trim -- +== a.cue +z: "hi" & string diff --git a/tools/trim/testdata/40.txtar b/tools/trim/testdata/40.txtar new file mode 100644 index 000000000..3db02b35f --- /dev/null +++ b/tools/trim/testdata/40.txtar @@ -0,0 +1,21 @@ +issue 2329 +-- a.cue -- +#D: list: [...{value: number | *1}] +d: #D & { list: [{value: 1}] } +-- out/trim-v3 -- +== a.cue +#D: list: [...{value: number | *1}] +d: #D & {list: [{}]} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,3 +1,3 @@ + == a.cue + #D: list: [...{value: number | *1}] +-d: #D & {} ++d: #D & {list: [{}]} +-- out/trim -- +== a.cue +#D: list: [...{value: number | *1}] +d: #D & {} diff --git a/tools/trim/testdata/41.txtar b/tools/trim/testdata/41.txtar new file mode 100644 index 000000000..2ac489cea --- /dev/null +++ b/tools/trim/testdata/41.txtar @@ -0,0 +1,90 @@ +issue 1547 + +Because of the embeddings, we can't remove the `base.helm.values` +struct. Consequently, we can't simplify to `base: helm: values: +cacert: _` + +-- a.cue -- +base: helm: values: cacert: "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" +base: #HelmRelease + +#HelmRelease: { + helm: { + #WithHelmUtils + values: {...} + } +} + +#HelmUtils: { + cacert?: string | *"/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" +} + +#WithHelmUtils: { + values: { + utils + ... + } + utils: #HelmUtils +} +-- out/trim-v3 -- +== a.cue +base: helm: values: cacert: "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" +base: #HelmRelease + +#HelmRelease: { + helm: { + #WithHelmUtils + values: {...} + } +} + +#HelmUtils: { + cacert?: string | *"/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" +} + +#WithHelmUtils: { + values: { + utils + ... + } + utils: #HelmUtils +} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,10 +1,11 @@ + == a.cue +-base: helm: {} ++base: helm: values: cacert: "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" + base: #HelmRelease + + #HelmRelease: { + helm: { + #WithHelmUtils ++ values: {...} + } + } + +-- out/trim -- +== a.cue +base: helm: {} +base: #HelmRelease + +#HelmRelease: { + helm: { + #WithHelmUtils + } +} + +#HelmUtils: { + cacert?: string | *"/var/run/secrets/kubernetes.io/serviceaccount/ca.crt" +} + +#WithHelmUtils: { + values: { + utils + ... + } + utils: #HelmUtils +} diff --git a/tools/trim/testdata/42.txtar b/tools/trim/testdata/42.txtar new file mode 100644 index 000000000..175db82a4 --- /dev/null +++ b/tools/trim/testdata/42.txtar @@ -0,0 +1,37 @@ +issue 1309 + +NB: because of #NamespacedResource: #ClusterResource, it seems like it +would be possible to remove line 4. But we can't because that would +make line 5 an error - the use of the X_Platform field. So I think +this is the best we can do. + +#skip-v2 +-- a.cue -- +#ClusterResource: X_Platform: string +#NamespacedResource: { + #ClusterResource + X_Platform: string + metadata: namespace: X_Platform +} + +Resources: [Platform=string]: { + #NamespacedResource + X_Platform: Platform +} + +Resources: myplatform: {} +-- out/trim-v3 -- +== a.cue +#ClusterResource: X_Platform: string +#NamespacedResource: { + #ClusterResource + X_Platform: string + metadata: namespace: X_Platform +} + +Resources: [Platform=string]: { + #NamespacedResource + X_Platform: Platform +} + +Resources: myplatform: {} diff --git a/tools/trim/testdata/43.txtar b/tools/trim/testdata/43.txtar new file mode 100644 index 000000000..c76503a2e --- /dev/null +++ b/tools/trim/testdata/43.txtar @@ -0,0 +1,31 @@ +issue 1298 + +See also 46, 52 and 56. + +-- a.cue -- +#DefA: A: "a" +#DefB: B: "b" + +First: { + #DefA | #DefB + B: "b" + C: "c" +} + +First: { + B: "b" +} +-- out/trim -- +== a.cue +#DefA: A: "a" +#DefB: B: "b" + +First: { + #DefA | #DefB + B: "b" + C: "c" +} + +First: { + B: "b" +} diff --git a/tools/trim/testdata/44.txtar b/tools/trim/testdata/44.txtar new file mode 100644 index 000000000..9b0481515 --- /dev/null +++ b/tools/trim/testdata/44.txtar @@ -0,0 +1,55 @@ +issue 1062 +-- a.cue -- +#def2: { + in2: string +} +#def1: { + in1: string + out: #def2 & { + in2: in1 + } +} +a: #def1 & { + in1: "test" +} +-- out/trim-v3 -- +== a.cue +#def2: { + in2: string +} +#def1: { + in1: string + out: #def2 & { + in2: in1 + } +} +a: #def1 & { + in1: "test" +} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -4,7 +4,9 @@ + } + #def1: { + in1: string +- out: #def2 & {} ++ out: #def2 & { ++ in2: in1 ++ } + } + a: #def1 & { + in1: "test" +-- out/trim -- +== a.cue +#def2: { + in2: string +} +#def1: { + in1: string + out: #def2 & {} +} +a: #def1 & { + in1: "test" +} diff --git a/tools/trim/testdata/45.txtar b/tools/trim/testdata/45.txtar new file mode 100644 index 000000000..90a9ed4d7 --- /dev/null +++ b/tools/trim/testdata/45.txtar @@ -0,0 +1,30 @@ +issue 346 +-- a.cue -- +payments: [{ + price: 110 + currency: "EUR" +}, { + price: 204 + currency: "EUR" +}, { + price: 35 + currency: "GBP" +}] + +payments: [... { + currency: *"EUR" | string +}] +-- out/trim -- +== a.cue +payments: [{ + price: 110 +}, { + price: 204 +}, { + price: 35 + currency: "GBP" +}] + +payments: [... { + currency: *"EUR" | string +}] diff --git a/tools/trim/testdata/46.txtar b/tools/trim/testdata/46.txtar new file mode 100644 index 000000000..c02cf48e9 --- /dev/null +++ b/tools/trim/testdata/46.txtar @@ -0,0 +1,35 @@ +Related to 43. The changes from 43 are the switch from an embedded +disjunction, and the fact that the disjunction is open. This means +that the disjunction is not simplified to a single branch, unlike 43. + +See also 52. + +-- a.cue -- +DefA: A: "a" +DefB: B: "b" + +First: { + B: "b" + C: "c" +} + +First: { + B: "b" +} + +First: DefA | DefB +-- out/trim -- +== a.cue +DefA: A: "a" +DefB: B: "b" + +First: { + B: "b" + C: "c" +} + +First: { + B: "b" +} + +First: DefA | DefB diff --git a/tools/trim/testdata/47.txtar b/tools/trim/testdata/47.txtar new file mode 100644 index 000000000..cb4daebb2 --- /dev/null +++ b/tools/trim/testdata/47.txtar @@ -0,0 +1,28 @@ +We can do simplifications due to comprehensions! The conjuncts from +lines 3 and 6 tie with those from 9, but the conjuncts from line 9 +must win, because within line 9 there are no redundancies. + +#skip-v2 +-- a.cue -- +s: { + x: { + port: 8080 + } + y: { + port: 8080 + } +} +#d: port: 8080 +for k, v in s { + s: "\(k)": #d +} +-- out/trim-v3 -- +== a.cue +s: { + x: _ + y: _ +} +#d: port: 8080 +for k, v in s { + s: "\(k)": #d +} diff --git a/tools/trim/testdata/48.txtar b/tools/trim/testdata/48.txtar new file mode 100644 index 000000000..35172ae8b --- /dev/null +++ b/tools/trim/testdata/48.txtar @@ -0,0 +1,29 @@ +Variant of 47 where the comprehension makes use of the value of the +key-value pair. + +#skip-v2 +-- a.cue -- +d: port: 8080 +s: { + x: { + port: 8080 + } + y: { + port: 8080 + } +} +for k, v in s { + s: (k): d + s: (k): portCopy: v.port +} +-- out/trim-v3 -- +== a.cue +d: port: 8080 +s: { + x: _ + y: _ +} +for k, v in s { + s: (k): d + s: (k): portCopy: v.port +} diff --git a/tools/trim/testdata/49.txtar b/tools/trim/testdata/49.txtar new file mode 100644 index 000000000..d209e2a66 --- /dev/null +++ b/tools/trim/testdata/49.txtar @@ -0,0 +1,77 @@ +-- a.cue -- +deployment: [ID=_]: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: ID + spec: { + template: { + metadata: labels: { + app: ID + component: #Component + } + // we always have one namesake container + spec: containers: [{name: ID}] + } + } +} + +#Component: string + +deployment: [ID=_]: _spec & { + _name: ID + spec: replicas: *1 | int +} + +_spec: { + _name: string + + metadata: name: _name + metadata: labels: component: #Component + spec: template: { + metadata: labels: { + app: _name + component: #Component + domain: "prod" + } + spec: containers: [{name: _name}] + } +} +-- out/trim -- +== a.cue +deployment: [ID=_]: { + apiVersion: "apps/v1" + kind: "Deployment" + metadata: name: ID + spec: { + template: { + metadata: labels: { + app: ID + component: #Component + } + // we always have one namesake container + spec: containers: [{name: ID}] + } + } +} + +#Component: string + +deployment: [ID=_]: _spec & { + _name: ID + spec: replicas: *1 | int +} + +_spec: { + _name: string + + metadata: name: _name + metadata: labels: component: #Component + spec: template: { + metadata: labels: { + app: _name + component: #Component + domain: "prod" + } + spec: containers: [{name: _name}] + } +} diff --git a/tools/trim/testdata/5.txtar b/tools/trim/testdata/5.txtar new file mode 100644 index 000000000..f24a8cb9a --- /dev/null +++ b/tools/trim/testdata/5.txtar @@ -0,0 +1,17 @@ +Compare and contrast with 3 and 30. + +The conjuncts in x and y are directly reachable (i.e. without going +via z) from the evaluation result. So they can't be removed. Because x +survives, we also keep all uses of x. So z: x cannot be removed. + +-- a.cue -- +x: string +y: "hi" +z: x +z: y +-- out/trim -- +== a.cue +x: string +y: "hi" +z: x +z: y diff --git a/tools/trim/testdata/50.txtar b/tools/trim/testdata/50.txtar new file mode 100644 index 000000000..95eddbb20 --- /dev/null +++ b/tools/trim/testdata/50.txtar @@ -0,0 +1,33 @@ +-- a.cue -- +service: [_]: { + metadata: {x: 3} + selector: metadata +} + +service: pastrychef: selector: {} +-- out/trim-v3 -- +== a.cue +service: [_]: { + metadata: {x: 3} + selector: metadata +} + +service: pastrychef: _ +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -4,4 +4,4 @@ + selector: metadata + } + +-service: pastrychef: selector: {} ++service: pastrychef: _ +-- out/trim -- +== a.cue +service: [_]: { + metadata: {x: 3} + selector: metadata +} + +service: pastrychef: selector: {} diff --git a/tools/trim/testdata/51.txtar b/tools/trim/testdata/51.txtar new file mode 100644 index 000000000..996efd38b --- /dev/null +++ b/tools/trim/testdata/51.txtar @@ -0,0 +1,24 @@ +Demonstration of dependencies of references. Within b.c we find the +reference to x; we resolve that to a vertex which contains conjuncts +from line 1 and and line 4. We spot that line 4 is in the same (or +ancestor) lexical scope as b.c, and so we ensure that if line 5 +survives, then line 4 must too, otherwise c: x will reference an +unknown field. + +See also 53, 54, and 55. + +-- a.cue -- +a: x: 5 +b: { + a + x: int + c: x +} +-- out/trim -- +== a.cue +a: x: 5 +b: { + a + x: int + c: x +} diff --git a/tools/trim/testdata/52.txtar b/tools/trim/testdata/52.txtar new file mode 100644 index 000000000..61c7b5543 --- /dev/null +++ b/tools/trim/testdata/52.txtar @@ -0,0 +1,45 @@ +Variant of 43 (and 46), where we bring the disjunction out so it's no +longer embedded (but here it's still closed, unlike 46). + +See also 56. + +-- a.cue -- +#DefA: A: "a" +#DefB: B: "b" + +First: B: "b" +First: B: "b" + +First: #DefA | #DefB +-- out/trim-v3 -- +== a.cue +#DefA: A: "a" +#DefB: B: "b" + +First: B: "b" +First: _ + +First: #DefA | #DefB +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -2,7 +2,7 @@ + #DefA: A: "a" + #DefB: B: "b" + +-First: {} +-First: {} ++First: B: "b" ++First: _ + + First: #DefA | #DefB +-- out/trim -- +== a.cue +#DefA: A: "a" +#DefB: B: "b" + +First: {} +First: {} + +First: #DefA | #DefB diff --git a/tools/trim/testdata/53.txtar b/tools/trim/testdata/53.txtar new file mode 100644 index 000000000..21c5260e5 --- /dev/null +++ b/tools/trim/testdata/53.txtar @@ -0,0 +1,19 @@ +Variant of 51, with a longer chain of references to follow. + +-- a.cue -- +d: x: 5 +a: d +b: { + a + x: int + c: x +} +-- out/trim -- +== a.cue +d: x: 5 +a: d +b: { + a + x: int + c: x +} diff --git a/tools/trim/testdata/54.txtar b/tools/trim/testdata/54.txtar new file mode 100644 index 000000000..9748b7942 --- /dev/null +++ b/tools/trim/testdata/54.txtar @@ -0,0 +1,22 @@ +Another variant of 51, moving the embedding out to an implicit +unification. + +See also 55. + +-- a.cue -- +d: x: 5 +a: d +b: a +b: { + x: int + c: x +} +-- out/trim -- +== a.cue +d: x: 5 +a: d +b: a +b: { + x: int + c: x +} diff --git a/tools/trim/testdata/55.txtar b/tools/trim/testdata/55.txtar new file mode 100644 index 000000000..10c3e3f7b --- /dev/null +++ b/tools/trim/testdata/55.txtar @@ -0,0 +1,20 @@ +Variant of 54, where now the reference chain specialises b.c, instead +of b.x. + +-- a.cue -- +d: c: 5 +a: d +b: a +b: { + x: int + c: x +} +-- out/trim -- +== a.cue +d: c: 5 +a: d +b: a +b: { + x: int + c: x +} diff --git a/tools/trim/testdata/56.txtar b/tools/trim/testdata/56.txtar new file mode 100644 index 000000000..1af884d90 --- /dev/null +++ b/tools/trim/testdata/56.txtar @@ -0,0 +1,17 @@ +Similar to 43 and 52, but with one level less depth. + +-- a.cue -- +#DefA: "a" +#DefB: "b" + +First: "b" + +First: #DefA | #DefB +-- out/trim -- +== a.cue +#DefA: "a" +#DefB: "b" + +First: "b" + +First: #DefA | #DefB diff --git a/tools/trim/testdata/57.txtar b/tools/trim/testdata/57.txtar new file mode 100644 index 000000000..786a75fb2 --- /dev/null +++ b/tools/trim/testdata/57.txtar @@ -0,0 +1,45 @@ +A variation on 25. + +This test justifies tracking dependencies within winning sets. + +In 25, line 4 is the only winner for y, and so it is required to +survive. Then, line 4 requires line 1, and line 1 requires line +6. Because line 6 is required, line 2 (with which line 6 is a tie), +can be removed. + +But here, we have an additional tie, this time for y, between line 4 +and line 7, and as such we have no "early" requirement that line 1 +(the comprehension) survives. So we have a tie for x between lines 2 +and 6, and a tie for y between lines 4 and 7. The tie for x gets +solved first. + +If we did not track dependencies *within* a winning set, then the +following would occur: +1. we would choose line 2 to win (based on file position) +2. line 2 requires line 1 +3. line 1 requires line 6. +So we would end up with both line 2 and line 6 surviving. + +But because we identify dependencies between winning nodes *within* +the same set, we find than line 2 transitively depends on line 6, and +as such, when choose a winner, we make sure we choose line 6. + +Having chosen the winner for x, we then chose the winner for y between +lines 4 and 7. This is much simpler - we have a tie of 1-1 for the +seenCounts, and so we just pick line 4 because 4 < 7. + +#skip-v2 +-- a.cue -- +if x == 5 { + x: 5 + x: int + y: "hi" +} +x: 5 +y: "hi" +-- out/trim-v3 -- +== a.cue +if x == 5 { + y: "hi" +} +x: 5 diff --git a/tools/trim/testdata/58.txtar b/tools/trim/testdata/58.txtar new file mode 100644 index 000000000..471dc966c --- /dev/null +++ b/tools/trim/testdata/58.txtar @@ -0,0 +1,27 @@ +Make sure we don't remove fields due to patterns, even when the +patterns are references. + +-- a.cue -- +x: [string]: y +y: z: 6 +x: a: z: int +-- out/trim-v3 -- +== a.cue +x: [string]: y +y: z: 6 +x: a: _ +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,4 +1,4 @@ + == a.cue + x: [string]: y + y: z: 6 +-x: a: {} ++x: a: _ +-- out/trim -- +== a.cue +x: [string]: y +y: z: 6 +x: a: {} diff --git a/tools/trim/testdata/59.txtar b/tools/trim/testdata/59.txtar new file mode 100644 index 000000000..4bc580e6b --- /dev/null +++ b/tools/trim/testdata/59.txtar @@ -0,0 +1,15 @@ +Demonstration that embeddings need to be treated specially. The +reference x does not appear in the vertex for z. Without special +treatment of embeddings, we would attempt to simplify z to `z: _`. + +-- a.cue -- +x: y: 5 +z: { + x +} +-- out/trim -- +== a.cue +x: y: 5 +z: { + x +} diff --git a/tools/trim/testdata/6.txtar b/tools/trim/testdata/6.txtar new file mode 100644 index 000000000..3fa54c0a4 --- /dev/null +++ b/tools/trim/testdata/6.txtar @@ -0,0 +1,20 @@ +1 should get simplified in the same way as this. + +-- a.cue -- +x: <10 +x: 7 +-- out/trim-v3 -- +== a.cue +x: 7 +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,3 +1,2 @@ + == a.cue +-x: <10 + x: 7 +-- out/trim -- +== a.cue +x: <10 +x: 7 diff --git a/tools/trim/testdata/60.txtar b/tools/trim/testdata/60.txtar new file mode 100644 index 000000000..f77b482f2 --- /dev/null +++ b/tools/trim/testdata/60.txtar @@ -0,0 +1,17 @@ +Similar to 10, but with non-redundant constraints in the pattern. + +-- a.cue -- +d: [ + {name: "jack", age: 5}, + {name: "gill", age: >4}, + {name: "hilbert", age: int}, +] +d: [...{name: string, age: int}] +-- out/trim -- +== a.cue +d: [ + {name: "jack", age: 5}, + {name: "gill", age: >4}, + {name: "hilbert"}, +] +d: [...{name: string, age: int}] diff --git a/tools/trim/testdata/7.txtar b/tools/trim/testdata/7.txtar new file mode 100644 index 000000000..e7b21b420 --- /dev/null +++ b/tools/trim/testdata/7.txtar @@ -0,0 +1,29 @@ +Simplification of implicit unification occurs as normal even within a +pattern. + +-- a.cue -- +a: [string]: { + x: string + x: "hi" +} +-- out/trim-v3 -- +== a.cue +a: [string]: { + x: "hi" +} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,5 +1,4 @@ + == a.cue + a: [string]: { +- x: string + x: "hi" + } +-- out/trim -- +== a.cue +a: [string]: { + x: string + x: "hi" +} diff --git a/tools/trim/testdata/8.txtar b/tools/trim/testdata/8.txtar new file mode 100644 index 000000000..954f77029 --- /dev/null +++ b/tools/trim/testdata/8.txtar @@ -0,0 +1,38 @@ +We simplify within a pattern - even for implicit unification of the +patterns, at least when CUE is able to spot they really are the same +thing. + +In b, post evaluation, the vertex has two patterns, whereas in a, +there is only 1 pattern. + +-- a.cue -- +a: [string]: x: string +a: [string]: x: "hi" + +b: [string]: x: string +b: [=~ ".*"]: x: "hi" +-- out/trim-v3 -- +== a.cue +a: [string]: _ +a: [string]: x: "hi" + +b: [string]: x: string +b: [=~".*"]: x: "hi" +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,5 +1,5 @@ + == a.cue +-a: [string]: x: string ++a: [string]: _ + a: [string]: x: "hi" + + b: [string]: x: string +-- out/trim -- +== a.cue +a: [string]: x: string +a: [string]: x: "hi" + +b: [string]: x: string +b: [=~".*"]: x: "hi" diff --git a/tools/trim/testdata/9.txtar b/tools/trim/testdata/9.txtar new file mode 100644 index 000000000..09b60685e --- /dev/null +++ b/tools/trim/testdata/9.txtar @@ -0,0 +1,49 @@ +This tests that we don't accidentally remove fields when conjuncts +added by a pattern are more specific: jack, gill etc can be +simplified, but must not be entirely removed. + +-- a.cue -- +a: 5 +s: { age: a } +d: { + jack: {age: 5}, + gill: {age: >4}, + hilbert: {age: int}, +} +d: [string]: s +-- out/trim-v3 -- +== a.cue +a: 5 +s: {age: a} +d: { + jack: _ + gill: _ + hilbert: _ +} +d: [string]: s +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -2,8 +2,8 @@ + a: 5 + s: {age: a} + d: { +- jack: {} +- gill: {} +- hilbert: {} ++ jack: _ ++ gill: _ ++ hilbert: _ + } + d: [string]: s +-- out/trim -- +== a.cue +a: 5 +s: {age: a} +d: { + jack: {} + gill: {} + hilbert: {} +} +d: [string]: s diff --git a/tools/trim/testdata/constraintroots.txtar b/tools/trim/testdata/constraintroots.txtar index cb0fb7ddc..b5edf4b99 100644 --- a/tools/trim/testdata/constraintroots.txtar +++ b/tools/trim/testdata/constraintroots.txtar @@ -12,16 +12,15 @@ keepPatternConstraintRoot: { deployment: [string]: spec: [] } -// TODO(additional): add once implemented -// keepAdditionalRoots: { -// if true { -// deployment: "name": {spec: [1]} -// } -// -// deployment: name: spec: [1] -// -// deployment: [string]: spec: [1] -// } +keepAdditionalRoots: { + if true { + deployment: "name": {spec: [1]} + } + + deployment: name: spec: [1] + + deployment: [string]: spec: [1] +} keepPatternConstraintRootSolo: { if true { @@ -54,6 +53,121 @@ keepPatternConstraintRootDef: { deployment: [string]: spec: [] } +-- out/trim-v3 -- +== in.cue +keepPatternConstraintRoot: { + if true { + deployment: "name": _ + } + + deployment: _ + + deployment: [string]: spec: [] +} + +keepAdditionalRoots: { + if true { + deployment: "name": _ + } + + deployment: _ + + deployment: [string]: spec: [1] +} + +keepPatternConstraintRootSolo: { + if true { + deployment: "name": _ + } + + deployment: [string]: spec: [1] +} + +keepPatternConstraintRootIndirect: { + if true { + deployment: "name": {spec: []} + } + + deployment: name: spec: [] + + deployment: indirect + + indirect: {{[string]: spec: []}} +} + +keepPatternConstraintRootDef: { + if true { + deployment: "name": _ + } + + #Deployment: spec: [] + + deployment: name: #Deployment + + deployment: [string]: spec: [] +} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,8 +1,10 @@ + == in.cue + keepPatternConstraintRoot: { + if true { +- deployment: "name": {spec: []} +- } ++ deployment: "name": _ ++ } ++ ++ deployment: _ + + deployment: [string]: spec: [] + } +@@ -9,8 +11,10 @@ + + keepAdditionalRoots: { + if true { +- deployment: "name": {spec: [1]} +- } ++ deployment: "name": _ ++ } ++ ++ deployment: _ + + deployment: [string]: spec: [1] + } +@@ -17,7 +21,7 @@ + + keepPatternConstraintRootSolo: { + if true { +- deployment: "name": {spec: [1]} ++ deployment: "name": _ + } + + deployment: [string]: spec: [1] +@@ -28,6 +32,8 @@ + deployment: "name": {spec: []} + } + ++ deployment: name: spec: [] ++ + deployment: indirect + + indirect: {{[string]: spec: []}} +@@ -35,10 +41,12 @@ + + keepPatternConstraintRootDef: { + if true { +- deployment: "name": {spec: []} ++ deployment: "name": _ + } + + #Deployment: spec: [] + ++ deployment: name: #Deployment ++ + deployment: [string]: spec: [] + } -- out/trim -- == in.cue keepPatternConstraintRoot: { @@ -64,16 +178,13 @@ keepPatternConstraintRoot: { deployment: [string]: spec: [] } -// TODO(additional): add once implemented -// keepAdditionalRoots: { -// if true { -// deployment: "name": {spec: [1]} -// } -// -// deployment: name: spec: [1] -// -// deployment: [string]: spec: [1] -// } +keepAdditionalRoots: { + if true { + deployment: "name": {spec: [1]} + } + + deployment: [string]: spec: [1] +} keepPatternConstraintRootSolo: { if true { diff --git a/tools/trim/testdata/defaults.txtar b/tools/trim/testdata/defaults.txtar index bb6236ae0..be59d26f0 100644 --- a/tools/trim/testdata/defaults.txtar +++ b/tools/trim/testdata/defaults.txtar @@ -65,6 +65,87 @@ dontEraseDefaultSelection: { verbs: *["a", "b"] | ["c"] } } +-- out/trim-v3 -- +== in.cue +domToSub: { + foo: [string]: a: *1 | int + foo: b: _ +} + +// Issue #759 +subToDom: { + #maybeString: {ip?: string} + something: ip: *"default" | string + something: #maybeString +} + +// references to definitions of a disjunction should be resolved and counted +// as dominator nodes. +resolveDefaults: { + #monitor: { + kind: "a" + } | { + kind: "b" + } + + monitor: #monitor + + monitor: kind: "a" +} + +issue781: { + #monitor_check: { + check_name: string + check_interval?: string + } + + #monitor_check: { + check_type: "nginx_config" + } | { + check_type: "docker_running" + vars: { + container_name: string + } + } + + monitor: { + checks: [...#monitor_check] + } + + monitor: { + checks: [{ + check_type: "nginx_config" + check_name: "nginx_config" + }] + } +} + +// Issue #801 +// Here the concrete value selects the default from a dominator, after which +// the dominator becomes an exact match. The exact match should not allow it +// to be erased, as the exact match is only there because subordinate value +// was first used to select the default. +dontEraseDefaultSelection: { + rule: _#Rule & { + verbs: ["c"] + } + _#Rule: { + verbs: *["a", "b"] | ["c"] + } +} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,7 +1,7 @@ + == in.cue + domToSub: { + foo: [string]: a: *1 | int +- foo: b: {} ++ foo: b: _ + } + + // Issue #759 -- out/trim -- == in.cue domToSub: { diff --git a/tools/trim/testdata/defaults_can_remove_non-defaults.txtar b/tools/trim/testdata/defaults_can_remove_non-defaults.txtar index 5ec4136a7..73df76684 100644 --- a/tools/trim/testdata/defaults_can_remove_non-defaults.txtar +++ b/tools/trim/testdata/defaults_can_remove_non-defaults.txtar @@ -2,6 +2,19 @@ -- in.cue -- foo: [string]: a: *1 | int foo: b: a: 1 +-- out/trim-v3 -- +== in.cue +foo: [string]: a: *1 | int +foo: b: _ +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,3 +1,3 @@ + == in.cue + foo: [string]: a: *1 | int +-foo: b: {} ++foo: b: _ -- out/trim -- == in.cue foo: [string]: a: *1 | int diff --git a/tools/trim/testdata/do_not_overmark_comprehension.txtar b/tools/trim/testdata/do_not_overmark_comprehension.txtar index bee9e3e7b..b200cddea 100644 --- a/tools/trim/testdata/do_not_overmark_comprehension.txtar +++ b/tools/trim/testdata/do_not_overmark_comprehension.txtar @@ -11,6 +11,33 @@ group: { comp: "\(k)": v } } +-- out/trim-v3 -- +== in.cue +foo: multipath: { + t: [string]: {x: 5} + + // Don't remove u! + t: u: _ +} + +group: { + for k, v in foo { + comp: "\(k)": v + } +} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -3,7 +3,7 @@ + t: [string]: {x: 5} + + // Don't remove u! +- t: u: {} ++ t: u: _ + } + + group: { -- out/trim -- == in.cue foo: multipath: { diff --git a/tools/trim/testdata/do_not_remove_field.txtar b/tools/trim/testdata/do_not_remove_field.txtar index 68d78e4e3..a39918331 100644 --- a/tools/trim/testdata/do_not_remove_field.txtar +++ b/tools/trim/testdata/do_not_remove_field.txtar @@ -1,6 +1,23 @@ +Because of the embedding, trimV3 does not simplify this. If line 1 was +rewritten without the embedding, then the simplification can +occur. See do_not_remove_field_2 for that. + -- in.cue -- {[_]: x: "hello"} a: x: "hello" +-- out/trim-v3 -- +== in.cue +{[_]: x: "hello"} +a: x: "hello" +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,3 +1,3 @@ + == in.cue + {[_]: x: "hello"} +-a: {} ++a: x: "hello" -- out/trim -- == in.cue {[_]: x: "hello"} diff --git a/tools/trim/testdata/do_not_remove_field_2.txtar b/tools/trim/testdata/do_not_remove_field_2.txtar new file mode 100644 index 000000000..c5f36a461 --- /dev/null +++ b/tools/trim/testdata/do_not_remove_field_2.txtar @@ -0,0 +1,21 @@ +A variant of do_not_remove_field, but with line 1 not embedded. +-- in.cue -- +[_]: x: "hello" +a: x: "hello" +-- out/trim-v3 -- +== in.cue +[_]: x: "hello" +a: _ +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -1,3 +1,3 @@ + == in.cue + [_]: x: "hello" +-a: {} ++a: _ +-- out/trim -- +== in.cue +[_]: x: "hello" +a: {} diff --git a/tools/trim/testdata/kube1.txtar b/tools/trim/testdata/kube1.txtar index 4885a5ed2..78665c697 100644 --- a/tools/trim/testdata/kube1.txtar +++ b/tools/trim/testdata/kube1.txtar @@ -26,6 +26,44 @@ service: a: { extra: 3 }] } +-- out/trim-v3 -- +== in.cue +service: [ID=string]: { + ports: [...{ + protocol: *"TCP" | "UDP" + extra: 3 + }] +} + +service: a: { + ports: [{ + name: "a" + key: "bar" + }] +} + +service: a: { + ports: [{}] +} + +service: a: { + ports: [{}] +} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -14,9 +14,7 @@ + } + + service: a: { +- ports: [{ +- key: "bar" +- }] ++ ports: [{}] + } + + service: a: { -- out/trim -- == in.cue service: [ID=string]: { diff --git a/tools/trim/testdata/list_removal_1.txtar b/tools/trim/testdata/list_removal_1.txtar index aa395a67a..e52175d6d 100644 --- a/tools/trim/testdata/list_removal_1.txtar +++ b/tools/trim/testdata/list_removal_1.txtar @@ -5,6 +5,25 @@ service: [string]: { service: a: { ports: [{a: 1}, {a: 1, extra: 3}, {}, { extra: 3 }] } +-- out/trim-v3 -- +== in.cue +service: [string]: { + ports: [{a: 1}, {a: 1}, ...{extra: 3}] +} +service: a: { + ports: [_, {extra: 3}, {}, {}] +} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -3,5 +3,5 @@ + ports: [{a: 1}, {a: 1}, ...{extra: 3}] + } + service: a: { +- ports: [{}, {extra: 3}, {}, {}] ++ ports: [_, {extra: 3}, {}, {}] + } -- out/trim -- == in.cue service: [string]: { diff --git a/tools/trim/testdata/list_removal_2.txtar b/tools/trim/testdata/list_removal_2.txtar index bd6d84e22..b076d8147 100644 --- a/tools/trim/testdata/list_removal_2.txtar +++ b/tools/trim/testdata/list_removal_2.txtar @@ -1,3 +1,11 @@ +This is correct. trimv2 resulted in + service: a: {} +but that creates an open list, rather than a closed list. +The result here, + service: a: ports: [_, _] +is correct. You can observe the difference by adding + x: service.a.ports & [_,_,_] +which *should* cause an error. -- in.cue -- service: [string]: { ports: [{a: 1}, {a: 1}, ...{ extra: 3 }] @@ -5,6 +13,26 @@ service: [string]: { service: a: { ports: [{a: 1}, {a: 1,}] } +-- out/trim-v3 -- +== in.cue +service: [string]: { + ports: [{a: 1}, {a: 1}, ...{extra: 3}] +} +service: a: { + ports: [_, _] +} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -2,4 +2,6 @@ + service: [string]: { + ports: [{a: 1}, {a: 1}, ...{extra: 3}] + } +-service: a: {} ++service: a: { ++ ports: [_, _] ++} -- out/trim -- == in.cue service: [string]: { diff --git a/tools/trim/testdata/optional.txtar b/tools/trim/testdata/optional.txtar index 2d91bf1f8..4bb5f5a64 100644 --- a/tools/trim/testdata/optional.txtar +++ b/tools/trim/testdata/optional.txtar @@ -20,6 +20,34 @@ a: [{ b: #B b: bb: c: 2 // c can be removed, bb not. #B: bb?: c: 2 +-- out/trim-v3 -- +== a.cue +package pkg + +a: [...#A] + +a: [{ + annotations: {} +}] + +#A: annotations?: [string]: string + +b: #B +b: bb: _ // c can be removed, bb not. +#B: bb?: c: 2 +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -9,6 +9,6 @@ + + #A: annotations?: [string]: string + +-b: #B +-b: bb: {} // c can be removed, bb not. ++b: #B ++b: bb: _ // c can be removed, bb not. + #B: bb?: c: 2 -- out/trim -- == a.cue package pkg diff --git a/tools/trim/testdata/remove_due_to_simplification.txtar b/tools/trim/testdata/remove_due_to_simplification.txtar index eb9ad6913..8ca198992 100644 --- a/tools/trim/testdata/remove_due_to_simplification.txtar +++ b/tools/trim/testdata/remove_due_to_simplification.txtar @@ -20,6 +20,42 @@ group: { comp: "\(k)": v } } +-- out/trim-v3 -- +== in.cue +foo: [string]: { + t: [string]: { + x: >=0 & <=5 + } +} + +foo: multipath: { + t: [string]: { + // Combined with the other constraints, we know the value must be 5 and + // thus the entry below can be eliminated. + x: >=5 & <=8 & int + } + + t: u: {x: 5} +} + +group: { + for k, v in foo { + comp: "\(k)": v + } +} +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -12,7 +12,7 @@ + x: >=5 & <=8 & int + } + +- t: u: {} ++ t: u: {x: 5} + } + + group: { -- out/trim -- == in.cue foo: [string]: { diff --git a/tools/trim/testdata/rmimport.txtar b/tools/trim/testdata/rmimport.txtar index d17d0dfca..067f0c90b 100644 --- a/tools/trim/testdata/rmimport.txtar +++ b/tools/trim/testdata/rmimport.txtar @@ -23,6 +23,29 @@ x: y: a.A -- cue.mod/module.cue -- module: "mod.test/blah" language: version: "v0.9.0" +-- out/trim-v3 -- +== b.cue +package b + +import ( +) + +#Def: { + y: 5 +} + +x: #Def +x: _ +-- diff/-out/trim-v3<==>+out/trim -- +diff old new +--- old ++++ new +@@ -9,4 +9,4 @@ + } + + x: #Def +-x: {} ++x: _ -- out/trim -- == b.cue package b diff --git a/tools/trim/testdata/shared.txtar b/tools/trim/testdata/shared.txtar index d286ff779..bb434e186 100644 --- a/tools/trim/testdata/shared.txtar +++ b/tools/trim/testdata/shared.txtar @@ -1,4 +1,6 @@ -// Removing fields from shared nodes not supported. +Removing fields from shared nodes not supported. + +issue 760 -- in.cue -- import "strings" @@ -13,19 +15,17 @@ a: { shared: name: "a" } -issue760: { - +b: { service: [ID=_]: { name: ID } service: foo: _shared - service: bar: _shared _shared: { name: "foo" // Do not remove! } } -issue760: { +c: { service: [ID=_]: { _service_name: *strings.TrimSuffix(ID, "-suffix") | string } @@ -50,19 +50,17 @@ a: { shared: name: "a" } -issue760: { - +b: { service: [ID=_]: { name: ID } service: foo: _shared - service: bar: _shared _shared: { name: "foo" // Do not remove! } } -issue760: { +c: { service: [ID=_]: { _service_name: *strings.TrimSuffix(ID, "-suffix") | string } diff --git a/tools/trim/trim.go b/tools/trim/trim.go index 05abb106f..f4146faab 100644 --- a/tools/trim/trim.go +++ b/tools/trim/trim.go @@ -19,6 +19,8 @@ import ( "cuelang.org/go/cue" "cuelang.org/go/cue/ast" + "cuelang.org/go/internal" + "cuelang.org/go/internal/core/runtime" ) // Config configures trim options. @@ -29,5 +31,10 @@ type Config struct { func Files(files []*ast.File, inst cue.InstanceOrValue, cfg *Config) error { val := inst.Value() - return filesV2(files, val, cfg) + version, _ := (*runtime.Runtime)(val.Context()).Settings() + if version == internal.EvalV3 { + return filesV3(files, val, cfg) + } else { + return filesV2(files, val, cfg) + } } diff --git a/tools/trim/trim_test.go b/tools/trim/trim_test.go index 51e18c445..983c41549 100644 --- a/tools/trim/trim_test.go +++ b/tools/trim/trim_test.go @@ -27,7 +27,7 @@ import ( ) var ( - matrix = cuetdtest.DefaultOnlyMatrix + matrix = cuetdtest.SmallMatrix ) const trace = false diff --git a/tools/trim/trimv3.go b/tools/trim/trimv3.go new file mode 100644 index 000000000..237bbfafa --- /dev/null +++ b/tools/trim/trimv3.go @@ -0,0 +1,1108 @@ +// Copyright 2025 CUE Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package trim + +// # Overview +// +// The goal of trim is to remove redundant code within the supplied +// CUE ASTs. +// +// This is achieved by analysis of both the ASTs and the result of +// evaluation: looking at conjuncts etc within vertices. For each +// vertex, we try to identify conjuncts which are, by subsumption, as +// specific as the vertex as a whole. There three possible outcomes: +// +// a) No conjuncts on their own are found to be as specific as the +// vertex. In this case, we keep all the conjuncts. This is +// conservative, and may lead to conjuncts being kept which don't need +// to be, because we don't attempt to detect subsumption between +// subsets of a vertex's conjuncts. It is however safe. +// +// b) Exactly one conjunct is found which is as specific as the +// vertex. We keep this conjunct. Note that we do not currently +// consider that there may be other conjuncts within this vertex which +// have to be kept for other reasons, and in conjunction are as +// specific as this vertex. So again, we may end up keeping more +// conjuncts than strictly necessary, but it is still safe. +// +// c) Several conjuncts are found which are individually as specific +// as the vertex. We save this set of "winning conjuncts" for later. +// +// As we progress, we record the number of times each conjunct is seen +// (conjunct identity is taken as the conjunct's source node). Once we +// have completed traversing the vertices, we may have several sets of +// "winning conjuncts" each of which needs a conjunct selected to +// keep. We order these sets individually by seen-count (descending), +// and collectively by the sum of seen-counts for each set (also +// descending). For each set in turn, if there is no conjunct that is +// already kept, we choose to keep the most widely seen conjunct. If +// there is still a tie, we order by source code position. +// +// Additionally, if a conjunct survives, then we make sure that all +// references to that conjunct also survive. This helps to prevent +// surprises for the user: a field `x` that constrains a field `y` +// will always do so, even if `y` is always found to be more +// specific. For example: +// +// x: >5 +// x: <10 +// y: 7 +// y: x +// +// Here, `y` will not be simplified to 7. By contrast, +// +// y: >5 +// y: <10 +// y: 7 +// +// will be simplified to `y: 7`. +// +// # Ignoring conjuncts +// +// When we inspect each vertex, there may be conjuncts that we must +// ignore for the purposes of finding conjuncts as specific as the +// vertex. The danger is that such conjuncts are found to be as +// specific as the whole vertex, thus causing the other conjuncts to +// be removed. But this can alter the semantics of the CUE code. For +// example, conjuncts that originate from within a disjunction branch +// must be ignored. Consider: +// +// d: 6 | string +// o: d & int +// +// The vertex for `o` will contain conjuncts for 6, and int. We would +// find the 6 is as specific as the vertex, so it is tempting to +// remove the `int`. But if we do, then the value of `o` changes +// because the string-branch of the disjunction can no longer be +// dismissed. Processing of disjunctions cannot be done on the AST, +// because disjunctions may contain references which we need to +// resolve, in order to know which conjuncts to ignore. For example: +// +// d: c | string +// o: d & int +// c: 6 +// +// Thus before we traverse the vertices to identify redundant +// conjuncts, we first traverse the vertices looking for disjunctions, +// and recording which conjuncts should be ignored. +// +// Another example is patterns: we must ignore conjuncts which are the +// roots of patterns. Consider: +// +// [string]: 5 +// o: int +// +// In the vertex for `o` we would find conjuncts for 5 and `int`. We +// must ignore the 5, otherwise we would find that it is as specific +// as `o`, which could cause the entire field declaration `o: int` to +// be removed, which then changes the value of the CUE program. +// +// As with disjunctions, an earlier pass over the vertices identifies +// patterns and marks them accordingly. +// +// Finally, embedded values require special treatment. Consider: +// +// x: y: 5 +// z: { +// x +// } +// +// Unfortunately, the evaluator doesn't track how different conjuncts +// arrive in a vertex: the vertex for `z` will not contain a conjunct +// which is a reference for `x`. All we will find in `z` is the arc +// for `y`. Because of this, we cannot discover that we must keep the +// embedded `x` -- it simply does not exist. So we take a rather blunt +// approach: an analysis of the AST will find where embeddings occur, +// which we record, and then when a vertex contains a struct which we +// know has an embedding, we always keep all the conjuncts in that +// vertex and its descendents. + +import ( + "fmt" + "io" + "os" + "slices" + "strings" + + "cuelang.org/go/cue" + "cuelang.org/go/cue/ast" + "cuelang.org/go/cue/ast/astutil" + "cuelang.org/go/cue/errors" + "cuelang.org/go/cue/token" + "cuelang.org/go/internal/core/adt" + "cuelang.org/go/internal/core/runtime" + "cuelang.org/go/internal/core/subsume" + "cuelang.org/go/internal/value" +) + +func filesV3(files []*ast.File, val cue.Value, cfg *Config) error { + dir := val.BuildInstance().Dir + dir = strings.TrimRight(dir, string(os.PathSeparator)) + + string(os.PathSeparator) + + if cfg.Trace && cfg.TraceWriter == nil { + cfg.TraceWriter = os.Stderr + } + + r, v := value.ToInternal(val) + ctx := adt.NewContext(r, v) + t := &trimmerV3{ + r: r, + ctx: ctx, + nodes: make(map[ast.Node]*nodeMeta), + trace: cfg.TraceWriter, + } + + t.logf("\nStarting trim in dir %q with files:", dir) + for i, file := range files { + t.logf(" %d: %s", i, file.Filename) + } + t.logf("\nFinding static dependencies") + t.findStaticDependencies(files) + t.logf("\nFinding patterns") + t.findPatterns(v) + t.logf("\nFinding disjunctions") + t.findDisjunctions(v) + t.logf("\nFinding redundances") + t.findRedundancies(v, false) + t.logf("\nSolve undecideds") + t.solveUndecideds() + + t.logf("\nTrimming source") + return t.trim(files, dir) +} + +type nodeMeta struct { + // The static parent - i.e. parent from the AST. + parent *nodeMeta + + src ast.Node + + // If true, then this node must not be removed, because it is not + // redundant in at least one place where it's used. + required bool + + // If true, then conjuncts of this node should be ignored for the + // purpose of testing for redundant conjuncts. + ignoreConjunct bool + + // If this is true then this node has one or more embedded values + // (statically) - i.e. EmbedDecl has been found within this node + // (and src will be either a File or a StructLit). + hasEmbedding bool + + // If x.requiredBy = {y,z} then it means x must be kept if one or + // more of {y,z} are kept. It is directional: if x must be kept for + // other reasons, then that says nothing about whether any of {y,z} + // must be kept. + requiredBy []*nodeMeta + + // The number of times conjuncts of this node have been found in + // the vertices. This is used for choosing winning conjuncts, and + // to ensure that we never remove a node which we have only seen in + // the AST, and not in result of evaluation. + seenCount int +} + +func (nm *nodeMeta) incSeenCount() { + nm.seenCount++ +} + +func (nm *nodeMeta) markRequired() { + nm.required = true +} + +func (nm *nodeMeta) addRequiredBy(e *nodeMeta) { + for _, f := range nm.requiredBy { + if f == e { + return + } + } + nm.requiredBy = append(nm.requiredBy, e) +} + +func (a *nodeMeta) isRequiredBy(b *nodeMeta) bool { + if a == b { + return true + } + return a._isRequiredBy(map[*nodeMeta]struct{}{a: {}}, b) +} + +// Need to cope with cycles, hence the seen/visited-set. +func (a *nodeMeta) _isRequiredBy(seen map[*nodeMeta]struct{}, b *nodeMeta) bool { + for _, e := range a.requiredBy { + if e == b { + return true + } + if _, found := seen[e]; found { + continue + } + seen[e] = struct{}{} + if e._isRequiredBy(seen, b) { + return true + } + } + return false +} + +// True iff this node is required, or any of the nodes that require +// this node are themselves required (transitively). +func (nm *nodeMeta) isRequired() bool { + if nm.required { + return true + } + if len(nm.requiredBy) == 0 { + return false + } + return nm._isRequired(map[*nodeMeta]struct{}{nm: {}}) +} + +func (nm *nodeMeta) _isRequired(seen map[*nodeMeta]struct{}) bool { + if nm.required { + return true + } + for _, e := range nm.requiredBy { + if _, found := seen[e]; found { + continue + } + seen[e] = struct{}{} + if e._isRequired(seen) { + nm.required = true + return true + } + } + return false +} + +// True iff this node or any of its parent nodes (static/AST parents), +// have been identified as containing embedded values. +func (nm *nodeMeta) isEmbedded() bool { + for ; nm != nil; nm = nm.parent { + if nm.hasEmbedding { + return true + } + } + return false +} + +// True iff a is an ancestor of b (in the static/AST parent-child +// sense). +func (a *nodeMeta) isAncestorOf(b *nodeMeta) bool { + if a == nil { + return false + } + for b != nil { + if b == a { + return true + } + b = b.parent + } + return false +} + +type trimmerV3 struct { + r *runtime.Runtime + ctx *adt.OpContext + nodes map[ast.Node]*nodeMeta + + undecided []nodeMetas + + // depth is purely for debugging trace indentation level. + depth int + trace io.Writer +} + +func (t *trimmerV3) logf(format string, args ...any) { + w := t.trace + if w == nil { + return + } + fmt.Fprintf(w, "%*s", t.depth*3, "") + fmt.Fprintf(w, format, args...) + fmt.Fprintln(w) +} + +func (t *trimmerV3) inc() { t.depth++ } +func (t *trimmerV3) dec() { t.depth-- } + +func (t *trimmerV3) getNodeMeta(n ast.Node) *nodeMeta { + if n == nil { + return nil + } + d, found := t.nodes[n] + if !found { + d = &nodeMeta{src: n} + t.nodes[n] = d + } + return d +} + +// Discovers findStaticDependencies between nodes by walking through the AST of +// the files. +// +// 1. Establishes that if a node survives then its parent must also +// survive. I.e. a parent is required by its children. +// +// 2. Marks the arguments for call expressions as required: no +// simplification can occur there. This is because we cannot discover +// the relationship between arguments to a function and the function's +// result, and so any simplification of the arguments may change the +// result of the function call in unknown ways. +// +// 3. The conjuncts in a adt.Vertex do not give any information as to +// whether they have arrived via embedding or not. But, in the AST, we +// do have that information. So find and record embedding information. +func (t *trimmerV3) findStaticDependencies(files []*ast.File) { + t.inc() + defer t.dec() + + var ancestors []*nodeMeta + callCount := 0 + for _, f := range files { + t.logf("%s", f.Filename) + ast.Walk(f, func(n ast.Node) bool { + t.inc() + t.logf("%p::%T %v", n, n, n.Pos()) + nm := t.getNodeMeta(n) + if field, ok := n.(*ast.Field); ok { + switch field.Constraint { + case token.NOT, token.OPTION: + t.logf(" ignoring %v", nm.src.Pos()) + nm.ignoreConjunct = true + nm.markRequired() + } + } + if l := len(ancestors); l > 0 { + parent := ancestors[l-1] + parent.addRequiredBy(nm) + nm.parent = parent + } + ancestors = append(ancestors, nm) + if _, ok := n.(*ast.CallExpr); ok { + callCount++ + } + if callCount > 0 { + // This is somewhat unfortunate, but for now, as soon as + // we're in the arguments for a function call, we prevent + // all simplifications. + nm.markRequired() + } + if _, ok := n.(*ast.EmbedDecl); ok && nm.parent != nil { + // The parent of an EmbedDecl is always either a File or a + // StructLit. + nm.parent.hasEmbedding = true + } + return true + }, func(n ast.Node) { + if _, ok := n.(*ast.CallExpr); ok { + callCount-- + } + ancestors = ancestors[:len(ancestors)-1] + t.dec() + }) + } +} + +// Discovers patterns by walking vertices and their arcs recursively. +// +// Conjuncts that originate from the pattern constraint must be +// ignored when searching for redundancies, otherwise they can be +// found to be more-or-equally-specific than the vertex in which +// they're found, and could lead to the entire field being +// removed. These conjuncts must also be kept because even if the +// pattern is not actually used, it may form part of the public API of +// the CUE, and so removing an unused pattern may alter the API. +// +// We only need to mark the conjuncts at the "top level" of the +// pattern constraint as required+ignore; we do not need to descend +// into the arcs of the pattern constraint. This is because the +// pattern only matches against a key, and not a path. So, even with: +// +// a: [string]: x: y: z: 5 +// +// we only need to mark the x as required+ignore, and not the y, z, or +// 5. This ensures we later ignore only this x when simplifying other +// conjuncts in a vertex who's label has matched this pattern. If we +// add: +// +// b: w: x: y: {} +// b: a +// +// This will get trimmed to: +// +// a: [string]: x: y: z: 5 +// b: w: _ +// b: a +// +// I.e. by ignoring the pattern`s "top level" conjuncts, we ensure we +// keep b: w, even though the pattern is equally specific to the +// vertex for b.w, and the explicit b: w (from line 2) is less +// specific. +func (t *trimmerV3) findPatterns(v *adt.Vertex) { + t.inc() + defer t.dec() + + worklist := []*adt.Vertex{v} + for len(worklist) != 0 { + v := worklist[0] + worklist = worklist[1:] + + t.logf("vertex %p; kind %v; value %p::%T", + v, v.Kind(), v.BaseValue, v.BaseValue) + t.inc() + + if patterns := v.PatternConstraints; patterns != nil { + for i, pair := range patterns.Pairs { + t.logf("pattern %d %p::%T", i, pair.Constraint, pair.Constraint) + t.inc() + pair.Constraint.VisitLeafConjuncts(func(c adt.Conjunct) bool { + field := c.Field() + elem := c.Elem() + expr := c.Expr() + t.logf("conjunct field: %p::%T, elem: %p::%T, expr: %p::%T", + field, field, elem, elem, expr, expr) + + if src := field.Source(); src != nil { + nm := t.getNodeMeta(src) + t.logf(" ignoring %v", nm.src.Pos()) + nm.ignoreConjunct = true + nm.markRequired() + } + + return true + }) + t.dec() + } + } + + t.dec() + + worklist = append(worklist, v.Arcs...) + if v, ok := v.BaseValue.(*adt.Vertex); ok { + worklist = append(worklist, v) + } + } +} + +// Discovers disjunctions by walking vertices and their arcs +// recursively. +// +// Disjunctions and their branches must be found before we attempt to +// simplify vertices. We must find disjunctions and mark all conjuncts +// within each branch of a disjunction, including all conjuncts that +// can be reached via resolution, as required+ignore. +// +// Failure to do this can lead to the removal of conjuncts in a vertex +// which were essential for discriminating between branches of a +// disjunction. +func (t *trimmerV3) findDisjunctions(v *adt.Vertex) { + t.inc() + defer t.dec() + + var branches []*adt.Vertex + seen := make(map[*adt.Vertex]struct{}) + worklist := []*adt.Vertex{v} + for len(worklist) != 0 { + v := worklist[0] + worklist = worklist[1:] + + if _, found := seen[v]; found { + continue + } + seen[v] = struct{}{} + + t.logf("vertex %p; kind %v; value %p::%T", + v, v.Kind(), v.BaseValue, v.BaseValue) + t.inc() + + v.VisitLeafConjuncts(func(c adt.Conjunct) bool { + switch disj := c.Elem().(type) { + case *adt.Disjunction: + t.logf("found disjunction") + for i, val := range disj.Values { + t.logf("branch %d", i) + branch := &adt.Vertex{ + Parent: v.Parent, + Label: v.Label, + } + c := adt.MakeConjunct(c.Env, val, c.CloseInfo) + branch.InsertConjunct(c) + branch.Finalize(t.ctx) + branches = append(branches, branch) + } + + case *adt.DisjunctionExpr: + t.logf("found disjunctionexpr") + for i, val := range disj.Values { + t.logf("branch %d", i) + branch := &adt.Vertex{ + Parent: v.Parent, + Label: v.Label, + } + c := adt.MakeConjunct(c.Env, val.Val, c.CloseInfo) + branch.InsertConjunct(c) + branch.Finalize(t.ctx) + branches = append(branches, branch) + } + } + return true + }) + + t.dec() + + worklist = append(worklist, v.Arcs...) + if v, ok := v.BaseValue.(*adt.Vertex); ok { + worklist = append(worklist, v) + } + } + + clear(seen) + worklist = branches + for len(worklist) != 0 { + v := worklist[0] + worklist = worklist[1:] + + if _, found := seen[v]; found { + continue + } + seen[v] = struct{}{} + + v.VisitLeafConjuncts(func(c adt.Conjunct) bool { + if src := c.Field().Source(); src != nil { + nm := t.getNodeMeta(src) + t.logf(" ignoring %v", nm.src.Pos()) + nm.ignoreConjunct = true + nm.markRequired() + } + t.resolveElemAll(c, func(resolver adt.Resolver, resolvedTo *adt.Vertex) { + worklist = append(worklist, resolvedTo.Arcs...) + }) + return true + }) + worklist = append(worklist, v.Arcs...) + } +} + +func (t *trimmerV3) keepAllChildren(n ast.Node) { + ast.Walk(n, func(n ast.Node) bool { + nm := t.getNodeMeta(n) + nm.markRequired() + return true + }, nil) +} + +// Once we have identified, and masked out, call expressions, +// embeddings, patterns, and disjunctions, we can finally work +// recursively through the vertices, testing their conjuncts to find +// redundant conjuncts. +func (t *trimmerV3) findRedundancies(v *adt.Vertex, keepAll bool) { + v = v.DerefDisjunct() + t.logf("vertex %p (parent %p); kind %v; value %p::%T", + v, v.Parent, v.Kind(), v.BaseValue, v.BaseValue) + t.inc() + defer t.dec() + + _, isDisjunct := v.BaseValue.(*adt.Disjunction) + for _, si := range v.Structs { + if src := si.StructLit.Src; src != nil { + t.logf("struct lit %p src: %p::%T %v", si.StructLit, src, src, src.Pos()) + nm := t.getNodeMeta(src) + nm.incSeenCount() + keepAll = keepAll || nm.isEmbedded() + if nm.hasEmbedding { + t.logf(" (has embedding root)") + } + if nm.isEmbedded() { + t.logf(" (isEmbedded)") + } else if keepAll { + t.logf(" (keepAll)") + } + + if !isDisjunct { + continue + } + v1 := &adt.Vertex{ + Parent: v.Parent, + Label: v.Label, + } + c := adt.MakeConjunct(si.Env, si.StructLit, si.CloseInfo) + v1.InsertConjunct(c) + v1.Finalize(t.ctx) + t.logf("exploring disj struct lit %p (src %v): start", si, src.Pos()) + t.findRedundancies(v1, keepAll) + t.logf("exploring disj struct lit %p (src %v): end", si, src.Pos()) + } + } + + if keepAll { + for _, si := range v.Structs { + if src := si.StructLit.Src; src != nil { + t.keepAllChildren(src) + } + } + } + + if patterns := v.PatternConstraints; patterns != nil { + for i, pair := range patterns.Pairs { + t.logf("pattern %d %p::%T", i, pair.Constraint, pair.Constraint) + t.findRedundancies(pair.Constraint, keepAll) + } + } + + var nodeMetas, winners, disjDefaultWinners []*nodeMeta + v.VisitLeafConjuncts(func(c adt.Conjunct) bool { + field := c.Field() + elem := c.Elem() + expr := c.Expr() + src := field.Source() + if src == nil { + t.logf("conjunct field: %p::%T, elem: %p::%T, expr: %p::%T, src nil", + field, field, elem, elem, expr, expr) + return true + } + + t.logf("conjunct field: %p::%T, elem: %p::%T, expr: %p::%T, src: %v", + field, field, elem, elem, expr, expr, src.Pos()) + + nm := t.getNodeMeta(src) + nm.incSeenCount() + + if exprSrc := expr.Source(); exprSrc != nil && len(v.Arcs) == 0 { + switch expr.(type) { + case *adt.StructLit, *adt.ListLit: + t.logf(" saving emptyness") + exprNm := t.getNodeMeta(exprSrc) + exprNm.addRequiredBy(nm) + } + } + + if nm.ignoreConjunct { + t.logf(" ignoring conjunct") + } else { + nodeMetas = append(nodeMetas, nm) + if t.equallySpecific(v, c) { + winners = append(winners, nm) + t.logf(" equally specific: %p::%T", field, field) + } else { + t.logf(" redundant here: %p::%T", field, field) + } + } + + if disj, ok := expr.(*adt.DisjunctionExpr); ok && disj.HasDefaults { + defaultCount := 0 + matchingDefaultCount := 0 + for _, branch := range disj.Values { + if !branch.Default { + continue + } + defaultCount++ + c := adt.MakeConjunct(c.Env, branch.Val, c.CloseInfo) + if t.equallySpecific(v, c) { + matchingDefaultCount++ + } + } + if defaultCount > 0 && defaultCount == matchingDefaultCount { + t.logf(" found %d matching defaults in disjunction", + matchingDefaultCount) + disjDefaultWinners = append(disjDefaultWinners, nm) + } + } + + if compr, ok := elem.(*adt.Comprehension); ok { + t.logf("comprehension found") + for _, clause := range compr.Clauses { + var conj adt.Conjunct + switch clause := clause.(type) { + case *adt.IfClause: + conj = adt.MakeConjunct(c.Env, clause.Condition, c.CloseInfo) + case *adt.ForClause: + conj = adt.MakeConjunct(c.Env, clause.Src, c.CloseInfo) + case *adt.LetClause: + conj = adt.MakeConjunct(c.Env, clause.Expr, c.CloseInfo) + } + t.linkResolvers(conj, true) + } + } + + t.linkResolvers(c, false) + return true + }) + + if keepAll { + t.logf("keeping all %d nodes", len(nodeMetas)) + for _, d := range nodeMetas { + t.logf(" %p::%T %v", d.src, d.src, d.src.Pos()) + d.markRequired() + } + + } else { + if len(disjDefaultWinners) != 0 { + // For all the conjuncts that were disjunctions and contained + // defaults, and *every* default is equally specific as the + // vertex as a whole, then we should be able to ignore all + // other winning conjuncts. + winners = disjDefaultWinners + } + switch len(winners) { + case 0: + t.logf("no winners; keeping all %d nodes", len(nodeMetas)) + for _, d := range nodeMetas { + t.logf(" %p::%T %v", d.src, d.src, d.src.Pos()) + d.markRequired() + } + + case 1: + t.logf("1 winner") + src := winners[0].src + t.logf(" %p::%T %v", src, src, src.Pos()) + winners[0].markRequired() + + default: + t.logf("%d winners found", len(winners)) + foundRequired := false + for _, d := range winners { + if d.isRequired() { + foundRequired = true + break + } + } + if !foundRequired { + t.logf("no winner already required") + t.undecided = append(t.undecided, winners) + } + } + } + + for i, a := range v.Arcs { + t.logf("arc %d %v", i, a.Label) + t.findRedundancies(a, keepAll) + } + + if v, ok := v.BaseValue.(*adt.Vertex); ok && v != nil { + t.logf("exploring base value: start") + t.findRedundancies(v, keepAll) + t.logf("exploring base value: end") + } +} + +// If somewhere within a conjunct, there's a *[adt.FieldReference], or +// other type of [adt.Resolver], then we need to find that, and ensure +// that: +// +// 1. if the resolver part of this conjunct survives, then the target +// of the resolver must survive too (i.e. we don't create dangling +// pointers). This bit is done for free, because if a vertex +// contains a conjunct for some reference `r`, then whatever `r` +// resolved to will also appear in this vertex's conjuncts. +// +// 2. if the target of the resolver survives, then we must +// survive. This enforces the basic rule that if a conjunct +// survives then all the references to that conjunct must also +// survive. +func (t *trimmerV3) linkResolvers(c adt.Conjunct, addInverse bool) { + var origNm *nodeMeta + if src := c.Field().Source(); src != nil { + origNm = t.getNodeMeta(src) + } + + t.resolveElemAll(c, func(resolver adt.Resolver, resolvedTo *adt.Vertex) { + resolvedTo.VisitLeafConjuncts(func(resolvedToC adt.Conjunct) bool { + src := resolvedToC.Source() + if src == nil { + return true + } + resolvedToNm := t.getNodeMeta(src) + resolverNm := t.getNodeMeta(resolver.Source()) + + // If the resolvedToC conjunct survives, then the resolver + // itself must survive too. + resolverNm.addRequiredBy(resolvedToNm) + t.logf(" (regular) %v reqBy %v", + resolverNm.src.Pos(), resolvedToNm.src.Pos()) + if addInverse { + t.logf(" (inverse) %v reqBy %v", + resolvedToNm.src.Pos(), resolverNm.src.Pos()) + resolvedToNm.addRequiredBy(resolverNm) + } + + // Don't break lexical scopes. Consider: + // + // c: { + // x: int + // y: x + // } + // c: x: 5 + // + // We must make sure that if `y: x` survives, then `x: + // int` survives (or at least the field does - it could + // be simplified to `x: _`) *even though* there is a + // more specific value for c.x in the final line. Thus + // the field which we have found by resolution, is + // required by the original element. + if origNm != nil && + resolvedToNm.parent.isAncestorOf(origNm) { + t.logf(" (extra) %v reqBy %v", + resolvedToNm.src.Pos(), origNm.src.Pos()) + resolvedToNm.addRequiredBy(origNm) + } + return true + }) + }) +} + +func (t *trimmerV3) resolveElemAll(c adt.Conjunct, f func(adt.Resolver, *adt.Vertex)) { + worklist := []adt.Elem{c.Elem()} + for len(worklist) != 0 { + elem := worklist[0] + worklist = worklist[1:] + + switch elemT := elem.(type) { + case *adt.UnaryExpr: + worklist = append(worklist, elemT.X) + case *adt.BinaryExpr: + worklist = append(worklist, elemT.X, elemT.Y) + case *adt.DisjunctionExpr: + for _, disjunct := range elemT.Values { + worklist = append(worklist, disjunct.Val) + } + case *adt.Disjunction: + for _, disjunct := range elemT.Values { + worklist = append(worklist, disjunct) + } + case *adt.Ellipsis: + worklist = append(worklist, elemT.Value) + case *adt.BoundExpr: + worklist = append(worklist, elemT.Expr) + case *adt.BoundValue: + worklist = append(worklist, elemT.Value) + case *adt.Interpolation: + for _, part := range elemT.Parts { + worklist = append(worklist, part) + } + case *adt.Conjunction: + for _, val := range elemT.Values { + worklist = append(worklist, val) + } + case *adt.CallExpr: + worklist = append(worklist, elemT.Fun) + for _, arg := range elemT.Args { + worklist = append(worklist, arg) + } + case *adt.Comprehension: + for _, y := range elemT.Clauses { + switch y := y.(type) { + case *adt.IfClause: + worklist = append(worklist, y.Condition) + case *adt.LetClause: + worklist = append(worklist, y.Expr) + case *adt.ForClause: + worklist = append(worklist, y.Src) + } + } + case *adt.LabelReference: + elem = &adt.ValueReference{UpCount: elemT.UpCount, Src: elemT.Src} + t.logf(" converting LabelReference to ValueReference") + } + + if r, ok := elem.(adt.Resolver); ok && elem.Source() != nil { + resolvedTo, bot := t.ctx.Resolve(c, r) + if bot != nil { + continue + } + t.logf(" resolved to %p", resolvedTo) + f(r, resolvedTo) + } + } +} + +// Are all the cs combined, (more or) equally as specific as v? +func (t *trimmerV3) equallySpecific(v *adt.Vertex, cs ...adt.Conjunct) bool { + t.inc() + // t.ctx.LogEval = 1 + conjVertex := &adt.Vertex{ + Parent: v.Parent, + Label: v.Label, + } + for _, c := range cs { + if r, ok := c.Elem().(adt.Resolver); ok { + v1, bot := t.ctx.Resolve(c, r) + if bot == nil { + v1.VisitLeafConjuncts(func(c adt.Conjunct) bool { + conjVertex.InsertConjunct(c) + return true + }) + continue + } + } + conjVertex.InsertConjunct(c) + } + conjVertex.Finalize(t.ctx) + err := subsume.Value(t.ctx, v, conjVertex) + if err != nil { + t.logf(" not equallySpecific") + if t.trace != nil && t.ctx.LogEval > 0 { + errors.Print(t.trace, err, nil) + } + } + // t.ctx.LogEval = 0 + t.dec() + return err == nil +} + +// NB this is not perfect. We do not attempt to track dependencies +// *between* different sets of "winning" nodes. +// +// We could have two sets, [a, b, c] and [c, d], and decide here to +// require a from the first set, and then c from the second set. This +// preserves more nodes than strictly necessary (preserving c on its +// own is sufficient to satisfy both sets). However, doing this +// perfectly is the “Hitting Set Problem”, and it is proven +// NP-complete. Thus for efficiency, we consider each set (more or +// less) in isolation. +func (t *trimmerV3) solveUndecideds() { + if len(t.undecided) == 0 { + return + } + undecided := t.undecided + for i, ds := range undecided { + ds.sort() + if ds.hasRequired() { + undecided[i] = nil + } + } + + slices.SortFunc(undecided, func(as, bs nodeMetas) int { + aSum, bSum := as.seenCountSum(), bs.seenCountSum() + if aSum != bSum { + return bSum - aSum + } + aLen, bLen := len(as), len(bs) + if aLen != bLen { + return bLen - aLen + } + for i, a := range as { + b := bs[i] + if posCmp := a.src.Pos().Compare(b.src.Pos()); posCmp != 0 { + return posCmp + } + } + return 0 + }) + + for _, nms := range undecided { + if len(nms) == 0 { + // once we get to length of 0, everything that follows must + // also be length of 0 + break + } + t.logf("choosing winner from %v", nms) + if nms.hasRequired() { + t.logf(" already contains required node") + continue + } + nms[0].markRequired() + } +} + +type nodeMetas []*nodeMeta + +// Sort a single set of nodeMetas. If a set contains x and y: +// +// - if x is required by y, then x will come first; +// - otherwise whichever node has a higher seenCount comes first; +// - otherwise sort x and y by their src position. +func (nms nodeMetas) sort() { + slices.SortFunc(nms, func(a, b *nodeMeta) int { + if a.isRequiredBy(b) { + return -1 + } + if b.isRequiredBy(a) { + return 1 + } + aSeen, bSeen := a.seenCount, b.seenCount + if aSeen != bSeen { + return bSeen - aSeen + } + return a.src.Pos().Compare(b.src.Pos()) + }) +} + +func (nms nodeMetas) seenCountSum() (sum int) { + for _, d := range nms { + sum += d.seenCount + } + return sum +} + +func (nms nodeMetas) hasRequired() bool { + for _, d := range nms { + if d.isRequired() { + return true + } + } + return false +} + +// After all the analysis is complete, trim finally modifies the AST, +// removing (or simplifying) nodes which have not been found to be +// required. +func (t *trimmerV3) trim(files []*ast.File, dir string) error { + t.inc() + defer t.dec() + + for _, f := range files { + if !strings.HasPrefix(f.Filename, dir) { + continue + } + t.logf("%s", f.Filename) + t.inc() + astutil.Apply(f, func(c astutil.Cursor) bool { + n := c.Node() + d := t.nodes[n] + + if !d.isRequired() && d.seenCount > 0 { + // The astutils cursor only supports deleting nodes if the + // node is a child of a structlit or a file. So in all + // other cases, we must replace the child with top. + var replacement ast.Node = ast.NewIdent("_") + if d.parent != nil { + switch parentN := d.parent.src.(type) { + case *ast.File, *ast.StructLit: + replacement = nil + case *ast.Comprehension: + if n == parentN.Value { + replacement = ast.NewStruct() + } + } + } + if replacement == nil { + t.logf("deleting node %p::%T %v", n, n, n.Pos()) + c.Delete() + } else { + t.logf("replacing node %p::%T with %T %v", + n, n, replacement, n.Pos()) + c.Replace(replacement) + } + } + + return true + }, nil) + if err := astutil.Sanitize(f); err != nil { + return err + } + t.dec() + } + return nil +}