Skip to content

Commit 196dc98

Browse files
committed
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]>
1 parent 4967eb4 commit 196dc98

File tree

2 files changed

+2
-16
lines changed

2 files changed

+2
-16
lines changed

internal/core/adt/eval_test.go

-1
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,6 @@ var skipDebugDepErrors = map[string]int{
9090
"disjunctions/errors": 2,
9191
"eval/conjuncts": 3,
9292
"eval/issue2146": 4,
93-
"eval/issue3301": 1,
9493
"eval/issue599": 1,
9594
"export/031": 1,
9695
"fulleval/054_issue312": 1,

internal/core/adt/expr.go

+2-15
Original file line numberDiff line numberDiff line change
@@ -988,16 +988,7 @@ func (x *SelectorExpr) Source() ast.Node {
988988
}
989989

990990
func (x *SelectorExpr) resolve(c *OpContext, state combinedFlags) *Vertex {
991-
// TODO: the node should really be evaluated as AllConjunctsDone, but the
992-
// order of evaluation is slightly off, causing too much to be evaluated.
993-
// This may especially result in incorrect results when using embedded
994-
// scalars.
995-
// In the new evaluator, evaluation of the node is done in lookup.
996-
// TODO:
997-
// - attempt: if we ensure that errors are propagated in pending arcs.
998-
// - require: if we want to ensure that all arcs
999-
// are known now.
1000-
n := c.node(x, x.X, x.Sel.IsRegular(), attempt(partial, needFieldSetKnown))
991+
n := c.node(x, x.X, x.Sel.IsRegular(), require(partial, needFieldSetKnown))
1001992
if n == emptyNode {
1002993
return n
1003994
}
@@ -1033,11 +1024,7 @@ func (x *IndexExpr) Source() ast.Node {
10331024

10341025
func (x *IndexExpr) resolve(ctx *OpContext, state combinedFlags) *Vertex {
10351026
// TODO: support byte index.
1036-
// TODO: the node should really be evaluated as AllConjunctsDone, but the
1037-
// order of evaluation is slightly off, causing too much to be evaluated.
1038-
// This may especially result in incorrect results when using embedded
1039-
// scalars.
1040-
n := ctx.node(x, x.X, true, attempt(partial, needFieldSetKnown))
1027+
n := ctx.node(x, x.X, true, require(partial, needFieldSetKnown))
10411028
i := ctx.value(x.Index, require(partial, scalarKnown))
10421029
if n == emptyNode {
10431030
return n

0 commit comments

Comments
 (0)