internal/core/adt: avoid duplicate insertion of conjuncts

The current code could result in adding conjuncts both dynamically and through insertConjuncts. Signed-off-by: Marcel van Lohuizen <[email protected]> Change-Id: I86b1bf37a2dc677f71e1a2f3b54d057d196b689f Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/546855 Unity-Result: CUEcueckoo <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Reviewed-by: Paul Jolly <[email protected]>
cue-lang · Nov 30, 2022 · d341814 · d341814
1 parent a68c6ce
commit d341814
Show file tree

Hide file tree

Showing 6 changed files with 20 additions and 31 deletions.
diff --git a/cue/testdata/comprehensions/pushdown.txtar b/cue/testdata/comprehensions/pushdown.txtar
@@ -709,7 +709,7 @@ Allocs: 23
 Retain: 70
 
 Unifications: 389
-Conjuncts:    644
+Conjuncts:    632
 Disjuncts:    433
 -- out/eval --
 Errors:

diff --git a/cue/testdata/cycle/chain.txtar b/cue/testdata/cycle/chain.txtar
@@ -209,14 +209,14 @@ issue2052: full: {
 }
 -- out/eval/stats --
 Leaks:  17
-Freed:  39364
-Reused: 39317
+Freed:  20940
+Reused: 20893
 Allocs: 64
 Retain: 143
 
-Unifications: 12118
-Conjuncts:    67085
-Disjuncts:    39452
+Unifications: 7522
+Conjuncts:    34391
+Disjuncts:    21028
 -- out/eval --
 (struct){
   chain: (struct){

diff --git a/cue/testdata/cycle/comprehension.txtar b/cue/testdata/cycle/comprehension.txtar
@@ -311,7 +311,7 @@ Allocs: 60
 Retain: 127
 
 Unifications: 828
-Conjuncts:    2532
+Conjuncts:    2523
 Disjuncts:    1377
 -- out/eval --
 Errors:

diff --git a/cue/testdata/cycle/self.txtar b/cue/testdata/cycle/self.txtar
@@ -270,7 +270,7 @@ Allocs: 23
 Retain: 33
 
 Unifications: 236
-Conjuncts:    700
+Conjuncts:    687
 Disjuncts:    435
 -- out/eval --
 Errors:
@@ -291,15 +291,6 @@ list.error2.a: conflicting values "3" and "2":
     ./in.cue:15:6
     ./in.cue:16:11
 list.error2.a: element at index 2 not allowed by earlier comprehension or reference cycle
-list.error2.a.0: conflicting values "3" and "1":
-    ./in.cue:15:6
-    ./in.cue:16:6
-list.error2.a.1: conflicting values "1" and "2":
-    ./in.cue:16:6
-    ./in.cue:16:11
-list.error2.a.1: conflicting values "3" and "2":
-    ./in.cue:15:6
-    ./in.cue:16:11
 list.error2.a.2: conflicting values "1" and "2":
     ./in.cue:16:6
     ./in.cue:16:11
@@ -348,9 +339,6 @@ Result:
           // [eval] list.error2.a: conflicting values "3" and "1":
           //     ./in.cue:15:6
           //     ./in.cue:16:6
-          // list.error2.a.0: conflicting values "3" and "1":
-          //     ./in.cue:15:6
-          //     ./in.cue:16:6
         }
         1: (_|_){
           // [eval] list.error2.a: conflicting values "1" and "2":
@@ -359,12 +347,6 @@ Result:
           // list.error2.a: conflicting values "3" and "2":
           //     ./in.cue:15:6
           //     ./in.cue:16:11
-          // list.error2.a.1: conflicting values "1" and "2":
-          //     ./in.cue:16:6
-          //     ./in.cue:16:11
-          // list.error2.a.1: conflicting values "3" and "2":
-          //     ./in.cue:15:6
-          //     ./in.cue:16:11
         }
         2: (_|_){
           // [eval] list.error2.a.2: conflicting values "1" and "2":

diff --git a/internal/core/adt/composite.go b/internal/core/adt/composite.go
@@ -757,11 +757,13 @@ func (v *Vertex) AddConjunct(c Conjunct) *Bottom {
 		// This is likely a bug in the evaluator and should not happen.
 		return &Bottom{Err: errors.Newf(token.NoPos, "cannot add conjunct")}
 	}
-	v.addConjunct(c)
+	if !v.hasConjunct(c) {
+		v.addConjunctUnchecked(c)
+	}
 	return nil
 }
 
-func (v *Vertex) addConjunct(c Conjunct) {
+func (v *Vertex) hasConjunct(c Conjunct) (added bool) {
 	switch c.x.(type) {
 	case *OptionalField, *BulkOptionalField, *Ellipsis:
 	default:
@@ -772,10 +774,10 @@ func (v *Vertex) addConjunct(c Conjunct) {
 		if x.CloseInfo.closeInfo == c.CloseInfo.closeInfo &&
 			x.x == c.x &&
 			x.Env.Up == c.Env.Up && x.Env.Vertex == c.Env.Vertex {
-			return
+			return true
 		}
 	}
-	v.addConjunctUnchecked(c)
+	return false
 }
 
 func (v *Vertex) addConjunctUnchecked(c Conjunct) {

diff --git a/internal/core/adt/eval.go b/internal/core/adt/eval.go
@@ -1964,13 +1964,18 @@ func (n *nodeContext) insertField(f Feature, x Conjunct) *Vertex {
 		arc.MultiLet = true
 		return arc
 	}
-	arc.addConjunct(x)
+	if arc.hasConjunct(x) {
+		return arc
+	}
 
 	switch {
 	case arc.state != nil:
+		arc.Conjuncts = append(arc.Conjuncts, x)
 		arc.state.addExprConjunct(x)
 
 	case arc.Status() == 0:
+		arc.addConjunctUnchecked(x)
+
 	default:
 		n.addBottom(&Bottom{
 			Code: IncompleteError,