diff --git a/cue/testdata/builtins/default.txtar b/cue/testdata/builtins/default.txtar index 0d2c6ca6de6..13801a7fa2d 100644 --- a/cue/testdata/builtins/default.txtar +++ b/cue/testdata/builtins/default.txtar @@ -45,6 +45,30 @@ Retain: 0 Unifications: 25 Conjuncts: 52 Disjuncts: 45 +-- out/evalalpha -- +(struct){ + Len: (int){ 3 } + Close: (#struct){ + } + And: (int){ 1 } + Or: (int){ 1 } + Div: (int){ 2 } + Mod: (int){ 1 } + Quo: (int){ 2 } + Rem: (int){ 1 } +} +-- diff/-out/evalalpha<==>+out/eval -- +diff old new +--- old ++++ new +@@ -1,6 +1,6 @@ + (struct){ + Len: (int){ 3 } +- Close: (struct){ ++ Close: (#struct){ + } + And: (int){ 1 } + Or: (int){ 1 } -- out/eval -- (struct){ Len: (int){ 3 } diff --git a/cue/testdata/cycle/evaluate.txtar b/cue/testdata/cycle/evaluate.txtar index 36d568e2a26..e0abf12413d 100644 --- a/cue/testdata/cycle/evaluate.txtar +++ b/cue/testdata/cycle/evaluate.txtar @@ -127,7 +127,6 @@ Conjuncts: 299 Disjuncts: 192 -- out/evalalpha -- Errors: -closeCycle.c: structural cycle printCycle.a.X: structural cycle structCycle.c: structural cycle letCycleOK.t2.a.X: structural cycle: @@ -139,6 +138,8 @@ disjunctionCycle.b: cannot use 1 (type int) as type list: ./in.cue:56:9 b: structural cycle: ./in.cue:62:6 +closeCycle.c: structural cycle: + ./in.cue:73:5 structural cycle: ./in.cue:85:12 listAddCycle.c: structural cycle: @@ -251,15 +252,17 @@ Result: } closeCycle: (_|_){ // [structural cycle] - a: ~(closeCycle.b) + a: (_|_){ + // [structural cycle] closeCycle.c: structural cycle: + // ./in.cue:73:5 + } b: (_|_){ - // [structural cycle] - d: (_|_){ - // [structural cycle] closeCycle.c: structural cycle - } + // [structural cycle] closeCycle.c: structural cycle: + // ./in.cue:73:5 } c: (_|_){ - // [structural cycle] closeCycle.c: structural cycle + // [structural cycle] closeCycle.c: structural cycle: + // ./in.cue:73:5 } } structCycle: (_|_){ @@ -350,7 +353,7 @@ Result: diff old new --- old +++ new -@@ -1,50 +1,49 @@ +@@ -1,50 +1,50 @@ Errors: -closeCycle.a: structural cycle -closeCycle.b.d: structural cycle @@ -365,7 +368,6 @@ diff old new -disjunctionCycle.a: cannot use 1 (type int) as type list: - ./in.cue:56:5 - ./in.cue:56:9 -+closeCycle.c: structural cycle +printCycle.a.X: structural cycle +structCycle.c: structural cycle +letCycleOK.t2.a.X: structural cycle: @@ -380,7 +382,7 @@ diff old new - ./in.cue:56:9 b: structural cycle: ./in.cue:62:6 --closeCycle.c: structural cycle: + closeCycle.c: structural cycle: - ./in.cue:73:15 -structCycle.c: structural cycle: - ./in.cue:79:14 @@ -388,6 +390,7 @@ diff old new - ./in.cue:85:11 -printCycle.a.X.X: structural cycle: - ./in.cue:113:6 ++ ./in.cue:73:5 +structural cycle: + ./in.cue:85:12 +listAddCycle.c: structural cycle: @@ -428,7 +431,7 @@ diff old new } } } -@@ -59,20 +58,16 @@ +@@ -59,20 +59,16 @@ // [structural cycle] letCycleFail.t1.a.X: structural cycle } c: (_|_){ @@ -459,7 +462,7 @@ diff old new } x: (struct){ y: (string){ "" } -@@ -88,17 +83,17 @@ +@@ -88,17 +84,17 @@ disjunctionCycle: (_|_){ // [eval] a: (_|_){ @@ -488,7 +491,7 @@ diff old new // ./in.cue:56:5 // ./in.cue:56:9 } -@@ -124,80 +119,77 @@ +@@ -124,80 +120,79 @@ } b: (struct){ } @@ -500,7 +503,7 @@ diff old new } closeCycle: (_|_){ // [structural cycle] -- a: (_|_){ + a: (_|_){ - // [structural cycle] closeCycle.a: structural cycle - } - b: (_|_){ @@ -512,15 +515,16 @@ diff old new - c: (_|_){ - // [structural cycle] closeCycle.c: structural cycle: - // ./in.cue:73:15 -+ a: ~(closeCycle.b) ++ // [structural cycle] closeCycle.c: structural cycle: ++ // ./in.cue:73:5 ++ } + b: (_|_){ -+ // [structural cycle] -+ d: (_|_){ -+ // [structural cycle] closeCycle.c: structural cycle -+ } ++ // [structural cycle] closeCycle.c: structural cycle: ++ // ./in.cue:73:5 + } + c: (_|_){ -+ // [structural cycle] closeCycle.c: structural cycle ++ // [structural cycle] closeCycle.c: structural cycle: ++ // ./in.cue:73:5 } } structCycle: (_|_){ @@ -618,7 +622,7 @@ diff old new } } closeFail: (_|_){ -@@ -207,21 +199,22 @@ +@@ -207,21 +202,22 @@ } x: (_|_){ // [eval] diff --git a/cue/testdata/cycle/inline_non_recursive.txtar b/cue/testdata/cycle/inline_non_recursive.txtar index 0a7ca8ad733..1dd380ae90f 100644 --- a/cue/testdata/cycle/inline_non_recursive.txtar +++ b/cue/testdata/cycle/inline_non_recursive.txtar @@ -80,14 +80,14 @@ issue3182: { } } -- out/evalalpha/stats -- -Leaks: 250 +Leaks: 272 Freed: 0 Reused: 0 -Allocs: 250 +Allocs: 272 Retain: 0 -Unifications: 240 -Conjuncts: 1277 +Unifications: 251 +Conjuncts: 1299 Disjuncts: 0 -- out/evalalpha -- Errors: @@ -203,17 +203,17 @@ diff old new -Reused: 401 -Allocs: 310 -Retain: 1066 -+Leaks: 250 ++Leaks: 272 +Freed: 0 +Reused: 0 -+Allocs: 250 ++Allocs: 272 +Retain: 0 -Unifications: 711 -Conjuncts: 2759 -Disjuncts: 1446 -+Unifications: 240 -+Conjuncts: 1277 ++Unifications: 251 ++Conjuncts: 1299 +Disjuncts: 0 -- diff/-out/evalalpha<==>+out/eval -- diff old new diff --git a/internal/core/adt/closed.go b/internal/core/adt/closed.go index 6c639ef084e..b11fedbbb58 100644 --- a/internal/core/adt/closed.go +++ b/internal/core/adt/closed.go @@ -330,9 +330,10 @@ func isClosed(v *Vertex) bool { // We could have used IsRecursivelyClosed here, but (effectively) // implementing it again here allows us to only have to iterate over // Structs once. - if v.ClosedRecursive { + if v.ClosedRecursive || v.ClosedNonRecursive { return true } + // TODO(evalv3): this can be removed once we delete the evalv2 code. for _, s := range v.Structs { if s.IsClosed || s.IsInOneOf(DefinitionSpan) { return true diff --git a/internal/core/adt/composite.go b/internal/core/adt/composite.go index 8eae8a41c52..255151d6e78 100644 --- a/internal/core/adt/composite.go +++ b/internal/core/adt/composite.go @@ -187,6 +187,10 @@ type Vertex struct { // of the conjuncts, or ancestor conjuncts, is a definition. ClosedRecursive bool + // ClosedNonRecursive indicates that this Vertex has been closed for this + // level only. This supports the close builtin. + ClosedNonRecursive bool + // HasEllipsis indicates that this Vertex is open by means of an ellipsis. // TODO: combine this field with Closed once we removed the old evaluator. HasEllipsis bool @@ -583,6 +587,15 @@ const ( finalized ) +// Wrap creates a Vertex that takes w as a shared value. This allows users +// to set different flags for a wrapped Vertex. +func (c *OpContext) Wrap(v *Vertex, id CloseInfo) *Vertex { + w := c.newInlineVertex(nil, nil, v.Conjuncts...) + n := w.getState(c) + n.share(makeAnonymousConjunct(nil, v, nil), v, CloseInfo{}) + return w +} + // Status returns the status of the current node. When reading the status, one // should always use this method over directly reading status field. // diff --git a/internal/core/adt/conjunct.go b/internal/core/adt/conjunct.go index b5adca0d5eb..faee627f2dd 100644 --- a/internal/core/adt/conjunct.go +++ b/internal/core/adt/conjunct.go @@ -552,20 +552,20 @@ func (n *nodeContext) insertValueConjunct(env *Environment, v Value, id CloseInf switch x := v.(type) { case *Vertex: - if m, ok := x.BaseValue.(*StructMarker); ok { + if x.ClosedNonRecursive { + n.node.ClosedNonRecursive = true + var cc *closeContext + id, cc = id.spawnCloseContext(n.ctx, 0) + cc.isClosedOnce = true + + if v, ok := x.BaseValue.(*Vertex); ok { + n.insertValueConjunct(env, v, id) + return + } + } + if _, ok := x.BaseValue.(*StructMarker); ok { n.aStruct = x n.aStructID = id - if m.NeedClose { - // TODO: In the new evaluator this is used to mark a struct - // as closed in the debug output. Once the old evaluator is - // gone, we could simplify this. - id.IsClosed = true - if ctx.isDevVersion() { - var cc *closeContext - id, cc = id.spawnCloseContext(n.ctx, 0) - cc.isClosedOnce = true - } - } } if !x.IsData() { diff --git a/internal/core/adt/expr.go b/internal/core/adt/expr.go index 23be3e612e5..0a2a5a8f1ac 100644 --- a/internal/core/adt/expr.go +++ b/internal/core/adt/expr.go @@ -421,6 +421,9 @@ func (x *ListMarker) node() {} type StructMarker struct { // NeedClose is used to signal that the evaluator should close this struct. // It is only set by the close builtin. + // TODO(evalv3: remove this field. Once we removed this, and also introduced + // open by default lists, we can get rid of StructMarker and ListMarker + // in its entirety in favor of using type bit masks. NeedClose bool } diff --git a/internal/core/adt/sched.go b/internal/core/adt/sched.go index f4f3186c0d3..0030ac478d9 100644 --- a/internal/core/adt/sched.go +++ b/internal/core/adt/sched.go @@ -703,7 +703,10 @@ func runTask(t *task, mode runMode) { defer ctx.popTask() if t.env != nil { id := t.id - id.cc = nil // this is done to avoid struct args from passing fields up. + // This is done to avoid struct args from passing fields up. + // Use [task.updateCI] to get the current CloseInfo with this field + // restored. + id.cc = nil s := ctx.PushConjunct(MakeConjunct(t.env, t.x, id)) defer ctx.PopState(s) } @@ -736,6 +739,13 @@ func runTask(t *task, mode runMode) { } } +// updateCI stitches back the closeContext that more removed from the CloseInfo +// before in the given CloseInfo. +func (t *task) updateCI(ci CloseInfo) CloseInfo { + ci.cc = t.id.cc + return ci +} + // waitFor blocks task t until the needs for scheduler s are met. func (t *task) waitFor(s *scheduler, needs condition) { if s.meets(needs) { diff --git a/internal/core/adt/share.go b/internal/core/adt/share.go index 9cf1d77f40a..6caa3f5437a 100644 --- a/internal/core/adt/share.go +++ b/internal/core/adt/share.go @@ -113,7 +113,9 @@ func (n *nodeContext) share(c Conjunct, arc *Vertex, id CloseInfo) { // have been processed and it is clear that sharing is possible. Delaying // such a count should not hurt performance, as a shared node is completed // anyway. - id.cc.incDependent(n.ctx, SHARED, n.node.cc()) + if id.cc != nil { + id.cc.incDependent(n.ctx, SHARED, n.node.cc()) + } } func (n *nodeContext) shareIfPossible(c Conjunct, arc *Vertex, id CloseInfo) bool { diff --git a/internal/core/adt/tasks.go b/internal/core/adt/tasks.go index 8779a9e4cc5..9301ba26a7d 100644 --- a/internal/core/adt/tasks.go +++ b/internal/core/adt/tasks.go @@ -90,8 +90,8 @@ func processExpr(ctx *OpContext, t *task, mode runMode) { // This is an optional cycle that we will ignore. return } - t.id.CycleInfo = ci.CycleInfo - t.node.insertValueConjunct(t.env, v, t.id) + ci = t.updateCI(ci) + t.node.insertValueConjunct(t.env, v, ci) } func processResolver(ctx *OpContext, t *task, mode runMode) { @@ -113,9 +113,11 @@ func processResolver(ctx *OpContext, t *task, mode runMode) { // TODO: consider moving after markCycle or removing. d := arc.DerefDisjunct() + ci := t.updateCI(ctx.ci) + // A reference that points to itself indicates equality. In that case // we are done computing and we can return the arc as is. - ci, skip := t.node.detectCycleV3(d, t.env, r, t.id) + ci, skip := t.node.detectCycleV3(d, t.env, r, ci) if skip { return } @@ -162,9 +164,14 @@ func processDynamic(ctx *OpContext, t *task, mode runMode) { return } - c := MakeConjunct(t.env, field, t.id) + // Do not update the CloseInfo, as we are passing the field value + // unevaluated. + ci := t.id + + c := MakeConjunct(t.env, field, ci) + // TODO(evalv3): this does not seem to be necessary and even treacherous. c.CloseInfo.cc = nil - n.insertArc(f, field.ArcType, c, t.id, true) + n.insertArc(f, field.ArcType, c, ci, true) } func processPatternConstraint(ctx *OpContext, t *task, mode runMode) { @@ -179,7 +186,11 @@ func processPatternConstraint(ctx *OpContext, t *task, mode runMode) { return } - n.insertPattern(v, MakeConjunct(t.env, t.x, t.id)) + // Do not update the CloseInfo, as we are passing the constraint value + // unevaluated. + ci := t.id + + n.insertPattern(v, MakeConjunct(t.env, t.x, ci)) } func processComprehension(ctx *OpContext, t *task, mode runMode) { diff --git a/internal/core/adt/unify.go b/internal/core/adt/unify.go index bea759be906..69ce7db8078 100644 --- a/internal/core/adt/unify.go +++ b/internal/core/adt/unify.go @@ -318,6 +318,8 @@ func (v *Vertex) unify(c *OpContext, needs condition, mode runMode) bool { if w.ClosedRecursive { v.ClosedRecursive = true } + // NOTE: setting ClosedNonRecursive is not necessary, as it is + // handled by scheduleValue. if w.HasEllipsis { v.HasEllipsis = true } diff --git a/internal/core/compile/builtin.go b/internal/core/compile/builtin.go index 7242946fade..79e5bbca477 100644 --- a/internal/core/compile/builtin.go +++ b/internal/core/compile/builtin.go @@ -16,6 +16,7 @@ package compile import ( "cuelang.org/go/cue/errors" + "cuelang.org/go/internal" "cuelang.org/go/internal/core/adt" ) @@ -81,19 +82,26 @@ var closeBuiltin = &adt.Builtin{ Name: "close", Params: []adt.Param{structParam}, Result: adt.StructKind, - // NonConcrete: true, // TODO: should probably be noncrete Func: func(c *adt.OpContext, args []adt.Value) adt.Expr { s, ok := args[0].(*adt.Vertex) if !ok { return c.NewErrf("struct argument must be concrete") } - if m, ok := s.BaseValue.(*adt.StructMarker); ok && m.NeedClose { - return s + var v *adt.Vertex + if c.Version == internal.DevVersion { + // TODO(evalv3) this is a rather convoluted and inefficient way to + // accomplish signaling vertex should be closed. In most cases, it + // would suffice to set IsClosed in the CloseInfo. However, that + // does not cover all code paths. Consider simplifying this. + v = c.Wrap(s, c.CloseInfo()) + v.ClosedNonRecursive = true + } else { + if m, ok := s.BaseValue.(*adt.StructMarker); ok && m.NeedClose { + return s + } + v = s.Clone() + v.BaseValue = &adt.StructMarker{NeedClose: true} } - v := s.Clone() - // TODO(perf): do not copy the arc, but rather find a way to mark the - // calling nodeContext. - v.BaseValue = &adt.StructMarker{NeedClose: true} return v }, } diff --git a/internal/core/export/expr.go b/internal/core/export/expr.go index 3eeefa1831f..14c00c31768 100644 --- a/internal/core/export/expr.go +++ b/internal/core/export/expr.go @@ -301,7 +301,12 @@ func (x *exporter) mergeValues(label adt.Feature, src *adt.Vertex, a []conjunct, func (e *conjuncts) wrapCloseIfNecessary(s *ast.StructLit, v *adt.Vertex) ast.Expr { if !e.hasEllipsis && v != nil { + if v.ClosedNonRecursive { + // Eval V3 logic + return ast.NewCall(ast.NewIdent("close"), s) + } if st, ok := v.BaseValue.(*adt.StructMarker); ok && st.NeedClose { + // Eval V2 logic return ast.NewCall(ast.NewIdent("close"), s) } }