Skip to content

Commit

Permalink
internal/core/adt: fix processing of additional disjunctions
Browse files Browse the repository at this point in the history
Sometimes evaluating disjunctions is triggered before all
conjuncts have been added. If then another disjunction was
added, these results were not properly merged with previously
computed disjunctions. This change fixes that.

Issue #2851
Issue #2884

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: I248b9ecc4694b2cb9a1e478343d9a8d233e14929
  • Loading branch information
mpvl committed Apr 17, 2024
1 parent 15c15df commit 4b96b4b
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 22 deletions.
24 changes: 12 additions & 12 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: 234
Freed: 42
Reused: 42
Allocs: 234
Leaks: 236
Freed: 38
Reused: 38
Allocs: 236
Retain: 0

Unifications: 46
Conjuncts: 467
Disjuncts: 184
Conjuncts: 463
Disjuncts: 180
-- out/evalalpha --
Errors:
issue2351.let.param: conflicting values [{}] and "foo" (mismatched types list and string):
Expand Down Expand Up @@ -163,18 +163,18 @@ diff old new
-Reused: 59
-Allocs: 18
-Retain: 22
+Leaks: 234
+Freed: 42
+Reused: 42
+Allocs: 234
+Leaks: 236
+Freed: 38
+Reused: 38
+Allocs: 236
+Retain: 0

-Unifications: 45
-Conjuncts: 135
-Disjuncts: 86
+Unifications: 46
+Conjuncts: 467
+Disjuncts: 184
+Conjuncts: 463
+Disjuncts: 180
-- diff/-out/evalalpha<==>+out/eval --
diff old new
--- old
Expand Down
25 changes: 15 additions & 10 deletions internal/core/adt/disjunct2.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,19 +311,23 @@ func (n *nodeContext) processDisjunctions() *Bottom {
cross, results = results, cross[:0]
}

switch len(cross) {
// Note it is under some circumstances possible that the disjunctions of a
// node are evaluated before all conjuncts are known. This could cause this
// method to be called more than once for a single node. In this case, we
// must properly merge disjuncts with existing disjunctions.
for _, x := range cross {
n.disjuncts = appendDisjunct(n.ctx, n.disjuncts, x)
}

switch a := n.disjuncts; len(a) {
case 0:
panic("unreachable: empty disjunction already handled above")

case 1:
d := cross[0].node
d := a[0].node
n.node.BaseValue = d
n.defaultMode = cross[0].defaultMode

default:
// append, rather than assign, to allow reusing the memory of
// a pre-existing slice.
n.disjuncts = append(n.disjuncts, cross...)
n.defaultMode = a[0].defaultMode
n.disjuncts = n.disjuncts[:0]
}

return nil
Expand All @@ -333,8 +337,6 @@ func (n *nodeContext) processDisjunctions() *Bottom {
// with an existing set of results.
func (n *nodeContext) crossProduct(dst, cross []*nodeContext, dn *envDisjunct, mode runMode) []*nodeContext {
for _, p := range cross {
// TODO: use a partial unify instead
// p.completeNodeConjuncts()
initArcs(n.ctx, p.node)

for j, d := range dn.disjuncts {
Expand Down Expand Up @@ -382,6 +384,9 @@ func (n *nodeContext) doDisjunct(c Conjunct, m defaultMode, mode runMode) (*node
}
n.ctx.stats.Disjuncts++

// Be sure we use the dereferenced node. This was probably already done.
n = n.node.Indirect().state

oc := newOverlayContext(n.ctx)

var ccHole *closeContext
Expand Down

0 comments on commit 4b96b4b

Please sign in to comment.