From 99407a4c11879dc216f5f2ab06abc0ca1c932810 Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Mon, 7 Oct 2024 14:16:32 +0200 Subject: [PATCH] internal/core/adt: adapt handling of inline structs Before inline structs were always recomputed for any given arc, so it was safe to assume a rooted struct could be dropped. This does not have a big impact on performance as of this CL, but it will have a big impact in combination with upcoming changes. Now inline structs can be structure shared, it is important to know where the inline struct orginated. This has an effect on dependency analysis. It also allows us to work around some issues with internal/core/dep w.r.t. these upcoming changes. Issue #2854 Signed-off-by: Marcel van Lohuizen Change-Id: Id795fc7d710b992782342d4db046a41f9ef703ff Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1202264 TryBot-Result: CUEcueckoo Reviewed-by: Matthew Sackman Unity-Result: CUE porcuepine --- cue/testdata/builtins/incomplete.txtar | 210 +++++++++++++++++++++++++ cue/testdata/builtins/validators.txtar | 78 +++++++-- cue/testdata/cycle/chain.txtar | 4 +- cue/testdata/cycle/comprehension.txtar | 4 +- cue/testdata/cycle/evaluate.txtar | 71 ++++----- cue/testdata/cycle/self.txtar | 16 +- cue/testdata/eval/conjuncts.txtar | 18 ++- cue/testdata/eval/v0.7.txtar | 4 +- cue/testdata/export/issue2244.txtar | 36 +++++ internal/core/adt/composite.go | 48 +++++- internal/core/adt/context.go | 5 + internal/core/adt/expr.go | 6 + internal/core/adt/fields.go | 1 + internal/core/adt/share.go | 22 +++ internal/core/debug/debug.go | 2 +- 15 files changed, 450 insertions(+), 75 deletions(-) diff --git a/cue/testdata/builtins/incomplete.txtar b/cue/testdata/builtins/incomplete.txtar index 886702135cc..a4298af1895 100644 --- a/cue/testdata/builtins/incomplete.txtar +++ b/cue/testdata/builtins/incomplete.txtar @@ -110,6 +110,216 @@ Retain: 61 Unifications: 109 Conjuncts: 264 Disjuncts: 156 +-- out/evalalpha -- +Errors: +badListType.decimal: cannot use 2 (type int) as list in argument 1 to list.Max: + ./in.cue:79:11 +badListType.str: cannot use 2 (type int) as list in argument 1 to strings.Join: + ./in.cue:79:11 +badListError.x: invalid operands 2 and "foo" to '+' (type int and string): + ./in.cue:85:11 + ./in.cue:86:11 + +Result: +(_|_){ + // [eval] + list1: (struct){ + Out1: (#list){ + 0: (_|_){ + // [incomplete] list1._Sub: undefined field: b: + // ./in.cue:19:10 + } + } + Out2: (#list){ + 0: (_|_){ + // [incomplete] list1._Sub: undefined field: b: + // ./in.cue:19:10 + } + } + Out3: (#list){ + 0: (_|_){ + // [incomplete] list1._Sub: undefined field: b: + // ./in.cue:19:10 + } + } + Top: (#list){ + 0: (_|_){ + // [incomplete] list1._Sub: undefined field: b: + // ./in.cue:19:10 + } + } + _Sub: (_|_){ + // [incomplete] list1._Sub: undefined field: b: + // ./in.cue:19:10 + } + a: (struct){ + } + } + list2: (struct){ + Out1: (_|_){ + // [incomplete] list2.#Sub: undefined field: b: + // ./in.cue:33:10 + } + Out2: (_|_){ + // [incomplete] list2.#Sub: undefined field: b: + // ./in.cue:33:10 + } + Out3: (_|_){ + // [incomplete] list2.#Sub: undefined field: b: + // ./in.cue:33:10 + } + _Top: (_|_){ + // [incomplete] list2.#Sub: undefined field: b: + // ./in.cue:33:10 + } + #Sub: (_|_){ + // [incomplete] list2.#Sub: undefined field: b: + // ./in.cue:33:10 + } + a: (struct){ + } + } + value1: (struct){ + a: (_|_){ + // [incomplete] value1.a: unresolved disjunction 'sf' | 'dd' (type bytes): + // ./in.cue:39:5 + } + } + value2: (_|_){ + // [incomplete] value2: unresolved disjunction 'sf' | 'dd' (type bytes): + // ./in.cue:43:2 + } + incompleteArgDecimalList: (struct){ + a: (#struct){ + param: (int){ 123 } + transformed: (int){ 123 } + max: (int){ 123 } + } + #a: (#struct){ + param: (int){ int } + transformed: (_|_){ + // [incomplete] incompleteArgDecimalList.#a.transformed: operand param of '+' not concrete (was int): + // ./in.cue:50:17 + } + max: (_|_){ + // [incomplete] 0: operand param of '+' not concrete (was int): + // ./in.cue:50:17 + } + } + } + incompleteArgStringList: (struct){ + a: (#struct){ + param: (string){ "123" } + transformed: (string){ "123" } + joined: (string){ "123" } + } + #a: (#struct){ + param: (string){ string } + transformed: (_|_){ + // [incomplete] incompleteArgStringList.#a.transformed: non-concrete value string in operand to +: + // ./in.cue:59:16 + // ./in.cue:58:16 + } + joined: (_|_){ + // [incomplete] 0: non-concrete value string in operand to +: + // ./in.cue:59:16 + // ./in.cue:58:16 + } + } + } + incompleteList: (struct){ + x: (_){ _ } + decimal: (_|_){ + // [incomplete] incompleteList.decimal: non-concrete list for argument 0: + // ./in.cue:66:11 + } + str: (_|_){ + // [incomplete] incompleteList.str: non-concrete list for argument 0: + // ./in.cue:67:11 + } + } + incompleteListError: (struct){ + x: (_|_){ + // [incomplete] incompleteListError.x: non-concrete value _ in operand to +: + // ./in.cue:72:11 + } + y: (_){ _ } + decimal: (_|_){ + // [incomplete] incompleteListError.x: non-concrete value _ in operand to +: + // ./in.cue:72:11 + } + str: (_|_){ + // [incomplete] incompleteListError.x: non-concrete value _ in operand to +: + // ./in.cue:72:11 + } + } + badListType: (_|_){ + // [eval] + x: (int){ 2 } + decimal: (_|_){ + // [eval] badListType.decimal: cannot use 2 (type int) as list in argument 1 to list.Max: + // ./in.cue:79:11 + } + str: (_|_){ + // [eval] badListType.str: cannot use 2 (type int) as list in argument 1 to strings.Join: + // ./in.cue:79:11 + } + } + badListError: (_|_){ + // [eval] + x: (_|_){ + // [eval] badListError.x: invalid operands 2 and "foo" to '+' (type int and string): + // ./in.cue:85:11 + // ./in.cue:86:11 + } + y: (string){ "foo" } + decimal: (_|_){ + // [eval] badListError.x: invalid operands 2 and "foo" to '+' (type int and string): + // ./in.cue:85:11 + // ./in.cue:86:11 + } + str: (_|_){ + // [eval] badListError.x: invalid operands 2 and "foo" to '+' (type int and string): + // ./in.cue:85:11 + // ./in.cue:86:11 + } + } + multipleErrors: (struct){ + #T: (#struct){ + params: (#struct){ + x: (string){ string } + y: (string){ string } + } + out: (_|_){ + // [incomplete] multipleErrors.#T.out: error in call to text/template.Execute: cannot convert non-concrete value string: + // ./in.cue:99:8 + // ./in.cue:96:4 + } + } + } +} +-- diff/-out/evalalpha<==>+out/eval -- +diff old new +--- old ++++ new +@@ -89,7 +89,7 @@ + // ./in.cue:50:17 + } + max: (_|_){ +- // [incomplete] incompleteArgDecimalList.#a.0: operand param of '+' not concrete (was int): ++ // [incomplete] 0: operand param of '+' not concrete (was int): + // ./in.cue:50:17 + } + } +@@ -108,7 +108,7 @@ + // ./in.cue:58:16 + } + joined: (_|_){ +- // [incomplete] incompleteArgStringList.#a.0: non-concrete value string in operand to +: ++ // [incomplete] 0: non-concrete value string in operand to +: + // ./in.cue:59:16 + // ./in.cue:58:16 + } -- out/eval -- Errors: badListType.decimal: cannot use 2 (type int) as list in argument 1 to list.Max: diff --git a/cue/testdata/builtins/validators.txtar b/cue/testdata/builtins/validators.txtar index e24c5b20c4d..d7bbbc627a6 100644 --- a/cue/testdata/builtins/validators.txtar +++ b/cue/testdata/builtins/validators.txtar @@ -201,25 +201,25 @@ Disjuncts: 131 Errors: callOfCallToValidator.e: cannot call previously called validator b: ./in.cue:94:5 -issue3418.t1: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.0: conflicting values 2 and 1))): conflicting values 2 and 1: +issue3418.t1: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: ./issue3418.cue:1:24 ./issue3418.cue:1:16 ./issue3418.cue:1:31 ./issue3418.cue:1:35 ./issue3418.cue:1:37 -issue3418.t2: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.0: conflicting values 2 and 1))): conflicting values 2 and 1: +issue3418.t2: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: ./issue3418.cue:2:41 ./issue3418.cue:2:16 ./issue3418.cue:2:48 ./issue3418.cue:2:52 ./issue3418.cue:2:54 -issue3418.t3.x: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.t3.0: conflicting values 2 and 1))): conflicting values 2 and 1: +issue3418.t3.x: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: ./issue3418.cue:6:5 ./issue3418.cue:4:5 ./issue3418.cue:6:12 ./issue3418.cue:6:16 ./issue3418.cue:6:18 -issue3418.t4.x: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.t4.0: conflicting values 2 and 1))): conflicting values 2 and 1: +issue3418.t4.x: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: ./issue3418.cue:10:5 ./issue3418.cue:9:5 ./issue3418.cue:10:12 @@ -337,7 +337,7 @@ Result: issue3418: (_|_){ // [eval] t1: (_|_){ - // [eval] issue3418.t1: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.0: conflicting values 2 and 1))): conflicting values 2 and 1: + // [eval] issue3418.t1: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: // ./issue3418.cue:1:24 // ./issue3418.cue:1:16 // ./issue3418.cue:1:31 @@ -345,7 +345,7 @@ Result: // ./issue3418.cue:1:37 } t2: (_|_){ - // [eval] issue3418.t2: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.0: conflicting values 2 and 1))): conflicting values 2 and 1: + // [eval] issue3418.t2: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: // ./issue3418.cue:2:41 // ./issue3418.cue:2:16 // ./issue3418.cue:2:48 @@ -355,7 +355,7 @@ Result: t3: (_|_){ // [eval] x: (_|_){ - // [eval] issue3418.t3.x: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.t3.0: conflicting values 2 and 1))): conflicting values 2 and 1: + // [eval] issue3418.t3.x: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: // ./issue3418.cue:6:5 // ./issue3418.cue:4:5 // ./issue3418.cue:6:12 @@ -366,7 +366,7 @@ Result: t4: (_|_){ // [eval] x: (_|_){ - // [eval] issue3418.t4.x: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.t4.0: conflicting values 2 and 1))): conflicting values 2 and 1: + // [eval] issue3418.t4.x: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: // ./issue3418.cue:10:5 // ./issue3418.cue:9:5 // ./issue3418.cue:10:12 @@ -446,15 +446,36 @@ Result: diff old new --- old +++ new -@@ -16,7 +16,6 @@ - issue3418.t3.x: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.t3.0: conflicting values 2 and 1))): conflicting values 2 and 1: +@@ -1,49 +1,43 @@ + Errors: + callOfCallToValidator.e: cannot call previously called validator b: + ./in.cue:94:5 +-issue3418.t1: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.0: conflicting values 2 and 1))): conflicting values 2 and 1: ++issue3418.t1: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: + ./issue3418.cue:1:24 + ./issue3418.cue:1:16 + ./issue3418.cue:1:31 + ./issue3418.cue:1:35 + ./issue3418.cue:1:37 +-issue3418.t2: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.0: conflicting values 2 and 1))): conflicting values 2 and 1: ++issue3418.t2: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: + ./issue3418.cue:2:41 + ./issue3418.cue:2:16 + ./issue3418.cue:2:48 + ./issue3418.cue:2:52 + ./issue3418.cue:2:54 +-issue3418.t3.x: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.t3.0: conflicting values 2 and 1))): conflicting values 2 and 1: ++issue3418.t3.x: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: ./issue3418.cue:6:5 ./issue3418.cue:4:5 - ./issue3418.cue:5:5 ./issue3418.cue:6:12 ./issue3418.cue:6:16 ./issue3418.cue:6:18 -@@ -26,24 +25,19 @@ +-issue3418.t4.x: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.t4.0: conflicting values 2 and 1))): conflicting values 2 and 1: ++issue3418.t4.x: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: + ./issue3418.cue:10:5 + ./issue3418.cue:9:5 ./issue3418.cue:10:12 ./issue3418.cue:10:16 ./issue3418.cue:10:18 @@ -513,15 +534,44 @@ diff old new // ./in.cue:112:20 } } -@@ -171,7 +158,6 @@ - // [eval] issue3418.t3.x: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.t3.0: conflicting values 2 and 1))): conflicting values 2 and 1: +@@ -150,7 +137,7 @@ + issue3418: (_|_){ + // [eval] + t1: (_|_){ +- // [eval] issue3418.t1: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.0: conflicting values 2 and 1))): conflicting values 2 and 1: ++ // [eval] issue3418.t1: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: + // ./issue3418.cue:1:24 + // ./issue3418.cue:1:16 + // ./issue3418.cue:1:31 +@@ -158,7 +145,7 @@ + // ./issue3418.cue:1:37 + } + t2: (_|_){ +- // [eval] issue3418.t2: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.0: conflicting values 2 and 1))): conflicting values 2 and 1: ++ // [eval] issue3418.t2: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: + // ./issue3418.cue:2:41 + // ./issue3418.cue:2:16 + // ./issue3418.cue:2:48 +@@ -168,10 +155,9 @@ + t3: (_|_){ + // [eval] + x: (_|_){ +- // [eval] issue3418.t3.x: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.t3.0: conflicting values 2 and 1))): conflicting values 2 and 1: ++ // [eval] issue3418.t3.x: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: // ./issue3418.cue:6:5 // ./issue3418.cue:4:5 - // ./issue3418.cue:5:5 // ./issue3418.cue:6:12 // ./issue3418.cue:6:16 // ./issue3418.cue:6:18 -@@ -186,7 +172,6 @@ +@@ -180,13 +166,12 @@ + t4: (_|_){ + // [eval] + x: (_|_){ +- // [eval] issue3418.t4.x: invalid value "foo" (does not satisfy matchN(1, _|_(issue3418.t4.0: conflicting values 2 and 1))): conflicting values 2 and 1: ++ // [eval] issue3418.t4.x: invalid value "foo" (does not satisfy matchN(1, _|_(0: conflicting values 2 and 1))): conflicting values 2 and 1: + // ./issue3418.cue:10:5 + // ./issue3418.cue:9:5 // ./issue3418.cue:10:12 // ./issue3418.cue:10:16 // ./issue3418.cue:10:18 diff --git a/cue/testdata/cycle/chain.txtar b/cue/testdata/cycle/chain.txtar index 1656bb121a9..ef02d66b95a 100644 --- a/cue/testdata/cycle/chain.txtar +++ b/cue/testdata/cycle/chain.txtar @@ -215,7 +215,7 @@ Allocs: 20363 Retain: 0 Unifications: 7655 -Conjuncts: 109511 +Conjuncts: 109521 Disjuncts: 13941 -- out/evalalpha -- Errors: @@ -735,7 +735,7 @@ diff old new -Conjuncts: 3177 -Disjuncts: 1979 +Unifications: 7655 -+Conjuncts: 109511 ++Conjuncts: 109521 +Disjuncts: 13941 -- diff/-out/evalalpha<==>+out/eval -- diff old new diff --git a/cue/testdata/cycle/comprehension.txtar b/cue/testdata/cycle/comprehension.txtar index 84e0c7c233d..7fdfa17ef0d 100644 --- a/cue/testdata/cycle/comprehension.txtar +++ b/cue/testdata/cycle/comprehension.txtar @@ -317,7 +317,7 @@ Allocs: 779 Retain: 0 Unifications: 499 -Conjuncts: 3043 +Conjuncts: 3361 Disjuncts: 196 -- out/evalalpha -- Errors: @@ -923,7 +923,7 @@ diff old new -Conjuncts: 2525 -Disjuncts: 1404 +Unifications: 499 -+Conjuncts: 3043 ++Conjuncts: 3361 +Disjuncts: 196 -- out/eval/stats -- Leaks: 50 diff --git a/cue/testdata/cycle/evaluate.txtar b/cue/testdata/cycle/evaluate.txtar index 9bb76bec931..8d38644601c 100644 --- a/cue/testdata/cycle/evaluate.txtar +++ b/cue/testdata/cycle/evaluate.txtar @@ -137,13 +137,13 @@ disjunctionCycle.b: cannot use 1 (type int) as list in argument 1 to and: ./in.cue:56:9 b: structural cycle: ./in.cue:62:6 -closeCycle.d: structural cycle: +d: structural cycle: ./in.cue:73:11 -embedCycle: structural cycle: +structural cycle: ./in.cue:85:12 -listAddCycle.0.0: structural cycle: +0.0: structural cycle: ./in.cue:91:5 -listMulCycle.0.a.b: structural cycle: +0.a.b: structural cycle: ./in.cue:97:5 closeFail.x.b: field not allowed: ./in.cue:105:6 @@ -249,15 +249,15 @@ Result: closeCycle: (_|_){ // [structural cycle] a: (_|_){ - // [structural cycle] closeCycle.d: structural cycle: + // [structural cycle] d: structural cycle: // ./in.cue:73:11 } b: (_|_){ - // [structural cycle] closeCycle.d: structural cycle: + // [structural cycle] d: structural cycle: // ./in.cue:73:11 } c: (_|_){ - // [structural cycle] closeCycle.d: structural cycle: + // [structural cycle] d: structural cycle: // ./in.cue:73:11 } } @@ -277,45 +277,45 @@ Result: embedCycle: (_|_){ // [structural cycle] a: (_|_){ - // [structural cycle] embedCycle: structural cycle: + // [structural cycle] structural cycle: // ./in.cue:85:12 } b: (_|_){ - // [structural cycle] embedCycle: structural cycle: + // [structural cycle] structural cycle: // ./in.cue:85:12 } c: (_|_){ - // [structural cycle] embedCycle: structural cycle: + // [structural cycle] structural cycle: // ./in.cue:85:12 } } listAddCycle: (_|_){ // [structural cycle] a: (_|_){ - // [structural cycle] listAddCycle.0.0: structural cycle: + // [structural cycle] 0.0: structural cycle: // ./in.cue:91:5 } b: (_|_){ - // [structural cycle] listAddCycle.0.0: structural cycle: + // [structural cycle] 0.0: structural cycle: // ./in.cue:91:5 } c: (_|_){ - // [structural cycle] listAddCycle.0.0: structural cycle: + // [structural cycle] 0.0: structural cycle: // ./in.cue:91:5 } } listMulCycle: (_|_){ // [structural cycle] a: (_|_){ - // [structural cycle] listMulCycle.0.a.b: structural cycle: + // [structural cycle] 0.a.b: structural cycle: // ./in.cue:97:5 } b: (_|_){ - // [structural cycle] listMulCycle.0.a.b: structural cycle: + // [structural cycle] 0.a.b: structural cycle: // ./in.cue:97:5 } c: (_|_){ - // [structural cycle] listMulCycle.0.a.b: structural cycle: + // [structural cycle] 0.a.b: structural cycle: // ./in.cue:97:5 } } @@ -379,16 +379,17 @@ diff old new - ./in.cue:73:15 -structCycle.c: structural cycle: - ./in.cue:79:14 -+closeCycle.d: structural cycle: -+ ./in.cue:73:11 - embedCycle: structural cycle: +-embedCycle: structural cycle: - ./in.cue:85:11 -printCycle.a.X.X: structural cycle: - ./in.cue:113:6 ++d: structural cycle: ++ ./in.cue:73:11 ++structural cycle: + ./in.cue:85:12 -+listAddCycle.0.0: structural cycle: ++0.0: structural cycle: + ./in.cue:91:5 -+listMulCycle.0.a.b: structural cycle: ++0.a.b: structural cycle: + ./in.cue:97:5 +closeFail.x.b: field not allowed: + ./in.cue:105:6 @@ -480,7 +481,7 @@ diff old new // ./in.cue:56:9 } } -@@ -118,41 +116,34 @@ +@@ -118,80 +116,79 @@ } b: (struct){ } @@ -504,15 +505,15 @@ diff old new - c: (_|_){ - // [structural cycle] closeCycle.c: structural cycle: - // ./in.cue:73:15 -+ // [structural cycle] closeCycle.d: structural cycle: ++ // [structural cycle] d: structural cycle: + // ./in.cue:73:11 + } + b: (_|_){ -+ // [structural cycle] closeCycle.d: structural cycle: ++ // [structural cycle] d: structural cycle: + // ./in.cue:73:11 + } + c: (_|_){ -+ // [structural cycle] closeCycle.d: structural cycle: ++ // [structural cycle] d: structural cycle: + // ./in.cue:73:11 } } @@ -542,10 +543,9 @@ diff old new } } embedCycle: (_|_){ -@@ -159,39 +150,45 @@ // [structural cycle] a: (_|_){ - // [structural cycle] embedCycle: structural cycle: +- // [structural cycle] embedCycle: structural cycle: - // ./in.cue:85:11 - } - b: (_|_){ @@ -555,14 +555,15 @@ diff old new - c: (_|_){ - // [structural cycle] embedCycle: structural cycle: - // ./in.cue:85:11 ++ // [structural cycle] structural cycle: + // ./in.cue:85:12 + } + b: (_|_){ -+ // [structural cycle] embedCycle: structural cycle: ++ // [structural cycle] structural cycle: + // ./in.cue:85:12 + } + c: (_|_){ -+ // [structural cycle] embedCycle: structural cycle: ++ // [structural cycle] structural cycle: + // ./in.cue:85:12 } } @@ -576,15 +577,15 @@ diff old new - } - c: (_|_){ - // [structural cycle] -+ // [structural cycle] listAddCycle.0.0: structural cycle: ++ // [structural cycle] 0.0: structural cycle: + // ./in.cue:91:5 + } + b: (_|_){ -+ // [structural cycle] listAddCycle.0.0: structural cycle: ++ // [structural cycle] 0.0: structural cycle: + // ./in.cue:91:5 + } + c: (_|_){ -+ // [structural cycle] listAddCycle.0.0: structural cycle: ++ // [structural cycle] 0.0: structural cycle: + // ./in.cue:91:5 } } @@ -598,15 +599,15 @@ diff old new - } - c: (_|_){ - // [structural cycle] -+ // [structural cycle] listMulCycle.0.a.b: structural cycle: ++ // [structural cycle] 0.a.b: structural cycle: + // ./in.cue:97:5 + } + b: (_|_){ -+ // [structural cycle] listMulCycle.0.a.b: structural cycle: ++ // [structural cycle] 0.a.b: structural cycle: + // ./in.cue:97:5 + } + c: (_|_){ -+ // [structural cycle] listMulCycle.0.a.b: structural cycle: ++ // [structural cycle] 0.a.b: structural cycle: + // ./in.cue:97:5 } } diff --git a/cue/testdata/cycle/self.txtar b/cue/testdata/cycle/self.txtar index d1e5ff0a357..669471edc71 100644 --- a/cue/testdata/cycle/self.txtar +++ b/cue/testdata/cycle/self.txtar @@ -282,9 +282,9 @@ expr.error1.a: conflicting values 4 and 3: expr.error2.a: conflicting values 4 and 3: ./in.cue:8:5 ./in.cue:9:5 -list.error1.0: structural cycle: +0: structural cycle: ./in.cue:13:5 -list.error2.1: structural cycle: +1: structural cycle: ./in.cue:17:5 Result: @@ -314,14 +314,14 @@ Result: error1: (_|_){ // [structural cycle] a: (_|_){ - // [structural cycle] list.error1.0: structural cycle: + // [structural cycle] 0: structural cycle: // ./in.cue:13:5 } } error2: (_|_){ // [structural cycle] a: (_|_){ - // [structural cycle] list.error2.1: structural cycle: + // [structural cycle] 1: structural cycle: // ./in.cue:17:5 } } @@ -632,9 +632,9 @@ diff old new expr.error2.a: conflicting values 4 and 3: ./in.cue:8:5 ./in.cue:9:5 -+list.error1.0: structural cycle: ++0: structural cycle: + ./in.cue:13:5 -+list.error2.1: structural cycle: ++1: structural cycle: + ./in.cue:17:5 Result: @@ -654,14 +654,14 @@ diff old new - // [structural cycle] - 0: (string){ "1" } - 1: (string){ "2" } -+ // [structural cycle] list.error1.0: structural cycle: ++ // [structural cycle] 0: structural cycle: + // ./in.cue:13:5 + } + } + error2: (_|_){ + // [structural cycle] + a: (_|_){ -+ // [structural cycle] list.error2.1: structural cycle: ++ // [structural cycle] 1: structural cycle: + // ./in.cue:17:5 } } diff --git a/cue/testdata/eval/conjuncts.txtar b/cue/testdata/eval/conjuncts.txtar index e84d3a5c4fd..7709c1f3780 100644 --- a/cue/testdata/eval/conjuncts.txtar +++ b/cue/testdata/eval/conjuncts.txtar @@ -76,11 +76,11 @@ Allocs: 235 Retain: 0 Unifications: 47 -Conjuncts: 425 +Conjuncts: 436 Disjuncts: 184 -- out/evalalpha -- Errors: -issue2351.let.param: conflicting values "foo" and [{}] (mismatched types string and list): +param: conflicting values "foo" and [{}] (mismatched types string and list): ./issue2351.cue:2:15 ./issue2351.cue:4:19 @@ -116,7 +116,7 @@ Result: issue2351: (_|_){ // [eval] let: (_|_){ - // [eval] issue2351.let.param: conflicting values "foo" and [{}] (mismatched types string and list): + // [eval] param: conflicting values "foo" and [{}] (mismatched types string and list): // ./issue2351.cue:2:15 // ./issue2351.cue:4:19 let _param#1 = (string){ "foo" } @@ -173,7 +173,7 @@ diff old new -Conjuncts: 135 -Disjuncts: 86 +Unifications: 47 -+Conjuncts: 425 ++Conjuncts: 436 +Disjuncts: 184 -- diff/-out/evalalpha<==>+out/eval -- diff old new @@ -181,7 +181,8 @@ diff old new +++ new @@ -1,7 +1,6 @@ Errors: - issue2351.let.param: conflicting values "foo" and [{}] (mismatched types string and list): +-issue2351.let.param: conflicting values "foo" and [{}] (mismatched types string and list): ++param: conflicting values "foo" and [{}] (mismatched types string and list): ./issue2351.cue:2:15 - ./issue2351.cue:3:2 ./issue2351.cue:4:19 @@ -209,9 +210,12 @@ diff old new } handleComprehensions: (struct){ #sub2: (int){ 2 } -@@ -39,7 +37,6 @@ +@@ -37,9 +35,8 @@ + issue2351: (_|_){ + // [eval] let: (_|_){ - // [eval] issue2351.let.param: conflicting values "foo" and [{}] (mismatched types string and list): +- // [eval] issue2351.let.param: conflicting values "foo" and [{}] (mismatched types string and list): ++ // [eval] param: conflicting values "foo" and [{}] (mismatched types string and list): // ./issue2351.cue:2:15 - // ./issue2351.cue:3:2 // ./issue2351.cue:4:19 diff --git a/cue/testdata/eval/v0.7.txtar b/cue/testdata/eval/v0.7.txtar index 523249329b7..7c13ba67e38 100644 --- a/cue/testdata/eval/v0.7.txtar +++ b/cue/testdata/eval/v0.7.txtar @@ -690,7 +690,7 @@ Allocs: 278 Retain: 0 Unifications: 248 -Conjuncts: 1883 +Conjuncts: 1900 Disjuncts: 44 -- out/evalalpha -- Errors: @@ -1110,7 +1110,7 @@ diff old new -Conjuncts: 1619 -Disjuncts: 436 +Unifications: 248 -+Conjuncts: 1883 ++Conjuncts: 1900 +Disjuncts: 44 -- diff/-out/evalalpha<==>+out/eval -- diff old new diff --git a/cue/testdata/export/issue2244.txtar b/cue/testdata/export/issue2244.txtar index 72d68cdaecb..3d3d90e5fef 100644 --- a/cue/testdata/export/issue2244.txtar +++ b/cue/testdata/export/issue2244.txtar @@ -44,6 +44,42 @@ Disjuncts: 39 -- diff/explanation -- New evaluator should ultimately disallow new definitions in closed structs and not allow #pattern in #step +-- out/evalalpha -- +(struct){ + _#matchPattern(:x): (_|_){ + // [incomplete] error in call to strings.HasSuffix: non-concrete value string: + // ./in.cue:11:6 + // ./in.cue:8:12 + #pattern: (string){ string } + } + _#isReleaseTag(:x): (string){ + "refs/tags/v" + #pattern: (string){ "refs/tags/v*" } + let prefix#1 = (string){ "refs/tags/v" } + } + #step: (#struct){ + if: ((string|number)){ |((number){ number }, (string){ string }) } + } + m: (#struct){ + if: (string){ + "refs/tags/v" + #pattern: (string){ "refs/tags/v*" } + let prefix#1 = (string){ "refs/tags/v" } + } + } +} +-- diff/-out/evalalpha<==>+out/eval -- +diff old new +--- old ++++ new +@@ -1,6 +1,6 @@ + (struct){ + _#matchPattern(:x): (_|_){ +- // [incomplete] _#matchPattern: error in call to strings.HasSuffix: non-concrete value string: ++ // [incomplete] error in call to strings.HasSuffix: non-concrete value string: + // ./in.cue:11:6 + // ./in.cue:8:12 + #pattern: (string){ string } -- out/eval -- (struct){ _#matchPattern(:x): (_|_){ diff --git a/internal/core/adt/composite.go b/internal/core/adt/composite.go index bff45f37d98..fa8956d4b0a 100644 --- a/internal/core/adt/composite.go +++ b/internal/core/adt/composite.go @@ -206,8 +206,20 @@ type Vertex struct { // Used for cycle detection. IsDynamic bool + // nonRooted indicates that this Vertex originates within the context of + // a dynamic, or inlined, Vertex (e.g. `{out: ...}.out``). Note that, + // through reappropriation, this Vertex may become rooted down the line. + // Use the !IsDetached method to determine whether this Vertex became + // rooted. nonRooted bool // indicates that there is no path from the root of the tree. + // anonymous indicates that this Vertex is being computed within a + // addressable context, or in other words, a context for which there is + // a path from the root of the file. Typically, the only addressable + // contexts are fields. Examples of fields that are not addressable are + // the for source of comprehensions and let fields or let clauses. + anonymous bool + // hasPendingArc is set if this Vertex has a void arc (e.g. for comprehensions) hasPendingArc bool @@ -307,13 +319,20 @@ func (v *Vertex) rootCloseContext(ctx *OpContext) *closeContext { // newInlineVertex creates a Vertex that is needed for computation, but for // which there is no CUE path defined from the root Vertex. func (ctx *OpContext) newInlineVertex(parent *Vertex, v BaseValue, a ...Conjunct) *Vertex { - return &Vertex{ - Parent: parent, + n := &Vertex{ BaseValue: v, IsDynamic: true, ArcType: ArcMember, Conjuncts: a, } + if !ctx.isDevVersion() { + n.Parent = parent + } + if ctx.inDetached > 0 { + n.anonymous = true + } + return n + } // updateArcType updates v.ArcType if t is more restrictive. @@ -367,12 +386,32 @@ func (v *Vertex) IsDefined(c *OpContext) bool { return v.isDefined() } -// Rooted reports whether there is a path from the root of the tree to this -// Vertex. +// Rooted reports if it is known there is a path from the root of the tree to +// this Vertex. If this returns false, it may still be rooted if the node +// originated from an inline struct, but was later reappropriated. func (v *Vertex) Rooted() bool { return !v.nonRooted && !v.Label.IsLet() && !v.IsDynamic } +// IsDetached reports whether this Vertex does not have a path from the root. +func (v *Vertex) IsDetached() bool { + // v might have resulted from an inline struct that was subsequently shared. + // In this case, it is still rooted. + for ; v != nil; v = v.Parent { + if v.Rooted() { + return false + } + } + + return true +} + +// MayAttach reports whether this Vertex may attach to another arc. +// The behavior is undefined if IsDetached is true. +func (v *Vertex) MayAttach() bool { + return !v.Label.IsLet() && !v.anonymous +} + type ArcType uint8 const ( @@ -1171,6 +1210,7 @@ func (v *Vertex) GetArc(c *OpContext, f Feature, t ArcType) (arc *Vertex, isNew Label: f, ArcType: t, nonRooted: v.IsDynamic || v.Label.IsLet() || v.nonRooted, + anonymous: v.anonymous || v.Label.IsLet(), } v.Arcs = append(v.Arcs, arc) if t == ArcPending { diff --git a/internal/core/adt/context.go b/internal/core/adt/context.go index a3b9ce8e580..812dbb275b3 100644 --- a/internal/core/adt/context.go +++ b/internal/core/adt/context.go @@ -263,6 +263,11 @@ type OpContext struct { // enabled. inConstraint int + // inDetached indicates that inline structs evaluated in the current context + // should never be shared. This is the case, for instance, with the source + // for the for clause in a comprehension. + inDetached int + // inValidator defines whether full evaluation need to be enforced, for // instance when comparing against bottom. inValidator int diff --git a/internal/core/adt/expr.go b/internal/core/adt/expr.go index 04662539506..f5d6318302c 100644 --- a/internal/core/adt/expr.go +++ b/internal/core/adt/expr.go @@ -1979,7 +1979,9 @@ func (c *OpContext) forSource(x Expr) *Vertex { // TODO: always get the vertex. This allows a whole bunch of trickery // down the line. + c.inDetached++ v := c.unifyNode(x, state) + c.inDetached-- node, ok := v.(*Vertex) if ok && c.isDevVersion() { @@ -2096,6 +2098,7 @@ func (x *ForClause) yield(s *compState) { // processing, eluding the deallocation step. status: finalized, IsDynamic: true, + anonymous: true, ArcType: ArcMember, } @@ -2104,6 +2107,7 @@ func (x *ForClause) yield(s *compState) { Label: x.Value, BaseValue: a, IsDynamic: true, + anonymous: true, ArcType: ArcPending, } n.Arcs = append(n.Arcs, b) @@ -2113,6 +2117,7 @@ func (x *ForClause) yield(s *compState) { v := &Vertex{ Label: x.Key, IsDynamic: true, + anonymous: true, } key := a.Label.ToValue(c) v.AddConjunct(MakeRootConjunct(c.Env(0), key)) @@ -2172,6 +2177,7 @@ func (x *LetClause) yield(s *compState) { { Label: x.Label, IsDynamic: true, + anonymous: true, Conjuncts: []Conjunct{{c.Env(0), x.Expr, c.ci}}, }, }} diff --git a/internal/core/adt/fields.go b/internal/core/adt/fields.go index 6d1559439ab..a91531ede1a 100644 --- a/internal/core/adt/fields.go +++ b/internal/core/adt/fields.go @@ -341,6 +341,7 @@ func (n *nodeContext) getArc(f Feature, mode ArcType) (arc *Vertex, isNew bool) Label: f, ArcType: mode, nonRooted: v.IsDynamic || v.Label.IsLet() || v.nonRooted, + anonymous: v.anonymous || v.Label.IsLet(), } if n.scheduler.frozen&fieldSetKnown != 0 { b := n.ctx.NewErrf("adding field %v not allowed as field set was already referenced", f) diff --git a/internal/core/adt/share.go b/internal/core/adt/share.go index 5d884c6de23..b8937751945 100644 --- a/internal/core/adt/share.go +++ b/internal/core/adt/share.go @@ -58,6 +58,20 @@ func (n *nodeContext) finalizeSharing() { n.sharedID.cc.decDependent(n.ctx, SHARED, n.node.cc) n.sharedID.cc = nil } + if n.isShared { + v := n.node.BaseValue.(*Vertex) + if v.Parent == n.node.Parent { + v.unify(n.ctx, needTasksDone, finalize) + if !v.Rooted() && v.MayAttach() { + n.isShared = false + n.node.Arcs = v.Arcs + n.node.BaseValue = v.BaseValue + n.node.status = v.status + n.node.Closed = v.Closed + n.node.HasEllipsis = v.HasEllipsis + } + } + } } func (n *nodeContext) share(c Conjunct, arc *Vertex, id CloseInfo) { @@ -71,6 +85,14 @@ func (n *nodeContext) share(c Conjunct, arc *Vertex, id CloseInfo) { n.shared = c n.sharedID = id + if arc.IsDetached() && !arc.anonymous { // Second check necessary ? XXX + // If the status is just "conjuncts", we could just take over the arcs. + arc.Parent = n.node.Parent + for _, a := range arc.Arcs { + a.Parent = n.node + } + } + // At this point, the node may still be unshared at a later point. For this // purpose we need to keep the retain count above zero until all conjuncts // have been processed and it is clear that sharing is possible. Delaying diff --git a/internal/core/debug/debug.go b/internal/core/debug/debug.go index 5e4f3525395..764b10b84e4 100644 --- a/internal/core/debug/debug.go +++ b/internal/core/debug/debug.go @@ -142,7 +142,7 @@ func (w *printer) printShared(v0 *adt.Vertex) (x *adt.Vertex, ok bool) { isCyclic := v0.IsCyclic s, ok := v0.BaseValue.(*adt.Vertex) v1 := v0.DerefValue() - useReference := v0.IsShared && v1.Rooted() + useReference := v0.IsShared && !v1.IsDetached() && v1.MayAttach() isCyclic = isCyclic || v1.IsCyclic _ = isCyclic // NOTE(debug): use this line instead of the following to expand shared