this repo has no description
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

cue: fix regression in merge

This reimplements the template stripper.

It is considerably more code, but easier to understand,
more performant, but more importantly, easier to debug.

Fixes #38.

Change-Id: Id4e6988941b43e47f02cbbd3c2fdfdc10ac2ffa7
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/1880
Reviewed-by: Marcel van Lohuizen <mpvl@google.com>

authored by

Marcel van Lohuizen and committed by
Marcel van Lohuizen
6c58f258 ef48a36d

+119 -91
+3 -8
cue/instance.go
··· 17 17 import ( 18 18 "cuelang.org/go/cue/ast" 19 19 "cuelang.org/go/cue/build" 20 - "cuelang.org/go/cue/token" 21 20 "cuelang.org/go/internal" 22 21 "golang.org/x/exp/errors/fmt" 23 22 ) ··· 146 145 return inst[0] 147 146 } 148 147 148 + values := []value{} 149 149 for _, i := range inst { 150 150 if i.Err != nil { 151 151 return i 152 152 } 153 + values = append(values, i.rootValue) 153 154 } 155 + merged := &mergedValues{values: values} 154 156 155 157 ctx := inst[0].newContext() 156 - 157 - merged := stripTemplates(ctx, inst[0].rootValue) 158 - 159 - for _, i := range inst[1:] { 160 - x := stripTemplates(ctx, i.rootValue) 161 - merged = mkBin(ctx, token.NoPos, opUnify, merged, x) 162 - } 163 158 164 159 st, ok := ctx.manifest(merged).(*structLit) 165 160 if !ok {
+14
cue/instance_test.go
··· 82 82 ), 83 83 out: `{obj:{alpha:{a:A,b:2},beta:{a:B,b:3}}}`, 84 84 }, { 85 + desc: "top-level comprehensions", 86 + instances: insts(` 87 + t: {"\(k)": 10 for k, x in s} 88 + s <Name>: {} 89 + s foo a: 1 90 + `, 91 + ` 92 + t: {"\(k)": 10 for k, x in s} 93 + s <Name>: {} 94 + s bar b: 2 95 + `, 96 + ), 97 + out: `{t:{foo:10,bar:10},s:{foo:{a:1},bar:{b:2}}}`, 98 + }, { 85 99 desc: "error", 86 100 instances: insts(`a:`), 87 101 out: `{}`,
+102 -22
cue/strip.go
··· 14 14 15 15 package cue 16 16 17 - // This file defines a rewriter that strips a fully evaluated value of its 18 - // template. 17 + import ( 18 + "sort" 19 + ) 19 20 20 - // TODO: currently strip templates does a full evaluation as it is hard to keep 21 - // nodeRef and copied structs in sync. This is far from efficient, but it is the 22 - // easiest to get correct. 23 - 24 - // stripTemplates evaluates v and strips the result of templates. 25 - func stripTemplates(ctx *context, v value) value { 26 - return rewrite(ctx, v, stripRewriter) 21 + // A mergedValues type merges structs without unifying their templates. 22 + // It evaluates structs in parallel and then creates a new mergedValues 23 + // for each duplicate arc. The mergedValues do not reappear once there is 24 + // only a single value per arc. 25 + // 26 + // This is used to merge different instances which may have incompatible 27 + // specializations, but have disjuncts objects that may otherwise be shared 28 + // in the same namespace. 29 + type mergedValues struct { 30 + baseValue 31 + values []value 27 32 } 28 33 29 - func stripRewriter(ctx *context, v value) (value, bool) { 30 - eval := ctx.manifest(v) 31 - switch x := eval.(type) { 32 - case *structLit: 33 - x = x.expandFields(ctx) 34 - if x.template != nil { 35 - arcs := make(arcs, len(x.arcs)) 36 - for i, a := range x.arcs { 37 - a.setValue(rewrite(ctx, x.at(ctx, i), stripRewriter)) 38 - arcs[i] = a 34 + func (x *mergedValues) evalPartial(ctx *context) evaluated { 35 + var structs []*structLit 36 + for _, v := range x.values { 37 + v = v.evalPartial(ctx) 38 + o, ok := v.(*structLit) 39 + if !ok { 40 + v := x.values[0] 41 + for _, w := range x.values[1:] { 42 + v = mkBin(ctx, w.Pos(), opUnify, v, w) 39 43 } 40 - // TODO: verify that len(x.comprehensions) == 0 41 - return &structLit{x.baseValue, x.emit, nil, nil, arcs, nil}, false 44 + return v.evalPartial(ctx) 45 + } 46 + o = o.expandFields(ctx) 47 + structs = append(structs, o) 48 + } 49 + 50 + // Pre-expand the arcs so that we can discard the templates. 51 + obj := &structLit{ 52 + baseValue: structs[0].baseValue, 53 + } 54 + var arcs arcs 55 + for _, v := range structs { 56 + for i := 0; i < len(v.arcs); i++ { 57 + w := v.iterAt(ctx, i) 58 + arcs = append(arcs, w) 42 59 } 43 60 } 44 - return eval, true 61 + obj.arcs = arcs 62 + sort.Stable(obj) 63 + 64 + values := []value{} 65 + for _, v := range structs { 66 + if v.emit != nil { 67 + values = append(values, v.emit) 68 + } 69 + } 70 + switch len(values) { 71 + case 0: 72 + case 1: 73 + obj.emit = values[0] 74 + default: 75 + obj.emit = &mergedValues{values[0].base(), values} 76 + } 77 + 78 + // merge arcs 79 + k := 0 80 + for i := 0; i < len(arcs); k++ { 81 + a := arcs[i] 82 + // TODO: consider storing the evaluated value. This is a performance 83 + // versus having more information tradeoff. It results in the same 84 + // value. 85 + values := []value{a.v} 86 + for i++; i < len(arcs) && a.feature == arcs[i].feature; i++ { 87 + values = append(values, arcs[i].v) 88 + a.optional = a.optional && arcs[i].optional 89 + var err evaluated 90 + a.attrs, err = unifyAttrs(ctx, a.v, a.attrs, arcs[i].attrs) 91 + if err != nil { 92 + return err 93 + } 94 + } 95 + if len(values) == 1 { 96 + arcs[k] = a 97 + continue 98 + } 99 + a.cache = nil 100 + a.v = &mergedValues{a.v.base(), values} 101 + arcs[k] = a 102 + } 103 + obj.arcs = arcs[:k] 104 + return obj 105 + } 106 + 107 + func (x *mergedValues) kind() kind { 108 + k := x.values[0].kind() 109 + for _, v := range x.values { 110 + k = unifyType(k, v.kind()) 111 + } 112 + return k 113 + } 114 + 115 + func (x *mergedValues) rewrite(ctx *context, fn rewriteFunc) value { 116 + vs := make([]value, len(x.values)) 117 + for i, v := range x.values { 118 + vs[i] = rewrite(ctx, v, fn) 119 + } 120 + return &mergedValues{x.baseValue, vs} 121 + } 122 + 123 + func (x *mergedValues) subsumesImpl(ctx *context, v value, mode subsumeMode) bool { 124 + return false 45 125 }
-61
cue/strip_test.go
··· 1 - // Copyright 2018 The CUE Authors 2 - // 3 - // Licensed under the Apache License, Version 2.0 (the "License"); 4 - // you may not use this file except in compliance with the License. 5 - // You may obtain a copy of the License at 6 - // 7 - // http://www.apache.org/licenses/LICENSE-2.0 8 - // 9 - // Unless required by applicable law or agreed to in writing, software 10 - // distributed under the License is distributed on an "AS IS" BASIS, 11 - // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. 12 - // See the License for the specific language governing permissions and 13 - // limitations under the License. 14 - 15 - package cue 16 - 17 - import "testing" 18 - 19 - func TestStripTemplates(t *testing.T) { 20 - testCases := []testCase{{ 21 - desc: "basic", 22 - in: ` 23 - foo <Name>: { name: Name } 24 - foo bar: { units: 5 } 25 - `, 26 - out: `<0>{foo: <1>{bar: <2>{name: "bar", units: 5}}}`, 27 - }, { 28 - desc: "top-level template", 29 - in: ` 30 - <Name>: { name: Name } 31 - bar: { units: 5 } 32 - `, 33 - out: `<0>{bar: <1>{name: "bar", units: 5}}`, 34 - }, { 35 - desc: "with reference", 36 - in: ` 37 - before: foo.bar 38 - foo <Name>: { name: Name } 39 - foo bar: { units: 5 } 40 - after: foo.bar 41 - `, 42 - out: `<0>{before: <1>{name: "bar", units: 5}, foo: <2>{bar: <3>{name: "bar", units: 5}}, after: <4>{name: "bar", units: 5}}`, 43 - }, { 44 - desc: "nested", 45 - in: ` 46 - <X1> foo <X2> bar <X3>: { name: X1+X2+X3 } 47 - a foo b bar c: { morning: true } 48 - `, 49 - out: `<0>{a: <1>{foo: <2>{b: <3>{bar: <4>{c: <5>{name: "abc", morning: true}}}}}}`}} 50 - for _, tc := range testCases { 51 - t.Run("", func(t *testing.T) { 52 - ctx, obj := compileFile(t, tc.in) 53 - 54 - v := stripTemplates(ctx, obj) 55 - 56 - if got := debugStr(ctx, v); got != tc.out { 57 - t.Errorf("output differs:\ngot %s\nwant %s", got, tc.out) 58 - } 59 - }) 60 - } 61 - }