Skip to content

Commit

Permalink
internal/core/adt: remove GroupUnify mechanism
Browse files Browse the repository at this point in the history
This mechanism was originally added to deal with
closedness issues around Unify. These have now
been addressed differently with various fixes,
allowing this mechanism to be removed.

Issue #3706

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: Ic5a2358351e0c3126cbbf59eab6246f30ab49f1a
Reviewed-on: https://cue.gerrithub.io/c/cue-lang/cue/+/1208588
TryBot-Result: CUEcueckoo <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>
  • Loading branch information
mpvl committed Feb 11, 2025
1 parent 455b96b commit ab1e2d9
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 11 deletions.
4 changes: 2 additions & 2 deletions cue/testdata/cycle/builtins.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ Allocs: 351
Retain: 0

Unifications: 310
Conjuncts: 1273
Conjuncts: 1216
Disjuncts: 28
-- diff/-out/evalalpha/stats<==>+out/eval/stats --
diff old new
Expand All @@ -238,7 +238,7 @@ diff old new
-Conjuncts: 625
-Disjuncts: 441
+Unifications: 310
+Conjuncts: 1273
+Conjuncts: 1216
+Disjuncts: 28
-- out/eval/stats --
Leaks: 23
Expand Down
4 changes: 2 additions & 2 deletions cue/testdata/cycle/inline_non_recursive.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ Allocs: 228
Retain: 0

Unifications: 183
Conjuncts: 828
Conjuncts: 827
Disjuncts: 0
-- out/evalalpha --
Errors:
Expand Down Expand Up @@ -252,7 +252,7 @@ diff old new
-Conjuncts: 2813
-Disjuncts: 1485
+Unifications: 183
+Conjuncts: 828
+Conjuncts: 827
+Disjuncts: 0
-- diff/-out/evalalpha<==>+out/eval --
diff old new
Expand Down
5 changes: 0 additions & 5 deletions internal/core/adt/closed.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,6 @@ type CloseInfo struct {
// NOTE: only used when using closeContext.
FromDef bool

// GroupUnify indicates that this conjunct needs to spawn its own
// closeContext. This is necessary when programmatically combining
// top-level values, such as with Value.Unify.
GroupUnify bool

// FieldTypes indicates which kinds of fields (optional, dynamic, patterns,
// etc.) are contained in this conjunct.
FieldTypes OptionalType
Expand Down
1 change: 0 additions & 1 deletion internal/core/adt/composite.go
Original file line number Diff line number Diff line change
Expand Up @@ -992,7 +992,6 @@ func Unify(c *OpContext, a, b Value) *Vertex {
func addConjuncts(ctx *OpContext, dst *Vertex, src Value) {
closeInfo := ctx.CloseInfo()
c := MakeConjunct(nil, src, closeInfo)
c.CloseInfo.GroupUnify = true

if v, ok := src.(*Vertex); ok && v.ClosedRecursive {
if ctx.Version == internal.EvalV2 {
Expand Down
3 changes: 2 additions & 1 deletion internal/core/adt/conjunct.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,10 @@ func (n *nodeContext) scheduleConjunct(c Conjunct, id CloseInfo) {
id.FromEmbed = false
}
}
if t != 0 || c.CloseInfo.GroupUnify {
if t != 0 {
id, _ = id.spawnCloseContext(n.ctx, t)
}

if !id.cc.done {
id.cc.incDependent(n.ctx, DEFER, nil)
defer id.cc.decDependent(n.ctx, DEFER, nil)
Expand Down

0 comments on commit ab1e2d9

Please sign in to comment.