diff --git a/cmd/cue/cmd/testdata/script/vet_yaml.txtar b/cmd/cue/cmd/testdata/script/vet_yaml.txtar index 3869d248796..e0f5af46dfa 100644 --- a/cmd/cue/cmd/testdata/script/vet_yaml.txtar +++ b/cmd/cue/cmd/testdata/script/vet_yaml.txtar @@ -2,7 +2,7 @@ cmp stderr expect-stderr -- expect-stderr -- -phrases: invalid value "phrases:\n # A quote from Mark Twain.\n quote1:\n lang: en\n attribution: Mark Twain\n\n # A Norwegian proverb.\n proverb:\n lang: no\n text: Stemmen som sier at du ikke klarer det, lyver." (does not satisfy encoding/yaml.Validate({phrases:{},#Phrase:{lang:=~"^[a-zA-Z0-9-_]{2,}$" | false,text:!=""},#LanguageTag:=~"^[a-zA-Z0-9-_]{2,}$" | false})): error in call to encoding/yaml.Validate: incomplete value !="": +phrases: invalid value "phrases:\n # A quote from Mark Twain.\n quote1:\n lang: en\n attribution: Mark Twain\n\n # A Norwegian proverb.\n proverb:\n lang: no\n text: Stemmen som sier at du ikke klarer det, lyver." (does not satisfy encoding/yaml.Validate({phrases:{},#Phrase:{lang:=~"^[a-zA-Z0-9-_]{2,}$" | false,text:!="",},#LanguageTag:=~"^[a-zA-Z0-9-_]{2,}$" | false})): error in call to encoding/yaml.Validate: incomplete value !="": ./yaml.cue:19:10 ./yaml.cue:11:17 ./yaml.cue:21:10 diff --git a/codereview.cfg b/codereview.cfg index 6750f31c7f5..1a92ceb9ef2 100644 --- a/codereview.cfg +++ b/codereview.cfg @@ -1,5 +1,5 @@ # Code generated internal/ci/ci_tool.cue; DO NOT EDIT. -github: https://github.com/cue-lang/cue gerrit: https://review.gerrithub.io/a/cue-lang/cue +github: https://github.com/cue-lang/cue cue-unity: https://github.com/cue-unity/unity diff --git a/cue/query.go b/cue/query.go index af046ee86f9..9c3c2974335 100644 --- a/cue/query.go +++ b/cue/query.go @@ -48,12 +48,16 @@ outer: f := sel.sel.feature(v.idx) for _, a := range n.Arcs { if a.Label == f { + if a.IsConstraint() && !sel.sel.optional() { + break + } parent = linkParent(parent, n, a) n = a continue outer } } if sel.sel.optional() { + // pattern or additional constraints. x := &adt.Vertex{ Parent: n, Label: sel.sel.feature(ctx), diff --git a/cue/testdata/cycle/compbottom2.txtar b/cue/testdata/cycle/compbottom2.txtar index 357e416e64d..7fb176dc399 100644 --- a/cue/testdata/cycle/compbottom2.txtar +++ b/cue/testdata/cycle/compbottom2.txtar @@ -264,14 +264,14 @@ nestedChain: { } -- out/eval/stats -- Leaks: 1 -Freed: 143 -Reused: 134 +Freed: 150 +Reused: 141 Allocs: 10 -Retain: 72 +Retain: 76 -Unifications: 144 -Conjuncts: 159 -Disjuncts: 194 +Unifications: 151 +Conjuncts: 166 +Disjuncts: 205 -- out/eval -- (struct){ self: (struct){ diff --git a/cue/testdata/cycle/structural.txtar b/cue/testdata/cycle/structural.txtar index 27df7fa5588..3481adf9167 100644 --- a/cue/testdata/cycle/structural.txtar +++ b/cue/testdata/cycle/structural.txtar @@ -533,14 +533,14 @@ n3: n1 & {n1} n4: n1 & {x: n1 & {y: n1 & {z: int}}} -- out/eval/stats -- Leaks: 16 -Freed: 792 -Reused: 780 +Freed: 794 +Reused: 782 Allocs: 28 Retain: 65 -Unifications: 622 -Conjuncts: 1219 -Disjuncts: 841 +Unifications: 624 +Conjuncts: 1221 +Disjuncts: 843 -- out/eval -- Errors: a1.f.0: structural cycle diff --git a/cue/testdata/definitions/036_closing_with_failed_optional.txtar b/cue/testdata/definitions/036_closing_with_failed_optional.txtar index c92051388bb..206b3609636 100644 --- a/cue/testdata/definitions/036_closing_with_failed_optional.txtar +++ b/cue/testdata/definitions/036_closing_with_failed_optional.txtar @@ -99,14 +99,14 @@ v1: } -- out/eval/stats -- Leaks: 0 -Freed: 20 -Reused: 15 +Freed: 28 +Reused: 23 Allocs: 5 Retain: 1 -Unifications: 16 -Conjuncts: 31 -Disjuncts: 21 +Unifications: 24 +Conjuncts: 43 +Disjuncts: 29 -- out/eval -- (struct){ #k1: (#struct){ diff --git a/cue/testdata/definitions/issue483.txtar b/cue/testdata/definitions/issue483.txtar index cedf61f2fa3..c97c51447d1 100644 --- a/cue/testdata/definitions/issue483.txtar +++ b/cue/testdata/definitions/issue483.txtar @@ -11,14 +11,14 @@ instance: #Type & { #Root: {...} -- out/eval/stats -- Leaks: 0 -Freed: 11 -Reused: 5 -Allocs: 6 +Freed: 14 +Reused: 7 +Allocs: 7 Retain: 3 -Unifications: 11 -Conjuncts: 31 -Disjuncts: 14 +Unifications: 14 +Conjuncts: 35 +Disjuncts: 17 -- out/eval -- (struct){ out: (#struct){ diff --git a/cue/testdata/disjunctions/elimination.txtar b/cue/testdata/disjunctions/elimination.txtar index 70a9ea1a44b..8d46099d2a0 100644 --- a/cue/testdata/disjunctions/elimination.txtar +++ b/cue/testdata/disjunctions/elimination.txtar @@ -489,14 +489,14 @@ issue2263: full: { -- out/eval/stats -- Leaks: 4 -Freed: 2151 -Reused: 2135 +Freed: 2339 +Reused: 2323 Allocs: 20 Retain: 119 -Unifications: 1196 -Conjuncts: 3205 -Disjuncts: 2270 +Unifications: 1260 +Conjuncts: 3417 +Disjuncts: 2458 -- out/eval -- Errors: issue2209.full.Bar.resource.spec: 6 errors in empty disjunction: diff --git a/cue/testdata/disjunctions/errors.txtar b/cue/testdata/disjunctions/errors.txtar index c4702365132..e160e1c9f0a 100644 --- a/cue/testdata/disjunctions/errors.txtar +++ b/cue/testdata/disjunctions/errors.txtar @@ -33,14 +33,14 @@ explicitDefaultError: { } -- out/eval/stats -- Leaks: 0 -Freed: 39 -Reused: 31 +Freed: 40 +Reused: 32 Allocs: 8 Retain: 0 -Unifications: 27 -Conjuncts: 55 -Disjuncts: 39 +Unifications: 28 +Conjuncts: 56 +Disjuncts: 40 -- out/eval -- Errors: issue516.x: 2 errors in empty disjunction: diff --git a/cue/testdata/eval/closed_disjunction.txtar b/cue/testdata/eval/closed_disjunction.txtar index e73daf8c009..8b248ad1063 100644 --- a/cue/testdata/eval/closed_disjunction.txtar +++ b/cue/testdata/eval/closed_disjunction.txtar @@ -15,14 +15,14 @@ b: #A & { } -- out/eval/stats -- Leaks: 0 -Freed: 32 -Reused: 26 -Allocs: 6 +Freed: 46 +Reused: 39 +Allocs: 7 Retain: 0 -Unifications: 20 -Conjuncts: 46 -Disjuncts: 32 +Unifications: 34 +Conjuncts: 60 +Disjuncts: 46 -- out/eval -- Errors: b: 2 errors in empty disjunction: diff --git a/cue/testdata/eval/issue2146.txtar b/cue/testdata/eval/issue2146.txtar index 33d1fb52f97..0e1103d1210 100644 --- a/cue/testdata/eval/issue2146.txtar +++ b/cue/testdata/eval/issue2146.txtar @@ -41,18 +41,19 @@ p2: { -- out/eval/stats -- Leaks: 37 -Freed: 143 -Reused: 134 -Allocs: 46 -Retain: 154 +Freed: 152 +Reused: 145 +Allocs: 44 +Retain: 77 -Unifications: 164 -Conjuncts: 544 -Disjuncts: 197 +Unifications: 173 +Conjuncts: 557 +Disjuncts: 206 -- out/eval -- (struct){ p1: (struct){ #A: (#struct){ |(*(#struct){ + y: (int){ 1 } let list#1 = (#list){ 0: (_|_){ // [incomplete] p1.#A.list.0: cannot reference optional field: x: @@ -63,7 +64,6 @@ Disjuncts: 197 all: (#list){ 0: (int){ 1 } } - y: (int){ 1 } }, (#struct){ let list#1 = (#list){ 0: (_|_){ @@ -79,6 +79,7 @@ Disjuncts: 197 } }) } a: (#struct){ + x: (int){ 3 } let list#1 = (#list){ 0: (int){ 3 } 1: (_|_){ @@ -89,9 +90,9 @@ Disjuncts: 197 all: (#list){ 0: (int){ 3 } } - x: (int){ 3 } } b: (#struct){ + x: (int){ 3 } let list#1multi = [ 〈1;x〉, 〈1;y〉, @@ -99,11 +100,11 @@ Disjuncts: 197 all: (#list){ 0: (int){ 3 } } - x: (int){ 3 } } } p2: (struct){ #A: (#struct){ |(*(#struct){ + y: (int){ 1 } let list#2 = (#list){ 0: (_|_){ // [incomplete] p2.#A.list.0: cannot reference optional field: x: @@ -114,7 +115,6 @@ Disjuncts: 197 all: (#list){ 0: (int){ 1 } } - y: (int){ 1 } }, (#struct){ let list#2 = (#list){ 0: (_|_){ @@ -130,6 +130,8 @@ Disjuncts: 197 } }) } a: (#struct){ + x: (int){ 3 } + y: (int){ 2 } let list#2 = (#list){ 0: (int){ 3 } 1: (int){ 2 } @@ -138,10 +140,10 @@ Disjuncts: 197 0: (int){ 3 } 1: (int){ 2 } } - x: (int){ 3 } - y: (int){ 2 } } b: (#struct){ + x: (int){ 3 } + y: (int){ 2 } let list#2multi = [ 〈1;x〉, 〈1;y〉, @@ -150,8 +152,6 @@ Disjuncts: 197 0: (int){ 3 } 1: (int){ 2 } } - x: (int){ 3 } - y: (int){ 2 } } } } diff --git a/cue/testdata/eval/issue353.txtar b/cue/testdata/eval/issue353.txtar index 788b4faac29..9ebb87d6b93 100644 --- a/cue/testdata/eval/issue353.txtar +++ b/cue/testdata/eval/issue353.txtar @@ -13,14 +13,14 @@ e: a: "hello" } -- out/eval/stats -- Leaks: 0 -Freed: 11 -Reused: 5 +Freed: 15 +Reused: 9 Allocs: 6 Retain: 0 -Unifications: 7 -Conjuncts: 17 -Disjuncts: 11 +Unifications: 11 +Conjuncts: 21 +Disjuncts: 15 -- out/eval -- (struct){ e: (#struct){ |((#struct){ diff --git a/cue/testdata/fulleval/001_conflicts_in_optional_fields_are_okay_.txtar b/cue/testdata/fulleval/001_conflicts_in_optional_fields_are_okay_.txtar index 1e57b60606c..6e368cb8eac 100644 --- a/cue/testdata/fulleval/001_conflicts_in_optional_fields_are_okay_.txtar +++ b/cue/testdata/fulleval/001_conflicts_in_optional_fields_are_okay_.txtar @@ -36,14 +36,14 @@ c: d & { } -- out/eval/stats -- Leaks: 0 -Freed: 11 -Reused: 6 +Freed: 14 +Reused: 9 Allocs: 5 Retain: 0 -Unifications: 7 -Conjuncts: 13 -Disjuncts: 11 +Unifications: 10 +Conjuncts: 17 +Disjuncts: 14 -- out/eval -- (struct){ d: (struct){ |((struct){ diff --git a/cue/testdata/fulleval/029_Issue_#94.txtar b/cue/testdata/fulleval/029_Issue_#94.txtar index 46e0a4ed213..55779002619 100644 --- a/cue/testdata/fulleval/029_Issue_#94.txtar +++ b/cue/testdata/fulleval/029_Issue_#94.txtar @@ -85,14 +85,14 @@ index: { } -- out/eval/stats -- Leaks: 0 -Freed: 21 -Reused: 18 +Freed: 22 +Reused: 19 Allocs: 3 Retain: 0 -Unifications: 21 -Conjuncts: 31 -Disjuncts: 21 +Unifications: 22 +Conjuncts: 32 +Disjuncts: 22 -- out/eval -- (struct){ foo: (struct){ diff --git a/cue/testdata/references/optional.txtar b/cue/testdata/references/optional.txtar index c0015e53b5c..45171afad36 100644 --- a/cue/testdata/references/optional.txtar +++ b/cue/testdata/references/optional.txtar @@ -14,14 +14,14 @@ a: { } -- out/eval/stats -- Leaks: 0 -Freed: 3 -Reused: 0 +Freed: 4 +Reused: 1 Allocs: 3 Retain: 0 -Unifications: 3 -Conjuncts: 3 -Disjuncts: 3 +Unifications: 4 +Conjuncts: 4 +Disjuncts: 4 -- out/eval -- (struct){ a: (struct){ diff --git a/cue/testdata/resolve/009_optional_field_unification.txtar b/cue/testdata/resolve/009_optional_field_unification.txtar index 27dd17d35d8..928af5fe001 100644 --- a/cue/testdata/resolve/009_optional_field_unification.txtar +++ b/cue/testdata/resolve/009_optional_field_unification.txtar @@ -65,14 +65,14 @@ g1: 1 } -- out/eval/stats -- Leaks: 0 -Freed: 9 -Reused: 6 +Freed: 11 +Reused: 8 Allocs: 3 Retain: 0 -Unifications: 9 -Conjuncts: 16 -Disjuncts: 9 +Unifications: 11 +Conjuncts: 19 +Disjuncts: 11 -- out/eval -- (struct){ a: (struct){ diff --git a/cue/testdata/resolve/010_optional_field_resolves_to_incomplete.txtar b/cue/testdata/resolve/010_optional_field_resolves_to_incomplete.txtar index d18bd5efc74..1a40d127eb3 100644 --- a/cue/testdata/resolve/010_optional_field_resolves_to_incomplete.txtar +++ b/cue/testdata/resolve/010_optional_field_resolves_to_incomplete.txtar @@ -27,14 +27,14 @@ r: { } -- out/eval/stats -- Leaks: 0 -Freed: 4 -Reused: 1 +Freed: 5 +Reused: 2 Allocs: 3 Retain: 0 -Unifications: 4 -Conjuncts: 4 -Disjuncts: 4 +Unifications: 5 +Conjuncts: 5 +Disjuncts: 5 -- out/eval -- (struct){ r: (struct){ diff --git a/cue/testdata/resolve/047_struct_comprehensions.txtar b/cue/testdata/resolve/047_struct_comprehensions.txtar index d303c6450e5..42915e96d74 100644 --- a/cue/testdata/resolve/047_struct_comprehensions.txtar +++ b/cue/testdata/resolve/047_struct_comprehensions.txtar @@ -79,14 +79,14 @@ reg: 4 } -- out/eval/stats -- Leaks: 2 -Freed: 11 -Reused: 5 +Freed: 12 +Reused: 6 Allocs: 8 Retain: 3 -Unifications: 9 -Conjuncts: 18 -Disjuncts: 12 +Unifications: 10 +Conjuncts: 19 +Disjuncts: 13 -- out/eval -- (struct){ obj: (struct){ diff --git a/cue/testdata/scalars/embed.txtar b/cue/testdata/scalars/embed.txtar index 20117c7181b..a8c6c15b3ce 100644 --- a/cue/testdata/scalars/embed.txtar +++ b/cue/testdata/scalars/embed.txtar @@ -152,14 +152,14 @@ selfRefInEmbed: t1: { -- out/eval/stats -- Leaks: 11 -Freed: 120 -Reused: 113 +Freed: 121 +Reused: 114 Allocs: 18 -Retain: 69 +Retain: 70 -Unifications: 127 -Conjuncts: 313 -Disjuncts: 175 +Unifications: 128 +Conjuncts: 314 +Disjuncts: 177 -- out/eval -- Errors: listEmbed.b6: invalid list index 5 (out of bounds): diff --git a/cue/testdata/scalars/emptystruct.txtar b/cue/testdata/scalars/emptystruct.txtar index 1edbcfd05d3..a2e1450075b 100644 --- a/cue/testdata/scalars/emptystruct.txtar +++ b/cue/testdata/scalars/emptystruct.txtar @@ -74,14 +74,14 @@ issue783: { } -- out/eval/stats -- Leaks: 0 -Freed: 52 -Reused: 46 +Freed: 54 +Reused: 48 Allocs: 6 Retain: 0 -Unifications: 46 -Conjuncts: 105 -Disjuncts: 52 +Unifications: 48 +Conjuncts: 107 +Disjuncts: 54 -- out/eval -- (struct){ elipsis: (struct){ diff --git a/cue/types.go b/cue/types.go index b4f6c44f2fe..9e6cce0c632 100644 --- a/cue/types.go +++ b/cue/types.go @@ -113,16 +113,7 @@ func (o *hiddenStructValue) At(i int) (key string, v Value) { func (o *hiddenStructValue) at(i int) (v *adt.Vertex, isOpt bool) { f := o.features[i] arc := o.obj.Lookup(f) - if arc == nil { - arc = &adt.Vertex{ - Parent: o.v.v, - Label: f, - } - o.obj.MatchAndInsert(o.ctx, arc) - arc.Finalize(o.ctx) - isOpt = true - } - return arc, isOpt + return arc, arc.IsConstraint() } // Lookup reports the field for the given key. The returned Value is invalid @@ -1411,19 +1402,12 @@ func (v Value) structValOpts(ctx *adt.OpContext, o options) (s structValue, err if f.IsHidden() && o.omitHidden { continue } - if arc := obj.Lookup(f); arc == nil { - if o.omitOptional { - continue - } - // ensure it really exists. - v := adt.Vertex{ - Parent: obj, - Label: f, - } - obj.MatchAndInsert(ctx, &v) - if len(v.Conjuncts) == 0 { - continue - } + arc := obj.Lookup(f) + if arc == nil { + continue + } + if arc.IsConstraint() && o.omitOptional { + continue } features[k] = f k++ diff --git a/internal/core/adt/composite.go b/internal/core/adt/composite.go index 05b0a390067..7398a6d73da 100644 --- a/internal/core/adt/composite.go +++ b/internal/core/adt/composite.go @@ -230,12 +230,24 @@ type Vertex struct { Structs []*StructInfo } +// UpdateArcType updates v.ArcType if t is more restrictive. +func (v *Vertex) UpdateArcType(t ArcType) { + if t < v.ArcType { + v.ArcType = t + } +} + // isDefined indicates whether this arc is a "value" field, and not a constraint // or void arc. func (v *Vertex) isDefined() bool { return v.ArcType == ArcMember } +// IsConstraint reports whether the Vertex is an optional or required field. +func (v *Vertex) IsConstraint() bool { + return v.ArcType == ArcOptional +} + // IsDefined indicates whether this arc is defined meaning it is not a // required or optional constraint and not a "void" arc. // It will evaluate the arc, and thus evaluate any comprehension, to make this @@ -255,6 +267,9 @@ const ( // (including regular, hidden, and definition fields). ArcMember ArcType = iota + // ArcOptional represents fields of the form foo?. + ArcOptional + // TODO: define a type for optional arcs. This will be needed for pulling // in optional fields into the Vertex, which, in turn, is needed for // structure sharing, among other things. @@ -624,9 +639,9 @@ func (v *Vertex) OptionalTypes() OptionalType { // IsOptional reports whether a field is explicitly defined as optional, // as opposed to whether it is allowed by a pattern constraint. func (v *Vertex) IsOptional(label Feature) bool { - for _, s := range v.Structs { - if s.IsOptionalField(label) { - return true + for _, a := range v.Arcs { + if a.Label == label { + return a.IsConstraint() } } return false @@ -752,6 +767,7 @@ func (v *Vertex) Elems() []*Vertex { func (v *Vertex) GetArc(c *OpContext, f Feature, t ArcType) (arc *Vertex, isNew bool) { arc = v.Lookup(f) if arc != nil { + arc.UpdateArcType(t) return arc, false } diff --git a/internal/core/adt/context.go b/internal/core/adt/context.go index b15d56c096e..ce98a8ef47b 100644 --- a/internal/core/adt/context.go +++ b/internal/core/adt/context.go @@ -863,6 +863,20 @@ func (c *OpContext) lookup(x *Vertex, pos token.Pos, l Feature, state VertexStat } else if a.state != nil { c.Unify(a, Partial) } + + if a.IsConstraint() { + code := IncompleteError + if hasCycle { + code = CycleError + } + label := l.SelectorString(c.Runtime) + c.AddBottom(&Bottom{ + Code: code, + Permanent: x.status >= Conjuncts, + Err: c.NewPosf(pos, + "cannot reference optional field: %s", label), + }) + } } else { if x.state != nil { for _, e := range x.state.exprs { @@ -895,12 +909,7 @@ func (c *OpContext) lookup(x *Vertex, pos token.Pos, l Feature, state VertexStat err = c.NewPosf(pos, "index out of range [%d] with length %d", l.Index(), len(x.Elems())) } else { - if code != 0 && x.IsOptional(l) { - err = c.NewPosf(pos, - "cannot reference optional field: %s", label) - } else { - err = c.NewPosf(pos, "undefined field: %s", label) - } + err = c.NewPosf(pos, "undefined field: %s", label) } c.AddBottom(&Bottom{ Code: code, diff --git a/internal/core/adt/disjunct.go b/internal/core/adt/disjunct.go index 5264e806f52..4d0b94ca136 100644 --- a/internal/core/adt/disjunct.go +++ b/internal/core/adt/disjunct.go @@ -388,6 +388,15 @@ func (n *nodeContext) expandDisjuncts( outer: for _, d := range n.disjuncts { for k, v := range p.disjuncts { + // As long as a vertex isn't finalized, it may be that potential + // errors are not yet detected. This may lead two structs that + // are identical except for closedness information, + // for instance, to appear identical. + if v.result.status < Finalized || d.result.status < Finalized { + break + } + // Even if a node is finalized, it may still have an + // "incomplete" component that may change down the line. if !d.done() || !v.done() { break } diff --git a/internal/core/adt/equality.go b/internal/core/adt/equality.go index dd2e7c65730..dcb8600a8f4 100644 --- a/internal/core/adt/equality.go +++ b/internal/core/adt/equality.go @@ -45,6 +45,9 @@ func equalVertex(ctx *OpContext, x *Vertex, v Value, flags Flag) bool { if x == y { return true } + if x.ArcType != y.ArcType { + return false + } xk := x.Kind() yk := y.Kind() @@ -52,8 +55,11 @@ func equalVertex(ctx *OpContext, x *Vertex, v Value, flags Flag) bool { return false } - if len(x.Arcs) != len(y.Arcs) { - return false + maxArcType := ArcMember + if flags&CheckStructural != 0 { + // Do not ignore optional fields + // TODO(required): consider making this unconditional + maxArcType = ArcOptional } // TODO: this really should be subsumption. @@ -68,15 +74,12 @@ func equalVertex(ctx *OpContext, x *Vertex, v Value, flags Flag) bool { loop1: for _, a := range x.Arcs { - if !a.IsDefined(ctx) { + if a.ArcType > maxArcType { continue } for _, b := range y.Arcs { - if !b.IsDefined(ctx) { - continue - } if a.Label == b.Label { - if !Equal(ctx, a, b, flags) { + if a.ArcType != b.ArcType || !Equal(ctx, a, b, flags) { return false } continue loop1 @@ -85,16 +88,24 @@ loop1: return false } - // We do not need to do the following check, because of the pigeon-hole principle. - // loop2: - // for _, b := range y.Arcs { - // for _, a := range x.Arcs { - // if a.Label == b.Label { - // continue loop2 - // } - // } - // return false - // } +loop2: + for _, b := range y.Arcs { + if b.ArcType > maxArcType { + continue + } + for _, a := range x.Arcs { + if a.Label == b.Label { + if a.ArcType > maxArcType { + // No need to continue: arc with label not found. + break + } + // Label found. Equality was already tested in loop 1. + continue loop2 + } + } + // Arc with same label not found. + return false + } v, ok1 := x.BaseValue.(Value) w, ok2 := y.BaseValue.(Value) diff --git a/internal/core/adt/eval.go b/internal/core/adt/eval.go index fe7d9e8eb26..9b7bbfbc545 100644 --- a/internal/core/adt/eval.go +++ b/internal/core/adt/eval.go @@ -78,9 +78,9 @@ func (c *OpContext) evaluate(v *Vertex, r Resolver, state VertexStatus) Value { // Use node itself to allow for cycle detection. c.Unify(v, state) - if !v.isDefined() { + if v.ArcType == ArcVoid { if v.status == Evaluating { - for ; v.Parent != nil && !v.isDefined(); v = v.Parent { + for ; v.Parent != nil && v.ArcType == ArcVoid; v = v.Parent { } err := c.Newf("cycle with field %v", r) b := &Bottom{Code: CycleError, Err: err} @@ -750,17 +750,17 @@ func (n *nodeContext) completeArcs(state VertexStatus) { // correctly and that we are not regressing. n.node.UpdateStatus(EvaluatingArcs) - wasVoid := !a.isDefined() + wasVoid := a.ArcType == ArcVoid ctx.Unify(a, Finalized) - if !a.isDefined() { + if a.ArcType == ArcVoid { continue } // Errors are allowed in let fields. Handle errors and failure to // complete accordingly. - if !a.Label.IsLet() { + if !a.Label.IsLet() && a.ArcType == ArcMember { // Don't set the state to Finalized if the child arcs are not done. if state == Finalized && a.status < Finalized { state = Conjuncts @@ -1717,7 +1717,7 @@ func (n *nodeContext) addValueConjunct(env *Environment, v Value, id CloseInfo) n.node.Structs = append(n.node.Structs, x.Structs...) for _, a := range x.Arcs { - if !a.IsDefined(ctx) { + if a.ArcType == ArcVoid { continue } // TODO(errors): report error when this is a regular field. @@ -1906,7 +1906,7 @@ func (n *nodeContext) addStruct( for _, d := range s.Decls { switch x := d.(type) { - case *Field, *LetField: + case *Field, *OptionalField, *LetField: // handle in next iteration. case *DynamicField: @@ -1927,7 +1927,7 @@ func (n *nodeContext) addStruct( // push and opo embedding type. n.addExprConjunct(MakeConjunct(childEnv, x, id), Partial) - case *OptionalField, *BulkOptionalField, *Ellipsis: + case *BulkOptionalField, *Ellipsis: // Nothing to do here. Note that the presence of these fields do not // excluded embedded scalars: only when they match actual fields // does it exclude those. @@ -1953,6 +1953,9 @@ func (n *nodeContext) addStruct( } n.insertField(x.Label, ArcMember, MakeConjunct(childEnv, x, closeInfo)) + case *OptionalField: + n.insertField(x.Label, ArcOptional, MakeConjunct(childEnv, x, closeInfo)) + case *LetField: arc := n.insertField(x.Label, ArcMember, MakeConjunct(childEnv, x, closeInfo)) if x.IsMulti { diff --git a/internal/core/adt/expr.go b/internal/core/adt/expr.go index a9e5474ce5c..84e2f2e619b 100644 --- a/internal/core/adt/expr.go +++ b/internal/core/adt/expr.go @@ -62,12 +62,11 @@ func (o *StructLit) IsFile() bool { } type FieldInfo struct { - Label Feature - Optional []Node + Label Feature } func (x *StructLit) HasOptional() bool { - return x.types&(HasField|HasPattern|HasAdditional) != 0 + return x.types&(HasPattern|HasAdditional) != 0 } func (x *StructLit) Source() ast.Node { return x.Src } @@ -107,10 +106,8 @@ func (o *StructLit) Init() { case *OptionalField: p := o.fieldIndex(x.Label) if p < 0 { - p = len(o.Fields) o.Fields = append(o.Fields, FieldInfo{Label: x.Label}) } - o.Fields[p].Optional = append(o.Fields[p].Optional, x) o.types |= HasField case *LetField: @@ -172,15 +169,6 @@ func (o *StructLit) OptionalTypes() OptionalType { return o.types } -func (o *StructLit) IsOptionalField(label Feature) bool { - for _, f := range o.Fields { - if f.Label == label && len(f.Optional) > 0 { - return true - } - } - return false -} - // FIELDS // // Fields can also be used as expressions whereby the value field is the diff --git a/internal/core/adt/optional.go b/internal/core/adt/optional.go index 573c6b23b8b..3606a6dae75 100644 --- a/internal/core/adt/optional.go +++ b/internal/core/adt/optional.go @@ -25,14 +25,12 @@ func (o *StructInfo) MatchAndInsert(c *OpContext, arc *Vertex) { // Match normal fields matched := false -outer: + // TODO: this could be lookup up more efficiently in the outer Vertex now. + // Keep this logic for now, though. for _, f := range o.Fields { if f.Label == arc.Label { - for _, e := range f.Optional { - arc.AddConjunct(MakeConjunct(env, e, closeInfo)) - } matched = true - break outer + break } } diff --git a/internal/core/adt/optional_test.go b/internal/core/adt/optional_test.go index c9d35b219f5..efc898de3cd 100644 --- a/internal/core/adt/optional_test.go +++ b/internal/core/adt/optional_test.go @@ -46,11 +46,6 @@ func TestOptionalTypes(t *testing.T) { "\(bar)": int `, out: adt.HasPattern | adt.HasDynamic, - }, { - in: ` - bar?: 3 - `, - out: adt.HasField, }} for _, tc := range testCases { t.Run("", func(t *testing.T) { diff --git a/internal/core/debug/compact.go b/internal/core/debug/compact.go index 87110a25c74..a9a4a87712c 100644 --- a/internal/core/debug/compact.go +++ b/internal/core/debug/compact.go @@ -62,8 +62,12 @@ func (w *compactPrinter) node(n adt.Node) { continue } w.node(a) - } else { + } else if !a.IsConstraint() { w.label(a.Label) + // TODO: remove if !a.IsConstraint() + if a.IsConstraint() { + w.string("?") + } w.string(":") w.node(a) } diff --git a/internal/core/debug/debug.go b/internal/core/debug/debug.go index 59084825b42..618775edc8b 100644 --- a/internal/core/debug/debug.go +++ b/internal/core/debug/debug.go @@ -221,8 +221,8 @@ func (w *printer) node(n adt.Node) { } for _, a := range x.Arcs { - w.string("\n") if a.Label.IsLet() { + w.string("\n") w.string("let ") w.label(a.Label) if a.MultiLet { @@ -234,8 +234,13 @@ func (w *printer) node(n adt.Node) { continue } w.node(a) - } else { + } else if !a.IsConstraint() { + // TODO: also show constraints. + w.string("\n") w.label(a.Label) + if a.IsConstraint() { + w.string("?") + } w.string(": ") w.node(a) } diff --git a/internal/core/export/testdata/main/adt.txtar b/internal/core/export/testdata/main/adt.txtar index 60d9826652c..4a5d412fd2a 100644 --- a/internal/core/export/testdata/main/adt.txtar +++ b/internal/core/export/testdata/main/adt.txtar @@ -292,7 +292,11 @@ errorListDef: { [e5] [e6] [e7] +[e8] [m1] +[m1 foo] +- foo is an optional field + [m1 bar] - bar is a field diff --git a/internal/core/export/testdata/main/alias.txtar b/internal/core/export/testdata/main/alias.txtar index 08921a61144..747ac32e47e 100644 --- a/internal/core/export/testdata/main/alias.txtar +++ b/internal/core/export/testdata/main/alias.txtar @@ -133,6 +133,7 @@ issue1308: { [fieldAlias simple] [fieldAlias simple "a-b"] [fieldAlias simple foo] +[fieldAlias simple bar] [fieldAlias simple "a-c"] [fieldAlias cross] [fieldAlias cross baz] @@ -234,10 +235,10 @@ was known to compile and is known to be correct. { fieldAlias: { simple: { - "a-b": 4 - foo: 4 - bar?: Y_1 - Y_1="a-c": 5 + "a-b": 4 + foo: 4 + bar?: 5 + "a-c": 5 } cross: { baz: 3 @@ -338,10 +339,10 @@ was known to compile and is known to be correct. { fieldAlias: { simple: { - "a-b": 4 - foo: 4 - bar?: Y_1 - Y_1="a-c": 5 + "a-b": 4 + foo: 4 + bar?: 5 + "a-c": 5 } cross: { baz: 3 @@ -398,10 +399,10 @@ was known to compile and is known to be correct. { fieldAlias: { simple: { - "a-b": 4 - foo: 4 - bar?: Y_1 - Y_1="a-c": 5 + "a-b": 4 + foo: 4 + bar?: 5 + "a-c": 5 } cross: { baz: 3 diff --git a/internal/core/export/testdata/main/attrs.txtar b/internal/core/export/testdata/main/attrs.txtar index 82557254e44..fbd2859b02a 100644 --- a/internal/core/export/testdata/main/attrs.txtar +++ b/internal/core/export/testdata/main/attrs.txtar @@ -198,9 +198,11 @@ dynamicSimple: { [comprehensions c3] [dynamicComplex] [dynamicComplex a] +[dynamicComplex b] [dynamicComplex foo] [dynamicSimple] [dynamicSimple a] +[dynamicSimple b] -- out/value -- == Simplified { diff --git a/internal/core/export/testdata/main/def.txtar b/internal/core/export/testdata/main/def.txtar index 7c5890d7a6b..397aafae4be 100644 --- a/internal/core/export/testdata/main/def.txtar +++ b/internal/core/export/testdata/main/def.txtar @@ -11,6 +11,7 @@ c: { -- out/doc -- [] [a] +[b] [c] -- out/value -- == Simplified diff --git a/internal/core/export/testdata/main/issue662.txtar b/internal/core/export/testdata/main/issue662.txtar index 84fa8590ac9..53f23bf4266 100644 --- a/internal/core/export/testdata/main/issue662.txtar +++ b/internal/core/export/testdata/main/issue662.txtar @@ -17,7 +17,10 @@ -- out/doc -- [] [#LineConfig] +[#LineConfig lineColor] [#GraphFieldConfig] +[#GraphFieldConfig lineColor] +[#GraphFieldConfig drawStyle] -- out/value -- == Simplified {} @@ -28,6 +31,7 @@ } #GraphFieldConfig: { lineColor?: string + drawStyle?: int } } == Final @@ -39,6 +43,7 @@ } #GraphFieldConfig: { lineColor?: string + drawStyle?: int } } == Eval @@ -48,5 +53,6 @@ } #GraphFieldConfig: { lineColor?: string + drawStyle?: int } } diff --git a/internal/core/export/testdata/main/simplify.txtar b/internal/core/export/testdata/main/simplify.txtar index d47530c4b34..a28e3c9e91e 100644 --- a/internal/core/export/testdata/main/simplify.txtar +++ b/internal/core/export/testdata/main/simplify.txtar @@ -6,6 +6,14 @@ x: { y: int } s: strings.MinRunes(4) & strings.MaxRunes(7) + +additional: { + ... + env: _ + confs: { + if env {} + } +} -- out/definition -- import "strings" @@ -16,11 +24,21 @@ x: { y: int } s: strings.MinRunes(4) & strings.MaxRunes(7) +additional: { + env: _ + confs: { + if env {} + } + ... +} -- out/doc -- [] [x] [x y] [s] +[additional] +[additional env] +[additional confs] -- out/value -- == Simplified { @@ -28,6 +46,12 @@ s: strings.MinRunes(4) & strings.MaxRunes(7) y: int64 } s: strings.MinRunes(4) & strings.MaxRunes(7) + additional: { + env: _ + confs: { + if env {} + } + } } == Raw { @@ -35,6 +59,12 @@ s: strings.MinRunes(4) & strings.MaxRunes(7) y: >=-9223372036854775808 & <=9223372036854775807 & int } s: strings.MinRunes(4) & strings.MaxRunes(7) + additional: { + env: _ + confs: { + if env {} + } + } } == Final { @@ -42,6 +72,10 @@ s: strings.MinRunes(4) & strings.MaxRunes(7) y: int64 } s: strings.MinRunes(4) & strings.MaxRunes(7) + additional: { + env: _ + confs: _|_ // additional.confs: incomplete bool: _ (and 2 more errors) + } } == All { @@ -49,6 +83,12 @@ s: strings.MinRunes(4) & strings.MaxRunes(7) y: int64 } s: strings.MinRunes(4) & strings.MaxRunes(7) + additional: { + env: _ + confs: { + if env {} + } + } } == Eval { @@ -56,4 +96,10 @@ s: strings.MinRunes(4) & strings.MaxRunes(7) y: >=-9223372036854775808 & <=9223372036854775807 & int } s: strings.MinRunes(4) & strings.MaxRunes(7) + additional: { + env: _ + confs: { + if env {} + } + } } diff --git a/internal/core/export/value.go b/internal/core/export/value.go index 706f0df3756..d6fe1cc2dd9 100644 --- a/internal/core/export/value.go +++ b/internal/core/export/value.go @@ -73,6 +73,11 @@ func (e *exporter) vertex(n *adt.Vertex) (result ast.Expr) { case *adt.Bottom: switch { + case n.IsConstraint(): + // Constraints may always be the original value. + // TODO: this was included for backwards compatibility. But + // should we show the error here? It signifies that this field + // may not be used. case e.cfg.ShowErrors && x.ChildError: // TODO(perf): use precompiled arc statistics if len(n.Arcs) > 0 && n.Arcs[0].Label.IsInt() && !e.showArcs(n) && attrs == nil { @@ -118,22 +123,6 @@ func (e *exporter) vertex(n *adt.Vertex) (result ast.Expr) { return result } -// TODO: do something more principled. Best would be to have a similar -// mechanism in ast.Ident as others do. -func stripRefs(x ast.Expr) ast.Expr { - ast.Walk(x, nil, func(n ast.Node) { - switch x := n.(type) { - case *ast.Ident: - switch x.Node.(type) { - case *ast.ImportSpec: - default: - x.Node = nil - } - } - }) - return x -} - func (e *exporter) value(n adt.Value, a ...adt.Conjunct) (result ast.Expr) { if e.cfg.TakeDefaults { n = adt.Default(n) @@ -444,29 +433,19 @@ func (e *exporter) structComposite(v *adt.Vertex, attrs []*ast.Attribute) ast.Ex } arc := v.Lookup(label) - switch { - case arc == nil: + if arc == nil { + continue + } + + if isOptional := arc.IsConstraint(); isOptional { if !p.ShowOptional { continue } f.Optional = token.NoSpace.Pos() - - arc = &adt.Vertex{Label: label} - v.MatchAndInsert(e.ctx, arc) - if len(arc.Conjuncts) == 0 { - continue - } - - // fall back to expression mode. - f.Value = stripRefs(e.expr(nil, arc)) - - // TODO: remove use of stripRefs. - // f.Value = e.expr(arc) - - default: - f.Value = e.vertex(arc) } + f.Value = e.vertex(arc) + if label.IsDef() { e.inDefinition-- } diff --git a/internal/core/export/value_test.go b/internal/core/export/value_test.go index 849fd404e4c..c8baa64d227 100644 --- a/internal/core/export/value_test.go +++ b/internal/core/export/value_test.go @@ -92,6 +92,8 @@ func TestValueX(t *testing.T) { -- in.cue -- ` + adt.Verbosity = 1 + archive := txtar.Parse([]byte(in)) a := cuetxtar.Load(archive, t.TempDir()) diff --git a/internal/core/subsume/vertex.go b/internal/core/subsume/vertex.go index 121b7caa91d..3f648f19550 100644 --- a/internal/core/subsume/vertex.go +++ b/internal/core/subsume/vertex.go @@ -31,6 +31,9 @@ func (s *subsumer) vertices(x, y *adt.Vertex) bool { if x == y { return true } + if x.ArcType < y.ArcType { + return false + } if s.Defaults { y = y.Default() @@ -124,6 +127,17 @@ func (s *subsumer) vertices(x, y *adt.Vertex) bool { continue } + aOpt = true + } else if a.IsConstraint() { + if s.IgnoreOptional { + continue + } + // If field a is optional and has value top, neither the + // omission of the field nor the field defined with any value + // may cause unification to fail. + if a.Kind() == adt.TopKind { + continue + } aOpt = true } @@ -187,6 +201,10 @@ outer: b = &adt.Vertex{Label: f} y.MatchAndInsert(ctx, b) + } else if b.IsConstraint() { + if s.IgnoreOptional || s.Final { + continue + } } if !x.Accept(ctx, f) { diff --git a/internal/core/validate/validate.go b/internal/core/validate/validate.go index aaf46916427..c702f14bdcc 100644 --- a/internal/core/validate/validate.go +++ b/internal/core/validate/validate.go @@ -98,7 +98,7 @@ func (v *validator) validate(x *adt.Vertex) { } for _, a := range x.Arcs { - if a.Label.IsLet() { + if a.Label.IsLet() || !a.IsDefined(v.ctx) { continue } if !v.AllErrors && v.err != nil { diff --git a/pkg/internal/types.go b/pkg/internal/types.go index 8b2d3373173..6d9201fc67f 100644 --- a/pkg/internal/types.go +++ b/pkg/internal/types.go @@ -63,7 +63,7 @@ func (s *Struct) IsOpen() bool { return true } ot := s.node.OptionalTypes() - if ot&^(adt.HasField|adt.HasDynamic) != 0 { + if ot&^adt.HasDynamic != 0 { return true } return false diff --git a/tools/flow/testdata/template.txtar b/tools/flow/testdata/template.txtar index e2b9e2e0291..9a605bb8217 100644 --- a/tools/flow/testdata/template.txtar +++ b/tools/flow/testdata/template.txtar @@ -48,14 +48,14 @@ graph TD } -- out/run/t1/stats -- Leaks: 0 -Freed: 41 -Reused: 34 +Freed: 42 +Reused: 35 Allocs: 7 Retain: 0 -Unifications: 24 -Conjuncts: 64 -Disjuncts: 41 +Unifications: 25 +Conjuncts: 65 +Disjuncts: 42 -- out/run/t2 -- graph TD t0("root.get [Terminated]") @@ -90,24 +90,24 @@ graph TD } -- out/run/t2/stats -- Leaks: 0 -Freed: 41 -Reused: 41 +Freed: 42 +Reused: 42 Allocs: 0 Retain: 0 -Unifications: 24 -Conjuncts: 68 -Disjuncts: 41 +Unifications: 25 +Conjuncts: 69 +Disjuncts: 42 -- out/run/stats/totals -- Leaks: 0 -Freed: 82 -Reused: 75 +Freed: 84 +Reused: 77 Allocs: 7 Retain: 0 -Unifications: 48 -Conjuncts: 132 -Disjuncts: 82 +Unifications: 50 +Conjuncts: 134 +Disjuncts: 84 -- out/run/t3 -- graph TD t0("root.get [Terminated]")