From e4c4b8e8a15fa5f36e1f0d02379b7b9d8563f506 Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Fri, 13 Dec 2024 13:58:06 +0100 Subject: [PATCH] internal/core/adt: dereference disjunct result With nested disjunctions, disjuncts may end up forwarded in Vertex.BaseValue in intermediate results. This causes closeContext.arcs and Arcs to be misaligned if these results are used in further processing. This is the case, for instance in the following example: {#v: 1} | 2 {#v: 1} | (2 | 3) {#v: 1} | 2 {#v: 1} | 2 This issue led to duplicate disjuncts and many other problems. The EvalAlpha test runs over 10% faster as a result of this change! The test in conjuncts.txtar removes a spurious element and is now correct. The tests for 3528 were already fixed by an earlier CL. We will leave the closing here, though, to signal these changes are related. Fixes #3528 Signed-off-by: Marcel van Lohuizen Change-Id: Ieaeba4b758006592b115240dac2e29b967103c5a Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1206178 Unity-Result: CUE porcuepine Reviewed-by: Matthew Sackman TryBot-Result: CUEcueckoo --- cue/testdata/eval/conjuncts.txtar | 28 ++++++++++++++-------------- internal/core/adt/disjunct2.go | 2 ++ internal/core/adt/eval_test.go | 2 +- 3 files changed, 17 insertions(+), 15 deletions(-) diff --git a/cue/testdata/eval/conjuncts.txtar b/cue/testdata/eval/conjuncts.txtar index f98f2024a..ecaec5ba7 100644 --- a/cue/testdata/eval/conjuncts.txtar +++ b/cue/testdata/eval/conjuncts.txtar @@ -69,15 +69,15 @@ issue2355: { } -- out/evalalpha/stats -- -Leaks: 187 -Freed: 25 -Reused: 25 -Allocs: 187 +Leaks: 216 +Freed: 30 +Reused: 30 +Allocs: 216 Retain: 0 Unifications: 43 -Conjuncts: 350 -Disjuncts: 124 +Conjuncts: 378 +Disjuncts: 152 -- out/evalalpha -- Errors: param: conflicting values "foo" and [{}] (mismatched types string and list): @@ -102,7 +102,7 @@ Result: string #early: (string){ |(*(string){ "X" }, (string){ string }) } }) } - t3: (string){ |(*(string){ "X" }, (string){ + t3: (string){ |(*(string){ "X" #early: (string){ |(*(string){ "X" }, (string){ string }) } }, (string){ @@ -163,18 +163,18 @@ diff old new -Reused: 59 -Allocs: 18 -Retain: 22 -+Leaks: 187 -+Freed: 25 -+Reused: 25 -+Allocs: 187 ++Leaks: 216 ++Freed: 30 ++Reused: 30 ++Allocs: 216 +Retain: 0 -Unifications: 45 -Conjuncts: 135 -Disjuncts: 86 +Unifications: 43 -+Conjuncts: 350 -+Disjuncts: 124 ++Conjuncts: 378 ++Disjuncts: 152 -- diff/-out/evalalpha<==>+out/eval -- diff old new --- old @@ -200,7 +200,7 @@ diff old new - // ./in.cue:15:2 - #early: (string){ |(*(string){ "X" }, (string){ string }) } - } -+ t3: (string){ |(*(string){ "X" }, (string){ ++ t3: (string){ |(*(string){ + "X" + #early: (string){ |(*(string){ "X" }, (string){ string }) } + }, (string){ diff --git a/internal/core/adt/disjunct2.go b/internal/core/adt/disjunct2.go index d6eb439b2..17071b29a 100644 --- a/internal/core/adt/disjunct2.go +++ b/internal/core/adt/disjunct2.go @@ -451,6 +451,8 @@ func (n *nodeContext) doDisjunct(c Conjunct, m defaultMode, mode runMode) (*node return nil, err } + d = d.node.DerefDisjunct().state + return d, nil } diff --git a/internal/core/adt/eval_test.go b/internal/core/adt/eval_test.go index a880d8ede..291eb6b38 100644 --- a/internal/core/adt/eval_test.go +++ b/internal/core/adt/eval_test.go @@ -88,7 +88,7 @@ var skipDebugDepErrors = map[string]int{ "disjunctions/edge": 1, "disjunctions/elimination": 11, "disjunctions/embed": 6, - "eval/conjuncts": 4, + "eval/conjuncts": 3, "eval/notify": 17, "fulleval/054_issue312": 1, "scalars/embed": 1,