Skip to content

Commit

Permalink
internal/core/adt: dereference disjunct result
Browse files Browse the repository at this point in the history
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 <[email protected]>
Change-Id: Ieaeba4b758006592b115240dac2e29b967103c5a
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1206178
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Matthew Sackman <[email protected]>
TryBot-Result: CUEcueckoo <[email protected]>
  • Loading branch information
mpvl committed Dec 20, 2024
1 parent 8f55942 commit e4c4b8e
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 15 deletions.
28 changes: 14 additions & 14 deletions cue/testdata/eval/conjuncts.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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){
Expand Down Expand Up @@ -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
Expand All @@ -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){
Expand Down
2 changes: 2 additions & 0 deletions internal/core/adt/disjunct2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion internal/core/adt/eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit e4c4b8e

Please sign in to comment.