internal/core/adt: require nodes for indexing

In general, it is desirable to evaluate things
eagerly when we can.

This is one of these cases. It will help improve
a followup CL that fixes a bug for optional
references. This also fixes a reference
count issue.

We include this as a separate CL to be able to
bisect any issues that may arise from this
change.

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

internal/core/adt/eval_test.go

@@ -90,7 +90,6 @@ var skipDebugDepErrors = map[string]int{
 	"disjunctions/errors":      2,
 	"eval/conjuncts":           3,
 	"eval/issue2146":           4,
-	"eval/issue3301":           1,
 	"eval/issue599":            1,
 	"export/031":               1,
 	"fulleval/054_issue312":    1,

internal/core/adt/expr.go

@@ -988,16 +988,7 @@ func (x *SelectorExpr) Source() ast.Node {
 }
 
 func (x *SelectorExpr) resolve(c *OpContext, state combinedFlags) *Vertex {
-	// TODO: the node should really be evaluated as AllConjunctsDone, but the
-	// order of evaluation is slightly off, causing too much to be evaluated.
-	// This may especially result in incorrect results when using embedded
-	// scalars.
-	// In the new evaluator, evaluation of the node is done in lookup.
-	// TODO:
-	// - attempt: if we ensure that errors are propagated in pending arcs.
-	// - require: if we want to ensure that all arcs
-	//  are known now.
-	n := c.node(x, x.X, x.Sel.IsRegular(), attempt(partial, needFieldSetKnown))
+	n := c.node(x, x.X, x.Sel.IsRegular(), require(partial, needFieldSetKnown))
 	if n == emptyNode {
 		return n
 	}
@@ -1033,11 +1024,7 @@ func (x *IndexExpr) Source() ast.Node {
 
 func (x *IndexExpr) resolve(ctx *OpContext, state combinedFlags) *Vertex {
 	// TODO: support byte index.
-	// TODO: the node should really be evaluated as AllConjunctsDone, but the
-	// order of evaluation is slightly off, causing too much to be evaluated.
-	// This may especially result in incorrect results when using embedded
-	// scalars.
-	n := ctx.node(x, x.X, true, attempt(partial, needFieldSetKnown))
+	n := ctx.node(x, x.X, true, require(partial, needFieldSetKnown))
 	i := ctx.value(x.Index, require(partial, scalarKnown))
 	if n == emptyNode {
 		return n