From 196dc98bf2573e48b0048467bc32d867145b48b8 Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Fri, 23 Aug 2024 09:36:49 +0200 Subject: [PATCH] internal/core/adt: require nodes for indexing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Change-Id: I76f406cde7f31be8ec9207a877eaff8ba4a98b02 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1199916 TryBot-Result: CUEcueckoo Reviewed-by: Matthew Sackman Reviewed-by: Daniel Martí Unity-Result: CUE porcuepine --- internal/core/adt/eval_test.go | 1 - internal/core/adt/expr.go | 17 ++--------------- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/internal/core/adt/eval_test.go b/internal/core/adt/eval_test.go index 109e4dcc5c7..25d49bf9a88 100644 --- a/internal/core/adt/eval_test.go +++ b/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, diff --git a/internal/core/adt/expr.go b/internal/core/adt/expr.go index f0bfb3bbe44..eb1d9fc72d9 100644 --- a/internal/core/adt/expr.go +++ b/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