From e40fe1542d27a70750be4e0be54d0817d51eed81 Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Fri, 6 Sep 2024 12:41:56 +0200 Subject: [PATCH] internal/core/adt: limit error propagation to cross package This check mostly necessary to catch cross-package errors. Within packages it may result in unintuitive error messages. This is because the error status of a node may change during evaluation. Note that package nodes are self-contained and that because of a lack of cycles, a package node is always fully evaluated before a referring node. In order to determine whether an error crosses package boundaries, we mark the originating node in the error. An error orginates from a different package if they root node is different. There are probably less error-prone mechanisms to determine the package, but the impact of getting it wrong is quite small, so this is sufficient for now. Signed-off-by: Marcel van Lohuizen Change-Id: I82c5e199a1bc0746abb60b825c31e03587061f39 Dispatch-Trailer: {"type":"trybot","CL":1200793,"patchset":5,"ref":"refs/changes/93/1200793/5","targetBranch":"master"} --- cue/testdata/comprehensions/iferror.txtar | 13 ++--- cue/testdata/cycle/023_reentrance.txtar | 26 ++++------ cue/testdata/cycle/chain.txtar | 28 +++++------ cue/testdata/cycle/compbottomnofinal.txtar | 48 +++---------------- cue/testdata/eval/issue2550.txtar | 29 ++++------- internal/core/adt/binop.go | 2 + internal/core/adt/composite.go | 5 +- internal/core/adt/constraints.go | 3 +- internal/core/adt/context.go | 22 +++++++-- internal/core/adt/cycle.go | 1 + internal/core/adt/disjunct.go | 1 + internal/core/adt/errors.go | 8 ++++ internal/core/adt/eval.go | 13 ++++- internal/core/adt/expr.go | 31 ++++++++++-- internal/core/adt/optional.go | 3 +- internal/core/adt/tasks.go | 1 + internal/core/adt/unify.go | 17 ++++++- internal/core/compile/builtin.go | 3 +- internal/core/convert/go.go | 3 +- .../core/export/testdata/main/dynamic.txtar | 12 ++--- 20 files changed, 141 insertions(+), 128 deletions(-) diff --git a/cue/testdata/comprehensions/iferror.txtar b/cue/testdata/comprehensions/iferror.txtar index cab203321..d240e8b94 100644 --- a/cue/testdata/comprehensions/iferror.txtar +++ b/cue/testdata/comprehensions/iferror.txtar @@ -190,6 +190,8 @@ Result: incomplete: (_|_){ // [incomplete] incomplete: undefined field: d: // ./in.cue:16:7 + // incomplete: undefined field: d: + // ./in.cue:23:21 list: (#list){ 0: (int){ 1 } 1: (int){ 2 } @@ -268,16 +270,7 @@ diff old new Result: (_|_){ -@@ -31,8 +31,6 @@ - incomplete: (_|_){ - // [incomplete] incomplete: undefined field: d: - // ./in.cue:16:7 -- // incomplete: undefined field: d: -- // ./in.cue:23:21 - list: (#list){ - 0: (int){ 1 } - 1: (int){ 2 } -@@ -66,17 +64,23 @@ +@@ -66,17 +66,23 @@ issue1972: (_|_){ // [eval] err1: (_|_){ diff --git a/cue/testdata/cycle/023_reentrance.txtar b/cue/testdata/cycle/023_reentrance.txtar index 8390fe294..22ed99b65 100644 --- a/cue/testdata/cycle/023_reentrance.txtar +++ b/cue/testdata/cycle/023_reentrance.txtar @@ -87,12 +87,19 @@ Result: // ./in.cue:7:5 // ./in.cue:3:35 // ./in.cue:5:5 + // non-concrete value int in operand to <: + // ./in.cue:10:5 + // ./in.cue:3:35 + // ./in.cue:5:5 } } fib: (_|_){ // [incomplete] fib: non-concrete value int in operand to >=: // ./in.cue:7:5 // ./in.cue:5:5 + // fib: non-concrete value int in operand to <: + // ./in.cue:10:5 + // ./in.cue:5:5 n: (int){ int } } fib1: (int){ 1 } @@ -150,24 +157,7 @@ diff old new Result: (_|_){ -@@ -16,10 +12,6 @@ - // ./in.cue:7:5 - // ./in.cue:3:35 - // ./in.cue:5:5 -- // non-concrete value int in operand to <: -- // ./in.cue:10:5 -- // ./in.cue:3:35 -- // ./in.cue:5:5 - } - } - fib: (_|_){ -@@ -26,33 +18,23 @@ - // [incomplete] fib: non-concrete value int in operand to >=: - // ./in.cue:7:5 - // ./in.cue:5:5 -- // fib: non-concrete value int in operand to <: -- // ./in.cue:10:5 -- // ./in.cue:5:5 +@@ -32,27 +28,20 @@ n: (int){ int } } fib1: (int){ 1 } diff --git a/cue/testdata/cycle/chain.txtar b/cue/testdata/cycle/chain.txtar index ff4db31a5..1656bb121 100644 --- a/cue/testdata/cycle/chain.txtar +++ b/cue/testdata/cycle/chain.txtar @@ -208,15 +208,15 @@ issue2052: full: { d: #Depth & {#in: tree} } -- out/evalalpha/stats -- -Leaks: 19999 -Freed: 1462 -Reused: 1461 -Allocs: 20000 +Leaks: 20362 +Freed: 1534 +Reused: 1533 +Allocs: 20363 Retain: 0 -Unifications: 7506 -Conjuncts: 107540 -Disjuncts: 13655 +Unifications: 7655 +Conjuncts: 109511 +Disjuncts: 13941 -- out/evalalpha -- Errors: issue2052.t1.#Depth: adding field #basic not allowed as field set was already referenced: @@ -725,18 +725,18 @@ diff old new -Reused: 1816 -Allocs: 70 -Retain: 169 -+Leaks: 19999 -+Freed: 1462 -+Reused: 1461 -+Allocs: 20000 ++Leaks: 20362 ++Freed: 1534 ++Reused: 1533 ++Allocs: 20363 +Retain: 0 -Unifications: 801 -Conjuncts: 3177 -Disjuncts: 1979 -+Unifications: 7506 -+Conjuncts: 107540 -+Disjuncts: 13655 ++Unifications: 7655 ++Conjuncts: 109511 ++Disjuncts: 13941 -- diff/-out/evalalpha<==>+out/eval -- diff old new --- old diff --git a/cue/testdata/cycle/compbottomnofinal.txtar b/cue/testdata/cycle/compbottomnofinal.txtar index d2f70a6cf..e6cd4db7c 100644 --- a/cue/testdata/cycle/compbottomnofinal.txtar +++ b/cue/testdata/cycle/compbottomnofinal.txtar @@ -384,8 +384,6 @@ Disjuncts: 215 #Y: (_|_){ // [incomplete] small.p1.#Y: undefined field: port: // ./in.cue:28:50 - // small.p1.#X.port: cyclic reference to field port: - // ./in.cue:31:4 } #X: (_|_){ // [incomplete] small.p1.#X.port: cyclic reference to field port: @@ -409,8 +407,6 @@ Disjuncts: 215 #Y: (_|_){ // [incomplete] medium.p1.#Y: undefined field: port: // ./in.cue:58:50 - // medium.p1.#X.port: cyclic reference to field port: - // ./in.cue:70:4 } Y: (struct){ } @@ -423,8 +419,6 @@ Disjuncts: 215 #Y: (_|_){ // [incomplete] medium.p2.#Y: undefined field: port: // ./in.cue:80:50 - // medium.p2.#X.port: cyclic reference to field port: - // ./in.cue:86:4 } #X: (_|_){ // [incomplete] medium.p2.#X.port: cyclic reference to field port: @@ -439,8 +433,6 @@ Disjuncts: 215 #Y: (_|_){ // [incomplete] medium.p3.#Y: undefined field: port: // ./in.cue:108:50 - // medium.p3.#X.port: cyclic reference to field port: - // ./in.cue:114:4 } #X: (_|_){ // [incomplete] medium.p3.#X.port: cyclic reference to field port: @@ -455,9 +447,7 @@ Disjuncts: 215 // ./in.cue:134:4 } #Y: (_|_){ - // [incomplete] medium.p4.#X.port: cyclic reference to field port: - // ./in.cue:134:4 - // medium.p4.#Y: undefined field: port: + // [incomplete] medium.p4.#Y: undefined field: port: // ./in.cue:142:50 } } @@ -496,8 +486,6 @@ Disjuncts: 215 X: (_|_){ // [incomplete] large.p1.X: undefined field: port: // ./in.cue:199:33 - // large.p1.#X.port: cyclic reference to field port: - // ./in.cue:211:4 } #X: (_|_){ // [incomplete] large.p1.#X.port: cyclic reference to field port: @@ -514,8 +502,6 @@ Disjuncts: 215 X: (_|_){ // [incomplete] large.p2.X: undefined field: port: // ./in.cue:235:33 - // large.p2.#X.port: cyclic reference to field port: - // ./in.cue:252:4 } Y: (struct){ userinfo: (string){ "user" } @@ -536,8 +522,6 @@ Disjuncts: 215 X: (_|_){ // [incomplete] large.p3.X: undefined field: port: // ./in.cue:276:33 - // large.p3.#X.port: cyclic reference to field port: - // ./in.cue:288:4 } #X: (_|_){ // [incomplete] large.p3.#X.port: cyclic reference to field port: @@ -558,8 +542,6 @@ Disjuncts: 215 X: (_|_){ // [incomplete] large.p4.X: undefined field: port: // ./in.cue:317:33 - // large.p4.#X.port: cyclic reference to field port: - // ./in.cue:329:4 } #X: (_|_){ // [incomplete] large.p4.#X.port: cyclic reference to field port: @@ -582,7 +564,7 @@ Disjuncts: 215 diff old new --- old +++ new -@@ -1,35 +1,36 @@ +@@ -1,35 +1,34 @@ (struct){ minimal: (struct){ a: (_|_){ @@ -622,8 +604,6 @@ diff old new + #Y: (_|_){ + // [incomplete] small.p1.#Y: undefined field: port: + // ./in.cue:28:50 -+ // small.p1.#X.port: cyclic reference to field port: -+ // ./in.cue:31:4 + } + #X: (_|_){ + // [incomplete] small.p1.#X.port: cyclic reference to field port: @@ -641,7 +621,7 @@ diff old new } } } -@@ -36,65 +37,69 @@ +@@ -36,65 +35,61 @@ medium: (struct){ #userHostPort: (string){ "^(:(?P\\d+))?$" } p1: (struct){ @@ -675,8 +655,6 @@ diff old new + #Y: (_|_){ + // [incomplete] medium.p1.#Y: undefined field: port: + // ./in.cue:58:50 -+ // medium.p1.#X.port: cyclic reference to field port: -+ // ./in.cue:70:4 + } + Y: (struct){ + } @@ -689,8 +667,6 @@ diff old new + #Y: (_|_){ + // [incomplete] medium.p2.#Y: undefined field: port: + // ./in.cue:80:50 -+ // medium.p2.#X.port: cyclic reference to field port: -+ // ./in.cue:86:4 + } + #X: (_|_){ + // [incomplete] medium.p2.#X.port: cyclic reference to field port: @@ -724,8 +700,6 @@ diff old new - // [cycle] medium.p4.#X: cycle with field Y.port: - // ./in.cue:134:7 + // ./in.cue:108:50 -+ // medium.p3.#X.port: cyclic reference to field port: -+ // ./in.cue:114:4 + } + #X: (_|_){ + // [incomplete] medium.p3.#X.port: cyclic reference to field port: @@ -740,9 +714,7 @@ diff old new + // ./in.cue:134:4 + } + #Y: (_|_){ -+ // [incomplete] medium.p4.#X.port: cyclic reference to field port: -+ // ./in.cue:134:4 -+ // medium.p4.#Y: undefined field: port: ++ // [incomplete] medium.p4.#Y: undefined field: port: + // ./in.cue:142:50 } } @@ -763,7 +735,7 @@ diff old new } Y: (struct){ } -@@ -101,14 +106,14 @@ +@@ -101,14 +96,14 @@ } p6: (struct){ #X: (_|_){ @@ -786,7 +758,7 @@ diff old new } } } -@@ -115,81 +120,91 @@ +@@ -115,81 +110,83 @@ large: (struct){ #userHostPort: (string){ "^((?P[[:alnum:]]*)@)?(?P[[:alnum:].]+)(:(?P\\d+))?$" } p1: (struct){ @@ -872,8 +844,6 @@ diff old new + X: (_|_){ + // [incomplete] large.p1.X: undefined field: port: + // ./in.cue:199:33 -+ // large.p1.#X.port: cyclic reference to field port: -+ // ./in.cue:211:4 + } + #X: (_|_){ + // [incomplete] large.p1.#X.port: cyclic reference to field port: @@ -890,8 +860,6 @@ diff old new + X: (_|_){ + // [incomplete] large.p2.X: undefined field: port: + // ./in.cue:235:33 -+ // large.p2.#X.port: cyclic reference to field port: -+ // ./in.cue:252:4 + } + Y: (struct){ + userinfo: (string){ "user" } @@ -912,8 +880,6 @@ diff old new + X: (_|_){ + // [incomplete] large.p3.X: undefined field: port: + // ./in.cue:276:33 -+ // large.p3.#X.port: cyclic reference to field port: -+ // ./in.cue:288:4 + } + #X: (_|_){ + // [incomplete] large.p3.#X.port: cyclic reference to field port: @@ -934,8 +900,6 @@ diff old new + X: (_|_){ + // [incomplete] large.p4.X: undefined field: port: + // ./in.cue:317:33 -+ // large.p4.#X.port: cyclic reference to field port: -+ // ./in.cue:329:4 + } + #X: (_|_){ + // [incomplete] large.p4.#X.port: cyclic reference to field port: diff --git a/cue/testdata/eval/issue2550.txtar b/cue/testdata/eval/issue2550.txtar index 2e3174141..3ea4e0f16 100644 --- a/cue/testdata/eval/issue2550.txtar +++ b/cue/testdata/eval/issue2550.txtar @@ -41,8 +41,13 @@ Result: let _bar#1 = (_){ _ } } -- out/evalalpha -- +Errors: +undefined field: missing: + ./in.cue:4:8 + +Result: (_|_){ - // [incomplete] undefined field: missing: + // [eval] undefined field: missing: // ./in.cue:4:8 foo: (_|_){ // [incomplete] undefined field: missing: @@ -50,24 +55,15 @@ Result: } bar: (#struct){ } - let _bar#1 = (_|_){ - // [incomplete] undefined field: missing: - // ./in.cue:4:8 - } + let _bar#1 = (_){ _ } } -- diff/-out/evalalpha<==>+out/eval -- diff old new --- old +++ new -@@ -1,13 +1,14 @@ --Errors: --undefined field: missing: -- ./in.cue:4:8 -- --Result: +@@ -6,7 +6,10 @@ (_|_){ -- // [eval] undefined field: missing: -+ // [incomplete] undefined field: missing: + // [eval] undefined field: missing: // ./in.cue:4:8 - foo: (string){ string } + foo: (_|_){ @@ -76,11 +72,6 @@ diff old new + } bar: (#struct){ } -- let _bar#1 = (_){ _ } -+ let _bar#1 = (_|_){ -+ // [incomplete] undefined field: missing: -+ // ./in.cue:4:8 -+ } - } + let _bar#1 = (_){ _ } -- diff/todo/p2 -- Let seems to have misplaced error, even though it does not affect outcome. diff --git a/internal/core/adt/binop.go b/internal/core/adt/binop.go index 52f45b4c6..0c75c0f84 100644 --- a/internal/core/adt/binop.go +++ b/internal/core/adt/binop.go @@ -32,12 +32,14 @@ func BinOp(c *OpContext, op Op, left, right Value) Value { return &Bottom{ Code: IncompleteError, Err: c.Newf(msg, left, op), + Node: c.vertex, } } if right.Concreteness() > Concrete { return &Bottom{ Code: IncompleteError, Err: c.Newf(msg, right, op), + Node: c.vertex, } } diff --git a/internal/core/adt/composite.go b/internal/core/adt/composite.go index 8c8cd76ea..69330304f 100644 --- a/internal/core/adt/composite.go +++ b/internal/core/adt/composite.go @@ -1196,7 +1196,10 @@ func (v *Vertex) AddConjunct(c Conjunct) *Bottom { // change the order of fields in some cases. // // This is likely a bug in the evaluator and should not happen. - return &Bottom{Err: errors.Newf(token.NoPos, "cannot add conjunct")} + return &Bottom{ + Err: errors.Newf(token.NoPos, "cannot add conjunct"), + Node: v, + } } if !v.hasConjunct(c) { v.addConjunctUnchecked(c) diff --git a/internal/core/adt/constraints.go b/internal/core/adt/constraints.go index 84e9f6035..dd2db8282 100644 --- a/internal/core/adt/constraints.go +++ b/internal/core/adt/constraints.go @@ -162,7 +162,8 @@ func matchPatternValue(ctx *OpContext, pattern Value, f Feature, label Value) (r addPositions(err, c) } ctx.AddBottom(&Bottom{ - Err: err, + Err: err, + Node: ctx.vertex, }) } if ctx.errs == nil { diff --git a/internal/core/adt/context.go b/internal/core/adt/context.go index ceae3bab1..5614e87bb 100644 --- a/internal/core/adt/context.go +++ b/internal/core/adt/context.go @@ -382,7 +382,11 @@ func (c *OpContext) addErrf(code ErrorCode, pos token.Pos, msg string, args ...i } func (c *OpContext) addErr(code ErrorCode, err errors.Error) { - c.AddBottom(&Bottom{Code: code, Err: err}) + c.AddBottom(&Bottom{ + Code: code, + Err: err, + Node: c.vertex, + }) } // AddBottom records an error in OpContext. @@ -393,7 +397,10 @@ func (c *OpContext) AddBottom(b *Bottom) { // AddErr records an error in OpContext. It returns errors collected so far. func (c *OpContext) AddErr(err errors.Error) *Bottom { if err != nil { - c.AddBottom(&Bottom{Err: err}) + c.AddBottom(&Bottom{ + Err: err, + Node: c.vertex, + }) } return c.errs } @@ -404,7 +411,12 @@ func (c *OpContext) NewErrf(format string, args ...interface{}) *Bottom { // TODO: consider renaming ot NewBottomf: this is now confusing as we also // have Newf. err := c.Newf(format, args...) - return &Bottom{Src: c.src, Err: err, Code: EvalError} + return &Bottom{ + Src: c.src, + Err: err, + Code: EvalError, + Node: c.vertex, + } } // AddErrf records an error in OpContext. It returns errors collected so far. @@ -615,6 +627,7 @@ func (c *OpContext) Evaluate(env *Environment, x Expr) (result Value, complete b val = &Bottom{ Code: IncompleteError, Err: c.Newf("UNANTICIPATED ERROR"), + Node: env.Vertex, } } @@ -639,6 +652,7 @@ func (c *OpContext) evaluateRec(v Conjunct, state combinedFlags) Value { val = &Bottom{ Code: IncompleteError, Err: c.Newf("UNANTICIPATED ERROR"), + Node: c.vertex, } } _ = c.PopState(s) @@ -961,6 +975,7 @@ func (c *OpContext) lookup(x *Vertex, pos token.Pos, l Feature, flags combinedFl Permanent: x.status >= conjuncts, Err: c.NewPosf(pos, "cannot reference optional field: %s", label), + Node: x, }) } } else { @@ -1007,6 +1022,7 @@ func (c *OpContext) lookup(x *Vertex, pos token.Pos, l Feature, flags combinedFl Code: code, Permanent: permanent, Err: err, + Node: x, }) } return a diff --git a/internal/core/adt/cycle.go b/internal/core/adt/cycle.go index e6966b97b..cc943b966 100644 --- a/internal/core/adt/cycle.go +++ b/internal/core/adt/cycle.go @@ -617,6 +617,7 @@ func (n *nodeContext) reportCycleError() { Code: StructuralCycleError, Err: n.ctx.Newf("structural cycle"), Value: n.node.Value(), + Node: n.node, // TODO: probably, this should have the referenced arc. })) n.node.Arcs = nil diff --git a/internal/core/adt/disjunct.go b/internal/core/adt/disjunct.go index 835f1d100..70f81f616 100644 --- a/internal/core/adt/disjunct.go +++ b/internal/core/adt/disjunct.go @@ -469,6 +469,7 @@ func (n *nodeContext) makeError() { b := &Bottom{ Code: code, Err: n.disjunctError(), + Node: n.node, } n.node.SetValue(n.ctx, b) } diff --git a/internal/core/adt/errors.go b/internal/core/adt/errors.go index 8a77e9b48..0c7da8936 100644 --- a/internal/core/adt/errors.go +++ b/internal/core/adt/errors.go @@ -86,6 +86,11 @@ type Bottom struct { ForCycle bool // this is a for cycle // Value holds the computed value so far in case Value Value + + // Node marks the node at which an error occurred. This is used to + // determine the package to which an error belongs. + // TODO: use a more precise mechanism for tracking the package. + Node *Vertex } func (x *Bottom) Source() ast.Node { return x.Src } @@ -146,6 +151,7 @@ func (n *nodeContext) AddChildError(recursive *Bottom) { HasRecursive: true, ChildError: true, Err: recursive.Err, + Node: n.node, }) return } @@ -229,6 +235,7 @@ func NewRequiredNotPresentError(ctx *OpContext, v *Vertex) *Bottom { b := &Bottom{ Code: IncompleteError, Err: err, + Node: v, } ctx.PopArc(saved) return b @@ -275,6 +282,7 @@ func (v *Vertex) reportFieldError(c *OpContext, pos token.Pos, f Feature, intMsg b := &Bottom{ Code: code, Err: err, + Node: v, } // TODO: yield failure c.AddBottom(b) // TODO: unify error mechanism. diff --git a/internal/core/adt/eval.go b/internal/core/adt/eval.go index 67e379fd9..f0218a80e 100644 --- a/internal/core/adt/eval.go +++ b/internal/core/adt/eval.go @@ -83,7 +83,11 @@ func (c *OpContext) evaluate(v *Vertex, r Resolver, state combinedFlags) Value { for ; v.Parent != nil && v.ArcType == ArcPending; v = v.Parent { } err := c.Newf("cycle with field %v", r) - b := &Bottom{Code: CycleError, Err: err} + b := &Bottom{ + Code: CycleError, + Err: err, + Node: v, + } v.setValue(c, v.status, b) return b // TODO: use this instead, as is usual for incomplete errors, @@ -872,6 +876,7 @@ func (n *nodeContext) completeArcs(state vertexStatus) { n.node.AddErr(ctx, &Bottom{ Src: c.expr.Source(), Code: CycleError, + Node: n.node, Err: ctx.NewPosf(pos(c.expr), "circular dependency in evaluation of conditionals: %v changed after evaluation", ctx.Str(c.expr)), @@ -1567,7 +1572,10 @@ func (n *nodeContext) addErr(err errors.Error) { n.assertInitialized() if err != nil { - n.addBottom(&Bottom{Err: err}) + n.addBottom(&Bottom{ + Err: err, + Node: n.node, + }) } } @@ -2152,6 +2160,7 @@ func (n *nodeContext) insertField(f Feature, mode ArcType, x Conjunct) *Vertex { default: n.addBottom(&Bottom{ Code: IncompleteError, + Node: n.node, Err: ctx.NewPosf(pos(x.Field()), "cannot add field %s: was already used", f.SelectorString(ctx)), diff --git a/internal/core/adt/expr.go b/internal/core/adt/expr.go index eb1d9fc72..cb6852e62 100644 --- a/internal/core/adt/expr.go +++ b/internal/core/adt/expr.go @@ -471,7 +471,10 @@ func (x *BoundExpr) evaluate(ctx *OpContext, state combinedFlags) Value { if x.Op != NotEqualOp { err := ctx.NewPosf(pos(x.Expr), "cannot use null for bound %s", x.Op) - return &Bottom{Err: err} + return &Bottom{ + Err: err, + Node: ctx.vertex, + } } default: mask := IntKind | FloatKind | NumberKind | StringKind | BytesKind @@ -485,7 +488,10 @@ func (x *BoundExpr) evaluate(ctx *OpContext, state combinedFlags) Value { } err := ctx.NewPosf(pos(x.Expr), "invalid value %s (type %s) for bound %s", v, k, x.Op) - return &Bottom{Err: err} + return &Bottom{ + Err: err, + Node: ctx.vertex, + } } if v, ok := x.Expr.(Value); ok { @@ -622,7 +628,12 @@ func (x *BoundValue) validate(c *OpContext, y Value) *Bottom { // predeclared identifier such as `int`. err := c.Newf("invalid value %v (out of bound %s)", y, x) err.AddPosition(y) - return &Bottom{Src: c.src, Err: err, Code: EvalError} + return &Bottom{ + Src: c.src, + Err: err, + Code: EvalError, + Node: c.vertex, + } default: panic(fmt.Sprintf("unsupported type %T", v)) @@ -1107,7 +1118,11 @@ func (x *SliceExpr) evaluate(c *OpContext, state combinedFlags) Value { for i, a := range v.Arcs[lo:hi] { label, err := MakeLabel(a.Source(), int64(i), IntLabel) if err != nil { - c.AddBottom(&Bottom{Src: a.Source(), Err: err}) + c.AddBottom(&Bottom{ + Src: a.Source(), + Err: err, + Node: v, + }) return nil } arc := *a @@ -1173,6 +1188,7 @@ func (x *Interpolation) evaluate(c *OpContext, state combinedFlags) Value { if err := c.Err(); err != nil { err = &Bottom{ Code: err.Code, + Node: c.vertex, Err: errors.Wrapf(err.Err, pos(x), "invalid interpolation"), } // c.AddBottom(err) @@ -1758,7 +1774,11 @@ func validateWithBuiltin(c *OpContext, src token.Pos, b *Builtin, args []Value) vErr.AddPosition(v) } - return &Bottom{Code: severeness, Err: errors.Wrap(vErr, err)} + return &Bottom{ + Code: severeness, + Err: errors.Wrap(vErr, err), + Node: c.vertex, + } } // A Disjunction represents a disjunction, where each disjunct may or may not @@ -1992,6 +2012,7 @@ func (x *ForClause) yield(s *compState) { Code: CycleError, ForCycle: true, Value: n, + Node: n, Err: errors.Newf(pos(x.Src), "comprehension source references itself"), }) return diff --git a/internal/core/adt/optional.go b/internal/core/adt/optional.go index 3606a6dae..91db97c5e 100644 --- a/internal/core/adt/optional.go +++ b/internal/core/adt/optional.go @@ -104,7 +104,8 @@ func matchBulk(c *OpContext, env *Environment, p *BulkOptionalField, f Feature, err.AddPosition(c.Elem()) } c.AddBottom(&Bottom{ - Err: err, + Err: err, + Node: c.vertex, }) } if c.errs == nil { diff --git a/internal/core/adt/tasks.go b/internal/core/adt/tasks.go index 49c2bcd06..2a82b1822 100644 --- a/internal/core/adt/tasks.go +++ b/internal/core/adt/tasks.go @@ -135,6 +135,7 @@ func processDynamic(ctx *OpContext, t *task, mode runMode) { if v.Concreteness() != Concrete { n.addBottom(&Bottom{ Code: IncompleteError, + Node: n.node, Err: ctx.NewPosf(pos(field.Key), "key value of dynamic field must be concrete, found %v", v), }) diff --git a/internal/core/adt/unify.go b/internal/core/adt/unify.go index 056bcf9a4..c3e1612b1 100644 --- a/internal/core/adt/unify.go +++ b/internal/core/adt/unify.go @@ -610,6 +610,13 @@ func (n *nodeContext) evalArcTypes() { } } +func root(v *Vertex) *Vertex { + for v.Parent != nil { + v = v.Parent + } + return v +} + func (v *Vertex) lookup(c *OpContext, pos token.Pos, f Feature, flags combinedFlags) *Vertex { task := c.current() needs := flags.conditions() @@ -626,10 +633,15 @@ func (v *Vertex) lookup(c *OpContext, pos token.Pos, f Feature, flags combinedFl // proceed with partial data, in which case a "pending" arc will be // created to be completed later. - // Report error for now. + // Propagate error if the error is from a different package. This + // compensates for the fact that we do not fully evaluate the package. if state.hasErr() { - c.AddBottom(state.getErr()) + err := state.getErr() + if err != nil && err.Node != nil && root(err.Node) != root(v) { + c.AddBottom(err) + } } + // TODO: ideally this should not be run at this point. Consider under // which circumstances this is still necessary, and at least ensure // this will not be run if node v currently has a running task. @@ -743,6 +755,7 @@ handleArcType: label := f.SelectorString(c.Runtime) b := &Bottom{ Code: IncompleteError, + Node: v, Err: c.NewPosf(pos, "cannot reference optional field: %s", label), } diff --git a/internal/core/compile/builtin.go b/internal/core/compile/builtin.go index 9a2045dd9..c8df3b543 100644 --- a/internal/core/compile/builtin.go +++ b/internal/core/compile/builtin.go @@ -136,7 +136,8 @@ var orBuiltin = &adt.Builtin{ // status if the source is open. return &adt.Bottom{ Code: adt.IncompleteError, - Err: errors.Newf(c.Pos(), "empty list in call to or"), + // TODO: get and set Vertex + Err: errors.Newf(c.Pos(), "empty list in call to or"), } } v := &adt.Vertex{} diff --git a/internal/core/convert/go.go b/internal/core/convert/go.go index e4f29339f..9ee874262 100644 --- a/internal/core/convert/go.go +++ b/internal/core/convert/go.go @@ -71,7 +71,8 @@ func toValue(e adt.Expr) adt.Value { func compileExpr(ctx *adt.OpContext, expr ast.Expr) adt.Value { c, err := compile.Expr(nil, ctx, pkgID(), expr) if err != nil { - return &adt.Bottom{Err: errors.Promote(err, "compile")} + return &adt.Bottom{ + Err: errors.Promote(err, "compile")} } return adt.Resolve(ctx, c) } diff --git a/internal/core/export/testdata/main/dynamic.txtar b/internal/core/export/testdata/main/dynamic.txtar index 73e02e63d..30bf33c97 100644 --- a/internal/core/export/testdata/main/dynamic.txtar +++ b/internal/core/export/testdata/main/dynamic.txtar @@ -54,11 +54,12 @@ r1: "r1" ["s-q"] ["s-r"] ["s-q1"] +["s-q1" r1] -- diff/-out/doc-v3<==>+out/doc -- diff old new --- old +++ new -@@ -8,9 +8,7 @@ +@@ -8,7 +8,6 @@ [q1] [r] [r1] @@ -66,14 +67,9 @@ diff old new [y] ["s-q"] ["s-r"] - ["s-q1"] --["s-q1" r1] -- diff/out/todo/p3 -- One missing entry is the result of the new evaluator not generating a bogus field for a failed dynamic field. -The second one results from the fact that the new evaluator does not -resolve `r1`, but rather propagates the error of the top-level node. -This is probably okay. -- out/doc -- [] [x] @@ -141,7 +137,7 @@ This is probably okay. } } == Final -_|_ // key value of dynamic field must be concrete, found string (and 2 more errors) +_|_ // key value of dynamic field must be concrete, found string (and 3 more errors) == All { x: string @@ -247,7 +243,7 @@ diff old new +} == Final -_|_ // invalid non-ground value string (must be concrete string) (and 1 more errors) -+_|_ // key value of dynamic field must be concrete, found string (and 2 more errors) ++_|_ // key value of dynamic field must be concrete, found string (and 3 more errors) == All -_|_ // invalid non-ground value string (must be concrete string) (and 1 more errors) +{