From df327a68194de5247fc551bf337d8e263bb35c1b Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Sun, 23 Feb 2025 17:05:21 +0100 Subject: [PATCH] internal/core/adt: fix FromDef This CL does some magic with FromDef in order to make the tests pass again after the reduction in closeContext graph depth. In general, FromDef should be taken from the ConjunctGroup leafs. However, sometimes FromDef is passed down too aggressively. We therefore erase the incoming FromDef in adt.Unify. At the same time, when adt.Unify is called from the API and passes a Vertex, we still need to honor the ClosedRecursive flag. We therefore keep setting this flag for adt.Unify (in addConjuncts). Another issue is that in insertSkipConjuncts, we now need to clear the FromDef not only in ConjunctGroups, but any conjunct with a lower depth, because there is no longer a conjunct group corresponding to each increase in depth of the graph. Issue #2850 Signed-off-by: Marcel van Lohuizen Change-Id: I89d53ff14e374350c1fff317d2f1baf20268aaec Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1209184 Reviewed-by: Matthew Sackman TryBot-Result: CUEcueckoo --- cmd/cue/cmd/testdata/script/vet_matchn.txtar | 2 +- cue/testdata/cycle/builtins.txtar | 32 +- cue/testdata/cycle/issue502.txtar | 439 +++---------------- cue/testdata/cycle/issue990.txtar | 16 +- internal/core/adt/composite.go | 1 + internal/core/adt/conjunct.go | 3 + 6 files changed, 78 insertions(+), 415 deletions(-) diff --git a/cmd/cue/cmd/testdata/script/vet_matchn.txtar b/cmd/cue/cmd/testdata/script/vet_matchn.txtar index b6f7d53cc7b..ff386f39e2a 100644 --- a/cmd/cue/cmd/testdata/script/vet_matchn.txtar +++ b/cmd/cue/cmd/testdata/script/vet_matchn.txtar @@ -5,7 +5,7 @@ exec cue vet -c schema.cue data.json # With the new evaluator. env CUE_EXPERIMENT=evalv3=1 env CUE_DEBUG=openinline=0 -! exec cue vet -c schema.cue data.json #TODO Fix +exec cue vet -c schema.cue data.json -- data.json -- { diff --git a/cue/testdata/cycle/builtins.txtar b/cue/testdata/cycle/builtins.txtar index 8fd5e1dbc3e..8f6eede7ee1 100644 --- a/cue/testdata/cycle/builtins.txtar +++ b/cue/testdata/cycle/builtins.txtar @@ -236,14 +236,14 @@ listMatchN: ok: { -- todo/p1 -- issue3443.noCycle: fix hang -- out/evalalpha/stats -- -Leaks: 426 +Leaks: 427 Freed: 0 Reused: 0 -Allocs: 426 +Allocs: 427 Retain: 0 -Unifications: 367 -Conjuncts: 806 +Unifications: 368 +Conjuncts: 805 Disjuncts: 28 -- diff/-out/evalalpha/stats<==>+out/eval/stats -- diff old new @@ -255,17 +255,17 @@ diff old new -Reused: 402 -Allocs: 35 -Retain: 95 -+Leaks: 426 ++Leaks: 427 +Freed: 0 +Reused: 0 -+Allocs: 426 ++Allocs: 427 +Retain: 0 -Unifications: 409 -Conjuncts: 764 -Disjuncts: 501 -+Unifications: 367 -+Conjuncts: 806 ++Unifications: 368 ++Conjuncts: 805 +Disjuncts: 28 -- out/eval/stats -- Leaks: 23 @@ -293,8 +293,8 @@ issue3649.cycle.t1.data.a: invalid value {b:"foo"} (does not satisfy matchN): 0 ./cycle.cue:31:6 ./cycle.cue:28:11 ./cycle.cue:31:13 -issue3649.cycle.t1.data.a.a: field not allowed: - ./cycle.cue:31:17 +issue3649.cycle.t1.data.a.a: structural cycle: + ./cycle.cue:31:6 jsonCycle.t1.x.y: invalid value "{}" (does not satisfy encoding/json.Validate): error in call to encoding/json.Validate: structural cycle: ./jsoncycle.cue:4:8 ./jsoncycle.cue:5:8 @@ -443,8 +443,8 @@ Result: // ./cycle.cue:31:6 // ./cycle.cue:28:11 // ./cycle.cue:31:13 - // issue3649.cycle.t1.data.a.a: field not allowed: - // ./cycle.cue:31:17 + // issue3649.cycle.t1.data.a.a: structural cycle: + // ./cycle.cue:31:6 b: (string){ "foo" } } b: (string){ string } @@ -844,8 +844,8 @@ diff old new + ./cycle.cue:31:6 + ./cycle.cue:28:11 + ./cycle.cue:31:13 -+issue3649.cycle.t1.data.a.a: field not allowed: -+ ./cycle.cue:31:17 ++issue3649.cycle.t1.data.a.a: structural cycle: ++ ./cycle.cue:31:6 +jsonCycle.t1.x.y: invalid value "{}" (does not satisfy encoding/json.Validate): error in call to encoding/json.Validate: structural cycle: + ./jsoncycle.cue:4:8 + ./jsoncycle.cue:5:8 @@ -997,8 +997,8 @@ diff old new + // ./cycle.cue:31:6 + // ./cycle.cue:28:11 + // ./cycle.cue:31:13 -+ // issue3649.cycle.t1.data.a.a: field not allowed: -+ // ./cycle.cue:31:17 ++ // issue3649.cycle.t1.data.a.a: structural cycle: ++ // ./cycle.cue:31:6 + b: (string){ "foo" } + } + b: (string){ string } diff --git a/cue/testdata/cycle/issue502.txtar b/cue/testdata/cycle/issue502.txtar index 8b7227f8ab9..9b3804524e4 100644 --- a/cue/testdata/cycle/issue502.txtar +++ b/cue/testdata/cycle/issue502.txtar @@ -49,37 +49,20 @@ Unifications: 120 Conjuncts: 320 Disjuncts: 120 -- out/evalalpha -- -Errors: -a.mas.one.link.mas.two.link: field not allowed -#a.mas.one.link.mas.two.link.mas: field not allowed: - ./in.cue:6:10 -a.mas.one._link.mas.two.link.mas: field not allowed: - ./in.cue:6:10 -a.mas.one.link.mas.two._link.mas: field not allowed: - ./in.cue:6:10 -reduced.t1.a.x.y.z.x.y.z.x: field not allowed: - ./reduced.cue:4:11 - ./reduced.cue:2:6 - -Result: -(_|_){ - // [eval] +(struct){ #T: (#struct){ config: (_){ _ } body: (_){ _ } mas: (#struct){ } } - #a: (_|_){ - // [eval] + #a: (#struct){ config: (#struct){ a: (int){ int } } body: (int){ int } - mas: (_|_){ - // [eval] - one: (_|_){ - // [eval] + mas: (#struct){ + one: (#struct){ cfg: (#struct){ b: (int){ int } } @@ -112,16 +95,13 @@ Result: } } } - link: (_|_){ - // [eval] + link: (#struct){ config: (#struct){ b: (int){ int } } body: (int){ int } - mas: (_|_){ - // [eval] - two: (_|_){ - // [eval] + mas: (#struct){ + two: (#struct){ cfg: (#struct){ c: (int){ int } } @@ -133,15 +113,12 @@ Result: mas: (#struct){ } } - link: (_|_){ - // [eval] + link: (#struct){ config: (#struct){ c: (int){ int } } body: (int){ int } - mas: (_|_){ - // [eval] #a.mas.one.link.mas.two.link.mas: field not allowed: - // ./in.cue:6:10 + mas: (#struct){ } } } @@ -150,29 +127,23 @@ Result: } } } - a: (_|_){ - // [eval] + a: (#struct){ config: (#struct){ a: (int){ 34 } } body: (int){ 34 } - mas: (_|_){ - // [eval] - one: (_|_){ - // [eval] + mas: (#struct){ + one: (#struct){ cfg: (#struct){ b: (int){ 34 } } - _link: (_|_){ - // [eval] + _link: (#struct){ config: (#struct){ b: (int){ int } } body: (int){ int } - mas: (_|_){ - // [eval] - two: (_|_){ - // [eval] + mas: (#struct){ + two: (#struct){ cfg: (#struct){ c: (int){ int } } @@ -184,46 +155,42 @@ Result: mas: (#struct){ } } - link: (_|_){ - // [eval] + link: (#struct){ config: (#struct){ c: (int){ int } } body: (int){ int } - mas: (_|_){ - // [eval] a.mas.one._link.mas.two.link.mas: field not allowed: - // ./in.cue:6:10 + mas: (#struct){ } } } } } - link: (_|_){ - // [eval] + link: (#struct){ config: (#struct){ b: (int){ 34 } } body: (int){ 34 } - mas: (_|_){ - // [eval] - two: (_|_){ - // [eval] + mas: (#struct){ + two: (#struct){ cfg: (#struct){ c: (int){ 34 } } - _link: (_|_){ - // [eval] + _link: (#struct){ config: (#struct){ c: (int){ int } } body: (int){ int } - mas: (_|_){ - // [eval] a.mas.one.link.mas.two._link.mas: field not allowed: - // ./in.cue:6:10 + mas: (#struct){ } } - link: (_|_){ - // [eval] a.mas.one.link.mas.two.link: field not allowed + link: (#struct){ + config: (#struct){ + c: (int){ 34 } + } + body: (int){ 34 } + mas: (#struct){ + } } } } @@ -231,10 +198,8 @@ Result: } } } - reduced: (_|_){ - // [eval] - t1: (_|_){ - // [eval] + reduced: (struct){ + t1: (struct){ #T: (#struct){ x: (#struct){ y?: (#struct){ @@ -253,12 +218,9 @@ Result: } } } - a: (_|_){ - // [eval] - x: (_|_){ - // [eval] - y: (_|_){ - // [eval] + a: (#struct){ + x: (#struct){ + y: (#struct){ V: (#struct){ x: (struct){ y: (struct){ @@ -267,20 +229,20 @@ Result: } } } - z: (_|_){ - // [eval] - x: (_|_){ - // [eval] - y: (_|_){ - // [eval] + z: (#struct){ + x: (#struct){ + y: (#struct){ V: (#struct){ } - z: (_|_){ - // [eval] - x: (_|_){ - // [eval] reduced.t1.a.x.y.z.x.y.z.x: field not allowed: - // ./reduced.cue:4:11 - // ./reduced.cue:2:6 + z: (#struct){ + x: (#struct){ + y?: (_|_){ + // [structural cycle] + V: (_){ _ } + z: (_|_){ + // [structural cycle] reduced.t1.a.x.y.z.x.y.z.x.y.z: structural cycle + } + } } } } @@ -296,318 +258,15 @@ Result: diff old new --- old +++ new -@@ -1,4 +1,18 @@ --(struct){ -+Errors: -+a.mas.one.link.mas.two.link: field not allowed -+#a.mas.one.link.mas.two.link.mas: field not allowed: -+ ./in.cue:6:10 -+a.mas.one._link.mas.two.link.mas: field not allowed: -+ ./in.cue:6:10 -+a.mas.one.link.mas.two._link.mas: field not allowed: -+ ./in.cue:6:10 -+reduced.t1.a.x.y.z.x.y.z.x: field not allowed: -+ ./reduced.cue:4:11 -+ ./reduced.cue:2:6 -+ -+Result: -+(_|_){ -+ // [eval] - #T: (#struct){ - config: (_){ _ } - body: (_){ _ } -@@ -5,13 +19,16 @@ - mas: (#struct){ - } - } -- #a: (#struct){ -+ #a: (_|_){ -+ // [eval] - config: (#struct){ - a: (int){ int } - } - body: (int){ int } -- mas: (#struct){ -- one: (#struct){ -+ mas: (_|_){ -+ // [eval] -+ one: (_|_){ -+ // [eval] - cfg: (#struct){ - b: (int){ int } - } -@@ -44,111 +61,129 @@ - } - } - } -- link: (#struct){ -- config: (#struct){ -- b: (int){ int } -- } -- body: (int){ int } -- mas: (#struct){ -- two: (#struct){ -- cfg: (#struct){ -- c: (int){ int } -- } -- _link: (#struct){ -- config: (#struct){ -- c: (int){ int } -- } -- body: (int){ int } -- mas: (#struct){ -- } -- } -- link: (#struct){ -- config: (#struct){ -- c: (int){ int } -- } -- body: (int){ int } -- mas: (#struct){ -- } -- } -- } -- } -- } -- } -- } -- } -- a: (#struct){ -+ link: (_|_){ -+ // [eval] -+ config: (#struct){ -+ b: (int){ int } -+ } -+ body: (int){ int } -+ mas: (_|_){ -+ // [eval] -+ two: (_|_){ -+ // [eval] -+ cfg: (#struct){ -+ c: (int){ int } -+ } -+ _link: (#struct){ -+ config: (#struct){ -+ c: (int){ int } -+ } -+ body: (int){ int } -+ mas: (#struct){ -+ } -+ } -+ link: (_|_){ -+ // [eval] -+ config: (#struct){ -+ c: (int){ int } -+ } -+ body: (int){ int } -+ mas: (_|_){ -+ // [eval] #a.mas.one.link.mas.two.link.mas: field not allowed: -+ // ./in.cue:6:10 -+ } -+ } -+ } -+ } -+ } -+ } -+ } -+ } -+ a: (_|_){ -+ // [eval] - config: (#struct){ - a: (int){ 34 } - } - body: (int){ 34 } -- mas: (#struct){ -- one: (#struct){ -+ mas: (_|_){ -+ // [eval] -+ one: (_|_){ -+ // [eval] - cfg: (#struct){ - b: (int){ 34 } - } -- _link: (#struct){ -- config: (#struct){ -- b: (int){ int } -- } -- body: (int){ int } -- mas: (#struct){ -- two: (#struct){ -- cfg: (#struct){ -- c: (int){ int } -- } -- _link: (#struct){ -- config: (#struct){ -- c: (int){ int } -- } -- body: (int){ int } -- mas: (#struct){ -- } -- } -- link: (#struct){ -- config: (#struct){ -- c: (int){ int } -- } -- body: (int){ int } -- mas: (#struct){ -- } -- } -- } -- } -- } -- link: (#struct){ -+ _link: (_|_){ -+ // [eval] -+ config: (#struct){ -+ b: (int){ int } -+ } -+ body: (int){ int } -+ mas: (_|_){ -+ // [eval] -+ two: (_|_){ -+ // [eval] -+ cfg: (#struct){ -+ c: (int){ int } -+ } -+ _link: (#struct){ -+ config: (#struct){ -+ c: (int){ int } -+ } -+ body: (int){ int } -+ mas: (#struct){ -+ } -+ } -+ link: (_|_){ -+ // [eval] -+ config: (#struct){ -+ c: (int){ int } -+ } -+ body: (int){ int } -+ mas: (_|_){ -+ // [eval] a.mas.one._link.mas.two.link.mas: field not allowed: -+ // ./in.cue:6:10 -+ } -+ } -+ } -+ } -+ } -+ link: (_|_){ -+ // [eval] - config: (#struct){ - b: (int){ 34 } - } - body: (int){ 34 } -- mas: (#struct){ -- two: (#struct){ -+ mas: (_|_){ -+ // [eval] -+ two: (_|_){ -+ // [eval] - cfg: (#struct){ - c: (int){ 34 } - } -- _link: (#struct){ -- config: (#struct){ -- c: (int){ int } -- } -- body: (int){ int } -- mas: (#struct){ -- } -- } -- link: (#struct){ -- config: (#struct){ -- c: (int){ 34 } -- } -- body: (int){ 34 } -- mas: (#struct){ -- } -- } -- } -- } -- } -- } -- } -- } -- reduced: (struct){ -- t1: (struct){ -+ _link: (_|_){ -+ // [eval] -+ config: (#struct){ -+ c: (int){ int } -+ } -+ body: (int){ int } -+ mas: (_|_){ -+ // [eval] a.mas.one.link.mas.two._link.mas: field not allowed: -+ // ./in.cue:6:10 -+ } -+ } -+ link: (_|_){ -+ // [eval] a.mas.one.link.mas.two.link: field not allowed -+ } -+ } -+ } -+ } -+ } -+ } -+ } -+ reduced: (_|_){ -+ // [eval] -+ t1: (_|_){ -+ // [eval] - #T: (#struct){ +@@ -170,7 +170,7 @@ + a: (#struct){ x: (#struct){ - y?: (#struct){ -@@ -167,10 +202,13 @@ - } - } - } -- a: (#struct){ -- x: (#struct){ -- y: (#struct){ + y: (#struct){ - V: (struct){ -+ a: (_|_){ -+ // [eval] -+ x: (_|_){ -+ // [eval] -+ y: (_|_){ -+ // [eval] + V: (#struct){ x: (struct){ y: (struct){ V: (struct){ -@@ -178,20 +216,20 @@ - } - } - } -- z: (#struct){ -- x: (#struct){ -- y: (#struct){ -+ z: (_|_){ -+ // [eval] -+ x: (_|_){ -+ // [eval] -+ y: (_|_){ -+ // [eval] - V: (#struct){ - } -- z: (#struct){ -- x: (#struct){ -- y?: (_|_){ -- // [structural cycle] -- V: (_){ _ } -- z: (_|_){ -- // [structural cycle] reduced.t1.a.x.y.z.x.y.z.x.y.z: structural cycle -- } -- } -+ z: (_|_){ -+ // [eval] -+ x: (_|_){ -+ // [eval] reduced.t1.a.x.y.z.x.y.z.x: field not allowed: -+ // ./reduced.cue:4:11 -+ // ./reduced.cue:2:6 - } - } - } -- out/eval -- (struct){ #T: (#struct){ diff --git a/cue/testdata/cycle/issue990.txtar b/cue/testdata/cycle/issue990.txtar index 4cc1b30a4af..d91e61f48cf 100644 --- a/cue/testdata/cycle/issue990.txtar +++ b/cue/testdata/cycle/issue990.txtar @@ -80,15 +80,15 @@ out: #sub & {#p: _test.s1} -- diff/todo/p3 -- Reordering -- out/evalalpha/stats -- -Leaks: 1273 +Leaks: 1265 Freed: 0 Reused: 0 -Allocs: 1273 +Allocs: 1265 Retain: 0 Unifications: 243 -Conjuncts: 2934 -Disjuncts: 206 +Conjuncts: 2901 +Disjuncts: 204 -- out/evalalpha -- (struct){ #AC: (#struct){ @@ -328,18 +328,18 @@ diff old new -Reused: 3213 -Allocs: 25 -Retain: 26 -+Leaks: 1273 ++Leaks: 1265 +Freed: 0 +Reused: 0 -+Allocs: 1273 ++Allocs: 1265 +Retain: 0 -Unifications: 2588 -Conjuncts: 12056 -Disjuncts: 3258 +Unifications: 243 -+Conjuncts: 2934 -+Disjuncts: 206 ++Conjuncts: 2901 ++Disjuncts: 204 -- diff/-out/evalalpha<==>+out/eval -- diff old new --- old diff --git a/internal/core/adt/composite.go b/internal/core/adt/composite.go index f586c1558ab..ed5b7d73680 100644 --- a/internal/core/adt/composite.go +++ b/internal/core/adt/composite.go @@ -1022,6 +1022,7 @@ func Unify(c *OpContext, a, b Value) *Vertex { func addConjuncts(ctx *OpContext, dst *Vertex, src Value) { closeInfo := ctx.CloseInfo() + closeInfo.FromDef = false c := MakeConjunct(nil, src, closeInfo) if v, ok := src.(*Vertex); ok && v.ClosedRecursive { diff --git a/internal/core/adt/conjunct.go b/internal/core/adt/conjunct.go index 984f17860c2..a1e5ffffdd5 100644 --- a/internal/core/adt/conjunct.go +++ b/internal/core/adt/conjunct.go @@ -517,6 +517,9 @@ func (n *nodeContext) insertAndSkipConjuncts(c Conjunct, id CloseInfo, depth int } if c.CloseInfo.cc.depth <= depth { + // Now we do not clone the closeContext, set FromDef to false if + // the depth is smaller. + c.CloseInfo.FromDef = false if x, ok := c.Elem().(*ConjunctGroup); ok { for _, c := range *x { n.insertAndSkipConjuncts(c, id, depth)