diff --git a/cue/testdata/cycle/chain.txtar b/cue/testdata/cycle/chain.txtar index 977a1fd4ec8..e617dc376d6 100644 --- a/cue/testdata/cycle/chain.txtar +++ b/cue/testdata/cycle/chain.txtar @@ -208,21 +208,21 @@ issue2052: full: { d: #Depth & {#in: tree} } -- out/evalalpha/stats -- -Leaks: 22127 -Freed: 1994 -Reused: 1993 -Allocs: 22128 +Leaks: 19985 +Freed: 1462 +Reused: 1461 +Allocs: 19986 Retain: 0 -Unifications: 8358 -Conjuncts: 121910 -Disjuncts: 15463 +Unifications: 7496 +Conjuncts: 107561 +Disjuncts: 13651 -- out/evalalpha -- Errors: issue2052.t1.#Depth: adding field #basic not allowed as field set was already referenced: ./issue2052.cue:8:20 issue2052.t2.#Depth: adding field #basic not allowed as field set was already referenced: - ./issue2052.cue:46:14 + ./issue2052.cue:41:20 Result: (_|_){ @@ -350,7 +350,7 @@ Result: // [eval] #Depth: (_|_){ // [eval] issue2052.t2.#Depth: adding field #basic not allowed as field set was already referenced: - // ./issue2052.cue:46:14 + // ./issue2052.cue:41:20 #maxiter: (int){ 4 } #funcs: (#struct){ "4": (null){ null } @@ -725,18 +725,18 @@ diff old new -Reused: 1801 -Allocs: 84 -Retain: 185 -+Leaks: 22127 -+Freed: 1994 -+Reused: 1993 -+Allocs: 22128 ++Leaks: 19985 ++Freed: 1462 ++Reused: 1461 ++Allocs: 19986 +Retain: 0 -Unifications: 800 -Conjuncts: 3175 -Disjuncts: 1974 -+Unifications: 8358 -+Conjuncts: 121910 -+Disjuncts: 15463 ++Unifications: 7496 ++Conjuncts: 107561 ++Disjuncts: 13651 -- diff/-out/evalalpha<==>+out/eval -- diff old new --- old @@ -747,7 +747,7 @@ diff old new +issue2052.t1.#Depth: adding field #basic not allowed as field set was already referenced: + ./issue2052.cue:8:20 +issue2052.t2.#Depth: adding field #basic not allowed as field set was already referenced: -+ ./issue2052.cue:46:14 ++ ./issue2052.cue:41:20 + +Result: +(_|_){ @@ -969,7 +969,7 @@ diff old new + // [eval] + #Depth: (_|_){ + // [eval] issue2052.t2.#Depth: adding field #basic not allowed as field set was already referenced: -+ // ./issue2052.cue:46:14 ++ // ./issue2052.cue:41:20 + #maxiter: (int){ 4 } + #funcs: (#struct){ + "4": (null){ null } diff --git a/cue/testdata/cycle/comprehension.txtar b/cue/testdata/cycle/comprehension.txtar index ba8b86ceb2c..befb0fb4bbf 100644 --- a/cue/testdata/cycle/comprehension.txtar +++ b/cue/testdata/cycle/comprehension.txtar @@ -310,15 +310,15 @@ issue2367: { } -- out/evalalpha/stats -- -Leaks: 451 +Leaks: 431 Freed: 12 Reused: 12 -Allocs: 451 +Allocs: 431 Retain: 0 -Unifications: 375 -Conjuncts: 2094 -Disjuncts: 58 +Unifications: 397 +Conjuncts: 2202 +Disjuncts: 36 -- out/evalalpha -- Errors: issue1881.p1.o.retry: field not allowed: @@ -1482,18 +1482,18 @@ diff old new -Reused: 1260 -Allocs: 60 -Retain: 145 -+Leaks: 451 ++Leaks: 431 +Freed: 12 +Reused: 12 -+Allocs: 451 ++Allocs: 431 +Retain: 0 -Unifications: 832 -Conjuncts: 2525 -Disjuncts: 1404 -+Unifications: 375 -+Conjuncts: 2094 -+Disjuncts: 58 ++Unifications: 397 ++Conjuncts: 2202 ++Disjuncts: 36 -- out/eval/stats -- Leaks: 50 Freed: 1270 diff --git a/cue/testdata/disjunctions/elimination.txtar b/cue/testdata/disjunctions/elimination.txtar index da0d8df94a1..4614e8debab 100644 --- a/cue/testdata/disjunctions/elimination.txtar +++ b/cue/testdata/disjunctions/elimination.txtar @@ -979,22 +979,37 @@ issue2263: full: { } BAZ: (_){ _ } } - t3: (struct){ - #A: (#struct){ - v: (int){ 1 } - } - BAZ: (_){ _ } - S: (#struct){ - x: (int){ 1 } - y: (int){ 1 } - } - #B: (#struct){ - x: (int){ 1 } - y: (int){ 1 } - } - f1: (int){ int } - f2: (int){ int } - } + t3: (struct){ |((struct){ + #A: (#struct){ + v: (int){ 1 } + } + BAZ: (_){ _ } + S: (#struct){ + x: (int){ 1 } + y: (int){ 1 } + } + #B: (#struct){ + x: (int){ 1 } + y: (int){ 1 } + } + f1: (int){ int } + f2: (int){ int } + }, (struct){ + #A: (#struct){ + v: (int){ 1 } + } + BAZ: (_){ _ } + S: (#struct){ + x: (int){ 1 } + y: (int){ 1 } + } + #B: (#struct){ + x: (int){ 1 } + y: (int){ 1 } + } + f1: (int){ int } + b2: (int){ int } + }) } } full: (struct){ Foo: (#struct){ @@ -1646,7 +1661,7 @@ diff old new t1: (struct){ #SpecFoo: (#struct){ foo: (#struct){ -@@ -508,185 +484,103 @@ +@@ -508,185 +484,118 @@ x: (int){ 1 } } } @@ -1669,16 +1684,9 @@ diff old new - t3: (_|_){ - // [eval] issue2209.simplified.t3.BAZ: undefined field: y: - // ./issue2209full.cue:35:9 -+ spec: (struct){ -+ bar: (struct){ -+ } -+ } -+ BAZ: (_){ _ } -+ } -+ t3: (struct){ - #A: (#struct){ - v: (int){ 1 } - } +- #A: (#struct){ +- v: (int){ 1 } +- } - BAZ: (_|_){ - // [eval] issue2209.simplified.t3.BAZ: undefined field: y: - // ./issue2209full.cue:35:9 @@ -1690,23 +1698,52 @@ diff old new - x: (int){ 1 } - y: (int){ 1 } - }) } -+ BAZ: (_){ _ } -+ S: (#struct){ -+ x: (int){ 1 } -+ y: (int){ 1 } -+ } - #B: (#struct){ - x: (int){ 1 } - y: (int){ 1 } - } +- #B: (#struct){ +- x: (int){ 1 } +- y: (int){ 1 } +- } - b2: (int){ int } - } - } - full: (_|_){ - // [eval] -+ f1: (int){ int } -+ f2: (int){ int } ++ spec: (struct){ ++ bar: (struct){ ++ } ++ } ++ BAZ: (_){ _ } + } ++ t3: (struct){ |((struct){ ++ #A: (#struct){ ++ v: (int){ 1 } ++ } ++ BAZ: (_){ _ } ++ S: (#struct){ ++ x: (int){ 1 } ++ y: (int){ 1 } ++ } ++ #B: (#struct){ ++ x: (int){ 1 } ++ y: (int){ 1 } ++ } ++ f1: (int){ int } ++ f2: (int){ int } ++ }, (struct){ ++ #A: (#struct){ ++ v: (int){ 1 } ++ } ++ BAZ: (_){ _ } ++ S: (#struct){ ++ x: (int){ 1 } ++ y: (int){ 1 } ++ } ++ #B: (#struct){ ++ x: (int){ 1 } ++ y: (int){ 1 } ++ } ++ f1: (int){ int } ++ b2: (int){ int } ++ }) } + } + full: (struct){ Foo: (#struct){ @@ -1921,7 +1958,7 @@ diff old new } } #Abstract: (#struct){ -@@ -702,34 +596,34 @@ +@@ -702,34 +611,34 @@ } }) } resource: (#struct){ @@ -1984,7 +2021,7 @@ diff old new } } _#Spec: (#struct){ |(*(#struct){ -@@ -756,36 +650,36 @@ +@@ -756,36 +665,36 @@ } } _Thing: (#struct){ @@ -2050,7 +2087,7 @@ diff old new } #Constrained: (#struct){ spec: (#struct){ |(*(#struct){ -@@ -869,19 +763,19 @@ +@@ -869,19 +778,19 @@ common: (int){ 3 } } #FormFoo: (#struct){ @@ -2077,7 +2114,7 @@ diff old new }) } } #Input: (#struct){ -@@ -930,10 +824,23 @@ +@@ -930,10 +839,23 @@ } full: (struct){ metrics: (#list){ diff --git a/cue/types.go b/cue/types.go index 27515e2956a..75fa7811c5e 100644 --- a/cue/types.go +++ b/cue/types.go @@ -235,11 +235,6 @@ func (i *Iterator) Next() bool { p := linkParent(i.val.parent_, i.val.v, arc) i.f = arc.Label i.arcType = arc.ArcType - // TODO(deref): this should not indirect disjunctions, as this will cause losing - // information, which is now available in the new evaluator. Find a safe - // way to preserve disjunction information while keeping backward - // compatibility in the API. - arc = arc.DerefValue() i.cur = makeValue(i.val.idx, arc, p) i.p++ return true diff --git a/cue/types_test.go b/cue/types_test.go index 3516fa3d6e4..4dc99d297ab 100644 --- a/cue/types_test.go +++ b/cue/types_test.go @@ -3559,8 +3559,6 @@ func TestPathCorrection(t *testing.T) { continue } runMatrix(t, "", func(t *testing.T, cfg *evalConfig) { - TODO_Sharing(t, cfg) - r := cfg.runtime() inst, err := r.Compile("in", tc.input) diff --git a/internal/core/adt/context.go b/internal/core/adt/context.go index 3508de6441f..f7a3fa80f58 100644 --- a/internal/core/adt/context.go +++ b/internal/core/adt/context.go @@ -710,6 +710,10 @@ func (c *OpContext) evalState(v Expr, state combinedFlags) (result Value) { if arc == nil { return nil } + // TODO(deref): what is the right level of dereferencing here? + // DerefValue seems to work too. + arc = arc.DerefNonShared() + // TODO: consider moving this after markCycle, depending on how we // implement markCycle, or whether we need it at all. // TODO: is this indirect necessary? @@ -838,6 +842,8 @@ func (c *OpContext) unifyNode(v Expr, state combinedFlags) (result Value) { if v == nil { return nil } + v = v.DerefValue() + // TODO: consider moving this after markCycle, depending on how we // implement markCycle, or whether we need it at all. // TODO: is this indirect necessary? diff --git a/internal/core/adt/share.go b/internal/core/adt/share.go index 7867228ba21..ed7063934ef 100644 --- a/internal/core/adt/share.go +++ b/internal/core/adt/share.go @@ -153,6 +153,17 @@ func (v *Vertex) DerefNonDisjunct() *Vertex { } } +// DerefNonRooted indirects a node that points to a value that is not rooted. +func (v *Vertex) DerefNonRooted() *Vertex { + for { + arc, ok := v.BaseValue.(*Vertex) + if !ok || arc.IsDisjunct || v.IsShared { + return v + } + v = arc + } +} + // DerefNonShared finds the indirection of an arc that is not the result of // structure sharing. This is especially relevant when indirecting disjunction // values. diff --git a/internal/core/adt/tasks.go b/internal/core/adt/tasks.go index a95732548e6..00412de8d94 100644 --- a/internal/core/adt/tasks.go +++ b/internal/core/adt/tasks.go @@ -101,6 +101,8 @@ func processResolver(ctx *OpContext, t *task, mode runMode) { // TODO: yield instead? return } + arc = arc.DerefNonDisjunct() + ctx.Logf(t.node.node, "RESOLVED %v to %v %v", r, arc.Label, fmt.Sprintf("%p", arc)) // TODO: consider moving after markCycle or removing. d := arc.DerefDisjunct() diff --git a/internal/core/adt/unify.go b/internal/core/adt/unify.go index f0ec8b40362..44943d46c31 100644 --- a/internal/core/adt/unify.go +++ b/internal/core/adt/unify.go @@ -624,9 +624,13 @@ func (v *Vertex) lookup(c *OpContext, pos token.Pos, f Feature, flags combinedFl // TODO: verify lookup types. arc := v.LookupRaw(f) + // We leave further dereferencing to the caller, but we do dereference for + // the remainder of this function to be able to check the status. + arcReturn := arc if arc != nil { - // TODO: ideally we should leave the dereferencing up to the caller. - arc = arc.DerefNonDisjunct() + arc = arc.DerefNonRooted() + // TODO(perf): NonRooted is the minimum, but consider doing more. + // arc = arc.DerefValue() } // TODO: clean up this logic: @@ -638,7 +642,7 @@ func (v *Vertex) lookup(c *OpContext, pos token.Pos, f Feature, flags combinedFl switch { case arc != nil: if arc.ArcType == ArcMember { - return arc + return arcReturn } arcState = arc.getState(c) @@ -681,7 +685,7 @@ func (v *Vertex) lookup(c *OpContext, pos token.Pos, f Feature, flags combinedFl if task != nil { arcState.addNotify2(task.node.node, task.id) } - return arc + return arcReturn case yield: arcState.process(needs, yield) @@ -696,7 +700,7 @@ func (v *Vertex) lookup(c *OpContext, pos token.Pos, f Feature, flags combinedFl switch arc.ArcType { case ArcMember: - return arc + return arcReturn case ArcOptional, ArcRequired: label := f.SelectorString(c.Runtime)