From 8e02b92d727d261d7f18103b7a72173d37bf832e Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Thu, 8 Aug 2024 18:58:09 +0200 Subject: [PATCH] internal/core/adt: split conjunct trees based on prefix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this change, any conjunct referred to was inserted at the insertion point in full. Top-level conjuncts have a tree representing the origin of their definition and embedding. This caused the closedness rules to be repeated when they should not, resulting in spurious "field not allowed" errors. The new algorithm splits a conjunct into branches with roots that are shared with the insertion point removed. This ensures that references that are in scope of a definition do not get spuriously inserted. Fixes #3330 Fixes #3331 Signed-off-by: Marcel van Lohuizen Change-Id: I7a573f9576d3c46374647045d8ec226a1cd543b3 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1199272 Reviewed-by: Matthew Sackman TryBot-Result: CUEcueckoo Reviewed-by: Daniel Martí Unity-Result: CUE porcuepine --- .../benchmarks/inlinedisjunction.txtar | 4 +- cue/testdata/benchmarks/issue1684.txtar | 4 +- cue/testdata/benchmarks/listdedup.txtar | 4 +- cue/testdata/cycle/023_reentrance.txtar | 4 +- cue/testdata/cycle/chain.txtar | 4 +- cue/testdata/cycle/inline_non_recursive.txtar | 4 +- cue/testdata/cycle/issue990.txtar | 4 +- cue/testdata/eval/closedness.txtar | 321 ++++-------------- cue/testdata/eval/conjuncts.txtar | 4 +- cue/testdata/eval/issue3330.txtar | 126 ++----- cue/testdata/eval/v0.7.txtar | 4 +- internal/core/adt/composite.go | 6 + internal/core/adt/conjunct.go | 87 ++++- internal/core/adt/debug.go | 9 + internal/core/adt/fields.go | 9 + internal/core/adt/overlay.go | 3 + 16 files changed, 211 insertions(+), 386 deletions(-) diff --git a/cue/testdata/benchmarks/inlinedisjunction.txtar b/cue/testdata/benchmarks/inlinedisjunction.txtar index 84562662450..f975a3d5147 100644 --- a/cue/testdata/benchmarks/inlinedisjunction.txtar +++ b/cue/testdata/benchmarks/inlinedisjunction.txtar @@ -21,7 +21,7 @@ Allocs: 147 Retain: 0 Unifications: 17 -Conjuncts: 199 +Conjuncts: 198 Disjuncts: 76 -- out/eval -- (struct){ @@ -60,7 +60,7 @@ diff old new -Conjuncts: 13409 -Disjuncts: 4674 +Unifications: 17 -+Conjuncts: 199 ++Conjuncts: 198 +Disjuncts: 76 -- out/eval/stats -- Leaks: 0 diff --git a/cue/testdata/benchmarks/issue1684.txtar b/cue/testdata/benchmarks/issue1684.txtar index 0dfe35980b0..e19be93864e 100644 --- a/cue/testdata/benchmarks/issue1684.txtar +++ b/cue/testdata/benchmarks/issue1684.txtar @@ -69,7 +69,7 @@ Allocs: 2533 Retain: 0 Unifications: 707 -Conjuncts: 5878 +Conjuncts: 5373 Disjuncts: 1102 -- out/evalalpha -- (struct){ @@ -159,7 +159,7 @@ diff old new -Conjuncts: 2480117 -Disjuncts: 1064333 +Unifications: 707 -+Conjuncts: 5878 ++Conjuncts: 5373 +Disjuncts: 1102 -- diff/-out/evalalpha<==>+out/eval -- diff old new diff --git a/cue/testdata/benchmarks/listdedup.txtar b/cue/testdata/benchmarks/listdedup.txtar index 04666e17a55..c412cdfeef1 100644 --- a/cue/testdata/benchmarks/listdedup.txtar +++ b/cue/testdata/benchmarks/listdedup.txtar @@ -40,7 +40,7 @@ Allocs: 163 Retain: 0 Unifications: 53 -Conjuncts: 431 +Conjuncts: 407 Disjuncts: 40 -- out/eval -- (struct){ @@ -226,7 +226,7 @@ diff old new -Conjuncts: 100730 -Disjuncts: 24097 +Unifications: 53 -+Conjuncts: 431 ++Conjuncts: 407 +Disjuncts: 40 -- out/eval/stats -- Leaks: 0 diff --git a/cue/testdata/cycle/023_reentrance.txtar b/cue/testdata/cycle/023_reentrance.txtar index 650d71f678b..8390fe29467 100644 --- a/cue/testdata/cycle/023_reentrance.txtar +++ b/cue/testdata/cycle/023_reentrance.txtar @@ -70,7 +70,7 @@ Allocs: 70 Retain: 0 Unifications: 62 -Conjuncts: 227 +Conjuncts: 225 Disjuncts: 0 -- out/evalalpha -- Errors: @@ -133,7 +133,7 @@ diff old new -Conjuncts: 464 -Disjuncts: 268 +Unifications: 62 -+Conjuncts: 227 ++Conjuncts: 225 +Disjuncts: 0 -- diff/-out/evalalpha<==>+out/eval -- diff old new diff --git a/cue/testdata/cycle/chain.txtar b/cue/testdata/cycle/chain.txtar index e617dc376d6..401cd8dbdde 100644 --- a/cue/testdata/cycle/chain.txtar +++ b/cue/testdata/cycle/chain.txtar @@ -215,7 +215,7 @@ Allocs: 19986 Retain: 0 Unifications: 7496 -Conjuncts: 107561 +Conjuncts: 107414 Disjuncts: 13651 -- out/evalalpha -- Errors: @@ -735,7 +735,7 @@ diff old new -Conjuncts: 3175 -Disjuncts: 1974 +Unifications: 7496 -+Conjuncts: 107561 ++Conjuncts: 107414 +Disjuncts: 13651 -- diff/-out/evalalpha<==>+out/eval -- diff old new diff --git a/cue/testdata/cycle/inline_non_recursive.txtar b/cue/testdata/cycle/inline_non_recursive.txtar index ac432da19d7..410805c0de1 100644 --- a/cue/testdata/cycle/inline_non_recursive.txtar +++ b/cue/testdata/cycle/inline_non_recursive.txtar @@ -67,7 +67,7 @@ Allocs: 209 Retain: 0 Unifications: 194 -Conjuncts: 1097 +Conjuncts: 1034 Disjuncts: 0 -- out/evalalpha -- Errors: @@ -203,7 +203,7 @@ diff old new -Conjuncts: 2709 -Disjuncts: 1414 +Unifications: 194 -+Conjuncts: 1097 ++Conjuncts: 1034 +Disjuncts: 0 -- diff/-out/evalalpha<==>+out/eval -- diff old new diff --git a/cue/testdata/cycle/issue990.txtar b/cue/testdata/cycle/issue990.txtar index 3d2953be119..0e33ae0f971 100644 --- a/cue/testdata/cycle/issue990.txtar +++ b/cue/testdata/cycle/issue990.txtar @@ -626,7 +626,7 @@ Allocs: 2198 Retain: 0 Unifications: 219 -Conjuncts: 8714 +Conjuncts: 6776 Disjuncts: 462 -- out/evalalpha -- Errors: @@ -816,7 +816,7 @@ diff old new -Conjuncts: 12056 -Disjuncts: 3258 +Unifications: 219 -+Conjuncts: 8714 ++Conjuncts: 6776 +Disjuncts: 462 -- diff/-out/evalalpha<==>+out/eval -- diff old new diff --git a/cue/testdata/eval/closedness.txtar b/cue/testdata/eval/closedness.txtar index 85f27ea252f..68892a228c0 100644 --- a/cue/testdata/eval/closedness.txtar +++ b/cue/testdata/eval/closedness.txtar @@ -766,9 +766,6 @@ Result: } -- out/evalalpha -- Errors: -issue3330.let.ok.0.0.field: conflicting values null and {} (mismatched types null and struct): - ./reroot.cue:6:16 - ./reroot.cue:8:11 nested.err1.x.b.f: field not allowed: ./reroot.cue:114:6 nested.err2.x.b.g: field not allowed: @@ -783,31 +780,14 @@ a.q.e: field not allowed: issue852.a.Foo: field not allowed: ./in.cue:23:16 ./in.cue:28:5 -issue3330.let.ok.0.0.field.n: field not allowed: - ./reroot.cue:9:11 - ./reroot.cue:9:21 -issue3330.matthew.ok1.out.field.n: field not allowed: - ./reroot.cue:16:22 - ./reroot.cue:16:13 -issue3331.original.ok.0.0.c.d: field not allowed: - ./reroot.cue:36:7 - ./reroot.cue:37:5 nested.err1.x.#V.c.d: field not allowed: ./reroot.cue:122:8 ./reroot.cue:118:5 nested.err1.x.#V.c.f: field not allowed: ./reroot.cue:122:8 ./reroot.cue:114:6 -nested.err1.x.#V.c.g: field not allowed: - ./reroot.cue:122:8 nested.err1.x.b.g: field not allowed: ./reroot.cue:122:8 -nested.err2.x.c.d: field not allowed: - ./reroot.cue:129:6 - ./reroot.cue:133:4 -nested.err2.x.c.g: field not allowed: - ./reroot.cue:129:6 - ./reroot.cue:137:8 inline.err1.age1: field not allowed: ./reroot.cue:143:8 ./reroot.cue:145:3 @@ -913,12 +893,9 @@ Result: key: (string){ "foo" } } } - issue3330: (_|_){ - // [eval] - let: (_|_){ - // [eval] - ok: (_|_){ - // [eval] + issue3330: (struct){ + let: (struct){ + ok: (struct){ #struct: (#struct){ let empty#1 = (#struct){ } @@ -926,20 +903,19 @@ Result: n: (int){ 3 } } } - out: (_|_){ - // [eval] issue3330.let.ok.0.0.field: conflicting values null and {} (mismatched types null and struct): - // ./reroot.cue:6:16 - // ./reroot.cue:8:11 - // issue3330.let.ok.0.0.field.n: field not allowed: - // ./reroot.cue:9:11 - // ./reroot.cue:9:21 + out: (#list){ + 0: (#struct){ + let empty#1 = (#struct){ + } + field: (#struct){ + n: (int){ 3 } + } + } } } } - matthew: (_|_){ - // [eval] - ok1: (_|_){ - // [eval] + matthew: (struct){ + ok1: (struct){ #struct: (#struct){ field: (#struct){ n: (int){ 3 } @@ -947,15 +923,9 @@ Result: g: (#struct){ } } - out: (_|_){ - // [eval] - field: (_|_){ - // [eval] - n: (_|_){ - // [eval] issue3330.matthew.ok1.out.field.n: field not allowed: - // ./reroot.cue:16:22 - // ./reroot.cue:16:13 - } + out: (#struct){ + field: (#struct){ + n: (int){ 3 } } g: (#struct){ } @@ -986,14 +956,9 @@ Result: } } } - issue3331: (_|_){ - // [eval] - original: (_|_){ - // [eval] - ok: (_|_){ - // [eval] issue3331.original.ok.0.0.c.d: field not allowed: - // ./reroot.cue:36:7 - // ./reroot.cue:37:5 + issue3331: (struct){ + original: (struct){ + ok: (#list){ #A: (#struct){ let b#2 = (#struct){ } @@ -1001,6 +966,13 @@ Result: d: (int){ 1 } } } + 0: (#struct){ + let b#2 = (#struct){ + } + c: (#struct){ + d: (int){ 1 } + } + } } } variant1: (struct){ @@ -1167,10 +1139,7 @@ Result: // ./reroot.cue:122:8 // ./reroot.cue:114:6 } - g: (_|_){ - // [eval] nested.err1.x.#V.c.g: field not allowed: - // ./reroot.cue:122:8 - } + g: (int){ 1 } } } #V: (_|_){ @@ -1187,10 +1156,7 @@ Result: // ./reroot.cue:122:8 // ./reroot.cue:114:6 } - g: (_|_){ - // [eval] nested.err1.x.#V.c.g: field not allowed: - // ./reroot.cue:122:8 - } + g: (int){ 1 } } } } @@ -1233,18 +1199,9 @@ Result: // ./reroot.cue:137:8 } } - c: (_|_){ - // [eval] - d: (_|_){ - // [eval] nested.err2.x.c.d: field not allowed: - // ./reroot.cue:129:6 - // ./reroot.cue:133:4 - } - g: (_|_){ - // [eval] nested.err2.x.c.g: field not allowed: - // ./reroot.cue:129:6 - // ./reroot.cue:137:8 - } + c: (#struct){ + d: (int){ 1 } + g: (int){ 1 } } } } @@ -1282,11 +1239,8 @@ Result: diff old new --- old +++ new -@@ -1,72 +1,60 @@ +@@ -1,72 +1,40 @@ Errors: -+issue3330.let.ok.0.0.field: conflicting values null and {} (mismatched types null and struct): -+ ./reroot.cue:6:16 -+ ./reroot.cue:8:11 +nested.err1.x.b.f: field not allowed: + ./reroot.cue:114:6 +nested.err2.x.b.g: field not allowed: @@ -1314,15 +1268,6 @@ diff old new - ./in.cue:26:5 + ./in.cue:23:16 ./in.cue:28:5 -+issue3330.let.ok.0.0.field.n: field not allowed: -+ ./reroot.cue:9:11 -+ ./reroot.cue:9:21 -+issue3330.matthew.ok1.out.field.n: field not allowed: -+ ./reroot.cue:16:22 -+ ./reroot.cue:16:13 -+issue3331.original.ok.0.0.c.d: field not allowed: -+ ./reroot.cue:36:7 -+ ./reroot.cue:37:5 nested.err1.x.#V.c.d: field not allowed: - ./reroot.cue:112:5 - ./reroot.cue:114:6 @@ -1346,8 +1291,6 @@ diff old new - ./reroot.cue:123:6 + ./reroot.cue:122:8 + ./reroot.cue:114:6 -+nested.err1.x.#V.c.g: field not allowed: -+ ./reroot.cue:122:8 nested.err1.x.b.g: field not allowed: - ./reroot.cue:112:5 - ./reroot.cue:114:6 @@ -1374,12 +1317,6 @@ diff old new - ./reroot.cue:136:5 - ./reroot.cue:137:8 + ./reroot.cue:122:8 -+nested.err2.x.c.d: field not allowed: -+ ./reroot.cue:129:6 -+ ./reroot.cue:133:4 -+nested.err2.x.c.g: field not allowed: -+ ./reroot.cue:129:6 -+ ./reroot.cue:137:8 +inline.err1.age1: field not allowed: + ./reroot.cue:143:8 + ./reroot.cue:145:3 @@ -1399,7 +1336,7 @@ diff old new #Items: (#struct){ } #Base: (#struct){ -@@ -76,31 +64,36 @@ +@@ -76,31 +44,36 @@ } } #Extended: (#struct){ @@ -1457,7 +1394,7 @@ diff old new } } } -@@ -110,8 +103,8 @@ +@@ -110,8 +83,8 @@ #A: (#struct){ b: (int){ int } q: (#struct){ @@ -1467,7 +1404,7 @@ diff old new } } a: (_|_){ -@@ -120,21 +113,17 @@ +@@ -120,21 +93,17 @@ q: (_|_){ // [eval] c: (int){ 2 } @@ -1491,7 +1428,7 @@ diff old new // ./in.cue:28:5 #A: (#struct){ } -@@ -142,8 +131,7 @@ +@@ -142,8 +111,7 @@ // [eval] Foo: (_|_){ // [eval] issue852.a.Foo: field not allowed: @@ -1501,90 +1438,17 @@ diff old new // ./in.cue:28:5 } } -@@ -154,13 +142,16 @@ +@@ -154,8 +122,8 @@ foo: (int){ int } } d: (#struct){ - key: (string){ "foo" } foo: (int){ 3 } -- } -- } -- issue3330: (struct){ -- let: (struct){ -- ok: (struct){ + key: (string){ "foo" } -+ } -+ } -+ issue3330: (_|_){ -+ // [eval] -+ let: (_|_){ -+ // [eval] -+ ok: (_|_){ -+ // [eval] - #struct: (#struct){ - let empty#1 = (#struct){ - } -@@ -168,29 +159,36 @@ - n: (int){ 3 } - } - } -- out: (#list){ -- 0: (#struct){ -- let empty#1 = (#struct){ -- } -- field: (#struct){ -- n: (int){ 3 } -- } -- } -- } -- } -- } -- matthew: (struct){ -- ok1: (struct){ -- #struct: (#struct){ -- field: (#struct){ -- n: (int){ 3 } -- } -- g: (#struct){ -- } -- } -- out: (#struct){ -- field: (#struct){ -- n: (int){ 3 } -+ out: (_|_){ -+ // [eval] issue3330.let.ok.0.0.field: conflicting values null and {} (mismatched types null and struct): -+ // ./reroot.cue:6:16 -+ // ./reroot.cue:8:11 -+ // issue3330.let.ok.0.0.field.n: field not allowed: -+ // ./reroot.cue:9:11 -+ // ./reroot.cue:9:21 -+ } -+ } -+ } -+ matthew: (_|_){ -+ // [eval] -+ ok1: (_|_){ -+ // [eval] -+ #struct: (#struct){ -+ field: (#struct){ -+ n: (int){ 3 } -+ } -+ g: (#struct){ -+ } -+ } -+ out: (_|_){ -+ // [eval] -+ field: (_|_){ -+ // [eval] -+ n: (_|_){ -+ // [eval] issue3330.matthew.ok1.out.field.n: field not allowed: -+ // ./reroot.cue:16:22 -+ // ./reroot.cue:16:13 -+ } - } - g: (#struct){ - } -@@ -211,27 +209,25 @@ + } + } + issue3330: (struct){ +@@ -211,11 +179,11 @@ g: (#struct){ } } @@ -1593,45 +1457,15 @@ diff old new - n: (int){ 3 } - } - g: (#struct){ -- } -- } -- } -- } -- } -- issue3331: (struct){ -- original: (struct){ -- ok: (#list){ -- #A: (#struct){ -- let b#2 = (#struct){ -- } -- c: (#struct){ -- d: (int){ 1 } -- } -- } -- 0: (#struct){ + out2: (struct){ + field: (struct){ + n: (int){ 3 } + } + g: (struct){ -+ } -+ } -+ } -+ } -+ } -+ issue3331: (_|_){ -+ // [eval] -+ original: (_|_){ -+ // [eval] -+ ok: (_|_){ -+ // [eval] issue3331.original.ok.0.0.c.d: field not allowed: -+ // ./reroot.cue:36:7 -+ // ./reroot.cue:37:5 -+ #A: (#struct){ - let b#2 = (#struct){ } - c: (#struct){ -@@ -261,8 +257,7 @@ + } + } +@@ -261,8 +229,7 @@ } } } @@ -1641,7 +1475,7 @@ diff old new embed: (struct){ err1: (struct){ #A: (#struct){ -@@ -299,17 +294,15 @@ +@@ -299,17 +266,15 @@ c: (int){ 2 } } } @@ -1666,7 +1500,7 @@ diff old new X: (struct){ a: (struct){ e: (int){ 1 } -@@ -317,24 +310,13 @@ +@@ -317,24 +282,13 @@ b: (struct){ } } @@ -1698,7 +1532,7 @@ diff old new } } #X: (#struct){ -@@ -397,17 +379,11 @@ +@@ -397,17 +351,11 @@ // [eval] f: (_|_){ // [eval] nested.err1.x.b.f: field not allowed: @@ -1718,7 +1552,7 @@ diff old new } } v: (_|_){ -@@ -414,26 +390,19 @@ +@@ -414,27 +362,17 @@ // [eval] c: (_|_){ // [eval] @@ -1742,6 +1576,7 @@ diff old new - // ./reroot.cue:118:5 - // ./reroot.cue:122:8 - // ./reroot.cue:123:6 +- } + d: (_|_){ + // [eval] nested.err1.x.#V.c.d: field not allowed: + // ./reroot.cue:122:8 @@ -1752,13 +1587,11 @@ diff old new + // ./reroot.cue:122:8 + // ./reroot.cue:114:6 + } -+ g: (_|_){ -+ // [eval] nested.err1.x.#V.c.g: field not allowed: -+ // ./reroot.cue:122:8 - } ++ g: (int){ 1 } } } -@@ -441,24 +410,19 @@ + #V: (_|_){ +@@ -441,25 +379,17 @@ // [eval] c: (_|_){ // [eval] @@ -1780,6 +1613,7 @@ diff old new - // ./reroot.cue:118:5 - // ./reroot.cue:122:8 - // ./reroot.cue:123:6 +- } + d: (_|_){ + // [eval] nested.err1.x.#V.c.d: field not allowed: + // ./reroot.cue:122:8 @@ -1790,13 +1624,11 @@ diff old new + // ./reroot.cue:122:8 + // ./reroot.cue:114:6 + } -+ g: (_|_){ -+ // [eval] nested.err1.x.#V.c.g: field not allowed: -+ // ./reroot.cue:122:8 - } ++ g: (int){ 1 } } } -@@ -469,14 +433,14 @@ + } +@@ -469,14 +399,14 @@ } v: (#struct){ c: (#struct){ @@ -1815,38 +1647,24 @@ diff old new } } } -@@ -499,19 +463,27 @@ +@@ -499,19 +429,18 @@ // [eval] g: (_|_){ // [eval] nested.err2.x.b.g: field not allowed: - // ./reroot.cue:128:6 - // ./reroot.cue:136:5 -- // ./reroot.cue:137:8 -- } -- } -- c: (#struct){ -- g: (int){ 1 } + // ./reroot.cue:137:8 + } + } + c: (#struct){ ++ d: (int){ 1 } + g: (int){ 1 } - d: (int){ 1 } - } - } - } - } - inline: (struct){ -+ // ./reroot.cue:137:8 -+ } -+ } -+ c: (_|_){ -+ // [eval] -+ d: (_|_){ -+ // [eval] nested.err2.x.c.d: field not allowed: -+ // ./reroot.cue:129:6 -+ // ./reroot.cue:133:4 -+ } -+ g: (_|_){ -+ // [eval] nested.err2.x.c.g: field not allowed: -+ // ./reroot.cue:129:6 -+ // ./reroot.cue:137:8 -+ } + } + } + } @@ -1856,7 +1674,7 @@ diff old new #x: (#struct){ y: (#struct){ z?: (#struct){ -@@ -519,13 +491,23 @@ +@@ -519,13 +448,23 @@ } } } @@ -1889,8 +1707,17 @@ diff old new } -- diff/todo/p2 -- Positions / reordering + +indirect.embed.b1.d: should be an error, but is not an error in either version. -- diff/todo/p0 -- -Various closedness issues. +issue3325.ok.broken.items."my-item": should not be an error +-- diff/todo/p1 -- +Should be an error, but are not: + indirect.closed.err1.Y.a.e + nested.err1.x.v.c.d + nested.err1.x.v.c.f +-- diff/explanation -- +inline.err*.age*: fields are now correctly not allowed. -- out/compile -- --- embed.cue { diff --git a/cue/testdata/eval/conjuncts.txtar b/cue/testdata/eval/conjuncts.txtar index e93e4a01a91..a3fea5b67c5 100644 --- a/cue/testdata/eval/conjuncts.txtar +++ b/cue/testdata/eval/conjuncts.txtar @@ -76,7 +76,7 @@ Allocs: 235 Retain: 0 Unifications: 47 -Conjuncts: 475 +Conjuncts: 425 Disjuncts: 184 -- out/evalalpha -- Errors: @@ -173,7 +173,7 @@ diff old new -Conjuncts: 135 -Disjuncts: 86 +Unifications: 47 -+Conjuncts: 475 ++Conjuncts: 425 +Disjuncts: 184 -- diff/-out/evalalpha<==>+out/eval -- diff old new diff --git a/cue/testdata/eval/issue3330.txtar b/cue/testdata/eval/issue3330.txtar index a4730ddbc4d..1275e24c046 100644 --- a/cue/testdata/eval/issue3330.txtar +++ b/cue/testdata/eval/issue3330.txtar @@ -50,24 +50,14 @@ Errors: eliminated.x: conflicting values null and {} (mismatched types null and struct): ./in.cue:21:10 ./in.cue:22:5 -issue3330.0.0.field: conflicting values null and {} (mismatched types null and struct): - ./in.cue:5:15 - ./in.cue:9:10 -issue3330.0.0.field.n: field not allowed: - ./in.cue:10:10 - ./in.cue:10:20 eliminated.x.n: field not allowed: ./in.cue:23:5 ./in.cue:23:16 -simplified.out.field.n: field not allowed: - ./in.cue:31:21 - ./in.cue:31:12 Result: (_|_){ // [eval] - issue3330: (_|_){ - // [eval] + issue3330: (struct){ #A: (#struct){ let empty#1 = (#struct){ } @@ -75,13 +65,14 @@ Result: n: (int){ 3 } } } - out: (_|_){ - // [eval] issue3330.0.0.field: conflicting values null and {} (mismatched types null and struct): - // ./in.cue:5:15 - // ./in.cue:9:10 - // issue3330.0.0.field.n: field not allowed: - // ./in.cue:10:10 - // ./in.cue:10:20 + out: (#list){ + 0: (#struct){ + let empty#1 = (#struct){ + } + field: (#struct){ + n: (int){ 3 } + } + } } } eliminated: (_|_){ @@ -110,8 +101,7 @@ Result: // ./in.cue:23:16 } } - simplified: (_|_){ - // [eval] + simplified: (struct){ #struct: (#struct){ field: (#struct){ n: (int){ 3 } @@ -119,15 +109,9 @@ Result: g: (#struct){ } } - out: (_|_){ - // [eval] - field: (_|_){ - // [eval] - n: (_|_){ - // [eval] simplified.out.field.n: field not allowed: - // ./in.cue:31:21 - // ./in.cue:31:12 - } + out: (#struct){ + field: (#struct){ + n: (int){ 3 } } g: (#struct){ } @@ -138,7 +122,7 @@ Result: diff old new --- old +++ new -@@ -1,18 +1,25 @@ +@@ -1,11 +1,8 @@ Errors: -eliminated.x: 2 errors in empty disjunction: -eliminated.x: conflicting values null and {n:3} (mismatched types null and struct): @@ -146,53 +130,13 @@ diff old new + ./in.cue:21:10 ./in.cue:22:5 - ./in.cue:23:14 -+issue3330.0.0.field: conflicting values null and {} (mismatched types null and struct): -+ ./in.cue:5:15 -+ ./in.cue:9:10 -+issue3330.0.0.field.n: field not allowed: -+ ./in.cue:10:10 -+ ./in.cue:10:20 eliminated.x.n: field not allowed: - ./in.cue:21:10 - ./in.cue:22:14 ./in.cue:23:5 ./in.cue:23:16 -+simplified.out.field.n: field not allowed: -+ ./in.cue:31:21 -+ ./in.cue:31:12 - Result: - (_|_){ - // [eval] -- issue3330: (struct){ -+ issue3330: (_|_){ -+ // [eval] - #A: (#struct){ - let empty#1 = (#struct){ - } -@@ -20,14 +27,13 @@ - n: (int){ 3 } - } - } -- out: (#list){ -- 0: (#struct){ -- let empty#1 = (#struct){ -- } -- field: (#struct){ -- n: (int){ 3 } -- } -- } -+ out: (_|_){ -+ // [eval] issue3330.0.0.field: conflicting values null and {} (mismatched types null and struct): -+ // ./in.cue:5:15 -+ // ./in.cue:9:10 -+ // issue3330.0.0.field.n: field not allowed: -+ // ./in.cue:10:10 -+ // ./in.cue:10:20 - } - } - eliminated: (_|_){ -@@ -35,36 +41,29 @@ +@@ -35,31 +32,23 @@ #empty: (#struct){ } x: (_|_){ @@ -225,47 +169,15 @@ diff old new - // eliminated.x.n: field not allowed: - // ./in.cue:21:10 - // ./in.cue:22:14 -- // ./in.cue:23:5 -- // ./in.cue:23:16 -- } -- } -- simplified: (struct){ + // [eval] eliminated.x: conflicting values null and {} (mismatched types null and struct): + // ./in.cue:21:10 + // ./in.cue:22:5 + // eliminated.x.n: field not allowed: -+ // ./in.cue:23:5 -+ // ./in.cue:23:16 -+ } -+ } -+ simplified: (_|_){ -+ // [eval] - #struct: (#struct){ - field: (#struct){ - n: (int){ 3 } -@@ -72,9 +71,15 @@ - g: (#struct){ - } + // ./in.cue:23:5 + // ./in.cue:23:16 } -- out: (#struct){ -- field: (#struct){ -- n: (int){ 3 } -+ out: (_|_){ -+ // [eval] -+ field: (_|_){ -+ // [eval] -+ n: (_|_){ -+ // [eval] simplified.out.field.n: field not allowed: -+ // ./in.cue:31:21 -+ // ./in.cue:31:12 -+ } - } - g: (#struct){ - } --- diff/todo/p1 -- -References of structs within a definition that are usually allowed are -now triggered by refering to a definition outside of that definition. -Both "issue3330" and "simplified" should not have any errors. +-- diff/todo/p3 -- +Small differences in error output. Empty disjunction error. -- out/eval -- Errors: eliminated.x: 2 errors in empty disjunction: diff --git a/cue/testdata/eval/v0.7.txtar b/cue/testdata/eval/v0.7.txtar index ab62a6307ff..d550f4b351d 100644 --- a/cue/testdata/eval/v0.7.txtar +++ b/cue/testdata/eval/v0.7.txtar @@ -690,7 +690,7 @@ Allocs: 288 Retain: 0 Unifications: 258 -Conjuncts: 2303 +Conjuncts: 1989 Disjuncts: 44 -- out/evalalpha -- Errors: @@ -1148,7 +1148,7 @@ diff old new -Conjuncts: 1619 -Disjuncts: 436 +Unifications: 258 -+Conjuncts: 2303 ++Conjuncts: 1989 +Disjuncts: 44 -- diff/-out/evalalpha<==>+out/eval -- diff old new diff --git a/internal/core/adt/composite.go b/internal/core/adt/composite.go index 35f2ea771ef..1559a3516c7 100644 --- a/internal/core/adt/composite.go +++ b/internal/core/adt/composite.go @@ -295,6 +295,12 @@ func (v *Vertex) rootCloseContext(ctx *OpContext) *closeContext { } v.cc.incDependent(ctx, ROOT, nil) // matched in REF(decrement:nodeDone) } + v.cc.origin = v.cc + if p := v.Parent; p != nil { + pcc := p.rootCloseContext(ctx) + v.cc.origin = pcc.origin + } + return v.cc } diff --git a/internal/core/adt/conjunct.go b/internal/core/adt/conjunct.go index 51f313d8b84..b4ecdd0e856 100644 --- a/internal/core/adt/conjunct.go +++ b/internal/core/adt/conjunct.go @@ -410,26 +410,85 @@ func (n *nodeContext) scheduleVertexConjuncts(c Conjunct, arc *Vertex, closeInfo } } - if d, ok := arc.BaseValue.(*Disjunction); ok && false { - n.scheduleConjunct(MakeConjunct(c.Env, d, closeInfo), closeInfo) - } else { - for i := 0; i < len(arc.Conjuncts); i++ { - c := arc.Conjuncts[i] + // TODO(perf): buffer or use stack. + var a []*closeContext + a = appendPrefix(a, closeInfo.cc) + + // Use explicit index in case Conjuncts grows during iteration. + for i := 0; i < len(arc.Conjuncts); i++ { + c := arc.Conjuncts[i] + n.insertAndSkipConjuncts(a, c, closeInfo) + } - // Note that we are resetting the tree here. We hereby assume that - // closedness conflicts resulting from unifying the referenced arc were - // already caught there and that we can ignore further errors here. - // c.CloseInfo = closeInfo + if state := arc.getBareState(n.ctx); state != nil { + n.toComplete = true + } +} - // We can use the original, but we know it will not be used +// appendPrefix records the closeContext from the root of the current node to +// cc by walking up the parent chain and storing the results ancestor first. +// This is used to split conjunct trees into a forest of little trees. +func appendPrefix(a []*closeContext, cc *closeContext) []*closeContext { + if cc.parent != nil { + a = appendPrefix(a, cc.parent) + } + a = append(a, cc) + return a +} - n.scheduleConjunct(c, closeInfo) - } +// insertAndSkipConjuncts analyzes the conjunct tree represented by c and splits +// it into branches from the point where it deviates from the conjunct branch +// represented by skip. +// +// TODO(evalv3): Consider this example: +// +// #A: { +// b: {} // error only reported here. +// c: b & { +// // error (g not allowed) not reported here, as it would be okay if b +// // were valid. Instead, it is reported at b only. This is conform +// // the spec. +// d: 1 +// } +// } +// x: #A +// x: b: g: 1 +// +// We could also report an error at g by tracing if a conjunct crosses a isDef +// boundary between the root of c and the cc of the conjunct. +// Not doing so might have an effect on the outcome of disjunctions. This may be +// okay (ideally closedness is not modal), but something to consider. For now, +// we should probably copy whatever v2 was doing. +func (n *nodeContext) insertAndSkipConjuncts(skip []*closeContext, c Conjunct, id CloseInfo) { + if c.CloseInfo.cc == nil { + n.scheduleConjunct(c, id) + return } - if state := arc.getBareState(n.ctx); state != nil { - n.toComplete = true + root := c.CloseInfo.cc.origin + + // TODO(perf): closeContexts should be exact prefixes. So instead of + // searching the list, we could test them incrementally. This seems more + // robust for now as the data structure might slightly change and cause + // disalignment. + for _, s := range skip { + if root == s.origin { + switch x := c.Elem().(type) { + case *ConjunctGroup: + for _, c := range *x { + n.insertAndSkipConjuncts(skip, c, id) + } + + default: + // TODO: do leaf conjuncts that match need different treatment + // from those that don't? Right now, we treat them the same. + n.scheduleConjunct(c, id) + } + return + } } + + n.scheduleConjunct(c, id) } func (n *nodeContext) addNotify2(v *Vertex, c CloseInfo) []receiver { diff --git a/internal/core/adt/debug.go b/internal/core/adt/debug.go index 966a2ccde51..4ecfb219763 100644 --- a/internal/core/adt/debug.go +++ b/internal/core/adt/debug.go @@ -648,6 +648,15 @@ func (m *mermaidContext) pstr(cc *closeContext) string { flags.WriteByte(cc.arcType.String()[0]) io.Copy(w, flags) + // Show the origin of the closeContext. + indentOnNewline(w, 3) + ptr = fmt.Sprintf("%p", cc.origin) + if cc.origin != nil { + ptr = ptr[len(ptr)-sigPtrLen:] + } + w.WriteString("R:") + w.WriteString(ptr) + w.WriteString(close) switch { diff --git a/internal/core/adt/fields.go b/internal/core/adt/fields.go index 46d0e7da4e3..30f36bab52b 100644 --- a/internal/core/adt/fields.go +++ b/internal/core/adt/fields.go @@ -157,6 +157,11 @@ type closeContext struct { // Used to recursively insert Vertices. parent *closeContext + // points to the closeContext this closeContext originates when following + // the reverse or ARC/EVAL dependencies corresponding to parent vertices. + // This is used to compute the prefix path when resolving a reference. + origin *closeContext + // overlay is used to temporarily link a closeContext to its "overlay" copy, // as it is used in a corresponding disjunction. overlay *closeContext @@ -387,6 +392,7 @@ func (cc *closeContext) getKeyedCC(ctx *OpContext, key *closeContext, c CycleInf }, mode, false, checkClosed) arc := &closeContext{ + origin: cc.origin, generation: cc.generation, parent: parent, parentConjuncts: parent, @@ -484,6 +490,9 @@ func (c CloseInfo) spawnCloseContext(ctx *OpContext, t closeNodeType) (CloseInfo parentConjuncts: cc, } + // By definition, a spawned closeContext is its own root. + c.cc.origin = c.cc + cc.incDependent(ctx, PARENT, c.cc) // REF(decrement: spawn) switch t { diff --git a/internal/core/adt/overlay.go b/internal/core/adt/overlay.go index e221d02dbc7..54d9df9cb0b 100644 --- a/internal/core/adt/overlay.go +++ b/internal/core/adt/overlay.go @@ -264,6 +264,8 @@ func (ctx *overlayContext) allocCC(cc *closeContext) *closeContext { o := &closeContext{generation: ctx.generation} cc.overlay = o + // TODO(evalv3): is it okay to use the same origin in overlays? + o.origin = cc.origin if cc.parent != nil { o.parent = ctx.allocCC(cc.parent) @@ -319,6 +321,7 @@ func (ctx *overlayContext) initCloneCC(x *closeContext) { o.src = o.parent.src } + o.origin = x.origin o.conjunctCount = x.conjunctCount o.disjunctCount = x.disjunctCount o.isDef = x.isDef