Skip to content
This repository has been archived by the owner on Nov 18, 2021. It is now read-only.

Commit

Permalink
cue: don't turn of manifesting in normalization
Browse files Browse the repository at this point in the history
The problem is that the cached results of partially
evaluated values end can cause spurious "incomplete"
errors later in evaluation.

The orignal comment as to why this was done does
not (any longer) apply: referenced structs are always
copied with a cleared cache. So it is safe to cache the
values now.

Change-Id: Ic79e0231190ca3c2c017a8850d236f01509fbbd7
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/3960
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
  • Loading branch information
mpvl committed Nov 7, 2019
1 parent fa59c10 commit 5c2d937
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 11 deletions.
2 changes: 0 additions & 2 deletions cue/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ type context struct {
inSum int
cycleErr bool

noManifest bool

// for debug strings
nodeRefs map[scope]string

Expand Down
3 changes: 0 additions & 3 deletions cue/evaluator.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,6 @@ package cue

func (c *context) manifest(v value) evaluated {
evaluated := v.evalPartial(c)
if c.noManifest {
return evaluated
}
outer:
switch x := evaluated.(type) {
case *disjunction:
Expand Down
17 changes: 17 additions & 0 deletions cue/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2733,6 +2733,23 @@ func TestFullEval(t *testing.T) {
`c: <9>C{C{[=~"^Q*$"]: <10>(_: string)->int}, C{(C{[=~"^[a-s]*$"]: <11>(_: string)->int} & C{[=~"^[m-z]*?"]: <12>(_: string)->int})}, QQ: 3}, ` +
`D :: <13>C{[=~"^[a-s]*$"]: <14>(_: string)->int, [=~"^[m-z]*?"]: <15>(_: string)->int, }, ` +
`d: <16>C{[=~"^[a-s]*$"]: <17>(_: string)->int, [=~"^[m-z]*?"]: <18>(_: string)->int, aaa: 4}}`,
}, {
in: `
Task :: {
{
op: "pull"
tag: *"latest" | string
refToTag: tag
tagExpr: tag + "dd"
tagInString: "\(tag)"
} | {
op: "scratch"
}
}
foo: Task & {"op": "pull"}
`,
out: `<0>{Task :: (<1>C{op: "pull", tag: (*"latest" | string), refToTag: <1>.tag, tagExpr: (<1>.tag + "dd"), tagInString: ""+<1>.tag+""} | <2>C{op: "scratch"}), foo: <3>C{op: "pull", tag: "latest", refToTag: "latest", tagExpr: "latestdd", tagInString: "latest"}}`,
}}
rewriteHelper(t, testCases, evalFull)
}
Expand Down
15 changes: 15 additions & 0 deletions cue/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1843,6 +1843,21 @@ func TestMashalJSON(t *testing.T) {
f: >=1.1 & <=1.1
`,
json: `{"a":1,"b":1,"c":2,"d":1,"e":1,"f":1.1}`,
}, {
value: `
Task :: {
{
op: "pull"
tag: *"latest" | string
tagInString: tag + "dd"
} | {
op: "scratch"
}
}
foo: Task & {"op": "pull"}
`,
json: `{"foo":{"op":"pull","tag":"latest","tagInString":"latestdd"}}`,
}}
for i, tc := range testCases {
t.Run(fmt.Sprintf("%d/%v", i, tc.value), func(t *testing.T) {
Expand Down
6 changes: 0 additions & 6 deletions cue/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -1624,11 +1624,6 @@ func (x *disjunction) normalize(ctx *context, src source) mVal {
}
k := 0

// manifesting values should be disabled for recursive evaluation as
// these values may still be bound to another value later on, for instance
// when the result of this value is unified with another value.
noManifest := ctx.noManifest
ctx.noManifest = true
hasMarked := false
var markedErr *bottom
outer:
Expand Down Expand Up @@ -1673,7 +1668,6 @@ outer:
x.values[k] = dValue{&bottom{}, true}
k++
}
ctx.noManifest = noManifest

switch k {
case 0:
Expand Down

0 comments on commit 5c2d937

Please sign in to comment.