internal/core/adt: fix attribute processing

The use of the Vertex.Structs fields has been almost
completely eliminated in the new evaluator. It is,
however, still used for some things, like keeping
track of attributes.

This change fixes the bookkeeping of StructLits. It
changes the signature of Init to allow it to determine
if a full init is necessary. This also gives us some
compiler support to check if all call sites in the
old evaluator have an equivalent in the new one.

Note that one use of Init was missing in the new
evaluator. The case used in list processing seems
unnecessary in the new evaluator.

Issue #3060

Signed-off-by: Marcel van Lohuizen <[email protected]>
Change-Id: I2eb3319f0f3ad26c3d64adcff819016a24ec6e7d
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1194658
TryBot-Result: CUEcueckoo <[email protected]>
Reviewed-by: Daniel Martí <[email protected]>
Unity-Result: CUE porcuepine <[email protected]>

Author: mpvl

cue/attribute_test.go

@@ -96,8 +96,6 @@ func TestAttributes(t *testing.T) {
 	}}
 	for _, tc := range testCases {
 		cuetdtest.FullMatrix.Run(t, tc.path, func(t *cuetdtest.M) {
-			t.TODO_V3()
-
 			v := getValue(t, config).LookupPath(ParsePath(tc.path))
 			a := v.Attributes(tc.flags)
 			got := fmt.Sprint(a)

internal/core/adt/closed_test.go

@@ -41,7 +41,7 @@ func TestClosedness(t *testing.T) {
 			t.Fatal(err)
 		}
 		st := expr.Elem().(*adt.StructLit)
-		st.Init()
+		st.Init(ctx)
 
 		return &adt.StructInfo{
 			StructLit: st,

internal/core/adt/comprehension.go

@@ -244,7 +244,7 @@ func (n *nodeContext) insertComprehension(
 			case !ec.done:
 				ec.structs = append(ec.structs, st)
 			case len(ec.envs) > 0:
-				st.Init()
+				st.Init(n.ctx)
 			}
 		}
 
@@ -453,7 +453,7 @@ func (n *nodeContext) processComprehensionInner(d *envYield, state vertexStatus)
 
 		if len(d.envs) > 0 {
 			for _, s := range d.structs {
-				s.Init()
+				s.Init(n.ctx)
 			}
 		}
 		d.structs = nil

internal/core/adt/conjunct.go

@@ -243,8 +243,10 @@ func (n *nodeContext) scheduleStruct(env *Environment,
 	hasEmbed := false
 	hasEllipsis := false
 
+	// TODO: do we still need this?
 	// shouldClose := ci.cc.isDef || ci.cc.isClosedOnce
-	// s.Init()
+
+	s.Init(n.ctx)
 
 	// TODO: do we still need to AddStruct and do we still need to Disable?
 	parent := n.node.AddStruct(s, childEnv, ci)

internal/core/adt/eval.go

@@ -2050,7 +2050,7 @@ func (n *nodeContext) addStruct(
 		Vertex: n.node,
 	}
 
-	s.Init()
+	s.Init(n.ctx)
 
 	if s.HasEmbed && !s.IsFile() {
 		closeInfo = closeInfo.SpawnGroup(nil)
@@ -2441,7 +2441,7 @@ outer:
 		s := l.list.info
 		if s == nil {
 			s = &StructLit{Decls: []Decl{l.elipsis}}
-			s.Init()
+			s.Init(n.ctx)
 			l.list.info = s
 		}
 		info := n.node.AddStruct(s, l.env, l.id)

internal/core/adt/expr.go

@@ -97,11 +97,16 @@ func (o *StructLit) MarkField(f Feature) {
 	o.Fields = append(o.Fields, FieldInfo{Label: f})
 }
 
-func (o *StructLit) Init() {
+func (o *StructLit) Init(ctx *OpContext) {
 	if o.initialized {
 		return
 	}
 	o.initialized = true
+
+	if ctx.isDevVersion() {
+		return
+	}
+
 	for _, d := range o.Decls {
 		switch x := d.(type) {
 		case *Field:

internal/core/adt/unify.go

@@ -562,6 +562,20 @@ func (n *nodeContext) completeAllArcs(needs condition, mode runMode) bool {
 	}
 	n.node.Arcs = n.node.Arcs[:k]
 
+	// Strip struct literals that were not initialized and are not part
+	// of the output.
+	//
+	// TODO(perf): we could keep track if any such structs exist and only
+	// do this removal if there is a change of shrinking the list.
+	k = 0
+	for _, s := range n.node.Structs {
+		if s.initialized {
+			n.node.Structs[k] = s
+			k++
+		}
+	}
+	n.node.Structs = n.node.Structs[:k]
+
 	// TODO: This seems to be necessary, but enables structural cycles.
 	// Evaluator whether we still need this.
 	//

internal/core/export/testdata/main/attrs.txtar

@@ -369,7 +369,6 @@ dynamicSimple: {
 	comprehensions: {
 		@step(0)
 		@step(2c)
-		@step(4c)
 		c1: {} @step(1)
 		c2: {
 			@step(2a)
@@ -435,7 +434,6 @@ dynamicSimple: {
 	comprehensions: {
 		@step(0)
 		@step(2c)
-		@step(4c)
 		c1: {} @step(1)
 		c2: {
 			@step(2a)
@@ -456,7 +454,7 @@ dynamicSimple: {
 diff old new
 --- old
 +++ new
-@@ -150,19 +150,20 @@
+@@ -150,11 +150,11 @@
  			@decl1()
  		}
  		b: {
@@ -465,32 +463,15 @@ diff old new
 -		}
 -		c: {
 -			@decl1(), @decl2()
--		}
--		d: {
--			@decl2(), @decl1()
--		}
--	}
--	comprehensions: {
--		@step(0)
--		@step(2c)
 +			@decl2(), @decl1()
 +			x: 1
 +		}
 +		c: {
 +			@decl2(), @decl1()
-+		}
-+		d: {
-+			@decl2(), @decl1()
-+		}
-+	}
-+	comprehensions: {
-+		@step(0)
-+		@step(2c)
-+		@step(4c)
- 		c1: {} @step(1)
- 		c2: {
- 			@step(2a)
-@@ -215,19 +216,20 @@
+ 		}
+ 		d: {
+ 			@decl2(), @decl1()
+@@ -215,11 +215,11 @@
  			@decl1()
  		}
  		b: {
@@ -499,33 +480,14 @@ diff old new
 -		}
 -		c: {
 -			@decl1(), @decl2()
--		}
--		d: {
--			@decl2(), @decl1()
--		}
--	}
--	comprehensions: {
--		@step(0)
--		@step(2c)
 +			@decl2(), @decl1()
 +			x: 1
 +		}
 +		c: {
 +			@decl2(), @decl1()
-+		}
-+		d: {
-+			@decl2(), @decl1()
-+		}
-+	}
-+	comprehensions: {
-+		@step(0)
-+		@step(2c)
-+		@step(4c)
- 		c1: {} @step(1)
- 		c2: {
- 			@step(2a)
--- diff/value/todo/p1 --
-comprehension: @step(4c) added in comprehensions
+ 		}
+ 		d: {
+ 			@decl2(), @decl1()
 -- diff/value/todo/p3 --
 Reordering of attributes.
 -- out/value --

internal/core/export/value_test.go

@@ -39,9 +39,10 @@ var exclude = map[string]string{
 func TestValue(t *testing.T) {
 	const debugValue = `
 -- in.cue --
-// Issue #2305
-g: #d
-#d: 1
+if false {
+	c4: { @step(4a) } @step(4b)
+	@step(4c)
+}
 `
 
 	test := cuetxtar.TxTarTest{