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 16, 2024
1 parent c275642 commit 897de07
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 18 deletions.
16 changes: 8 additions & 8 deletions cue/testdata/eval/conjuncts.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ issue2355: {
}

-- out/evalalpha/stats --
Leaks: 228
Freed: 40
Reused: 40
Allocs: 228
Leaks: 236
Freed: 38
Reused: 38
Allocs: 236
Retain: 0

Unifications: 46
Expand Down Expand Up @@ -163,10 +163,10 @@ diff old new
-Reused: 59
-Allocs: 18
-Retain: 22
+Leaks: 228
+Freed: 40
+Reused: 40
+Allocs: 228
+Leaks: 236
+Freed: 38
+Reused: 38
+Allocs: 236
+Retain: 0

-Unifications: 45
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 897de07

Please sign in to comment.