Skip to content

Commit

Permalink
internal/core/adt: fix regression in close builtin
Browse files Browse the repository at this point in the history
Would in some cases recursively close.

Now uses just one method for marking closedness. Marking
Closed in the Vertex was only used so that the debug printer
would show the closed status. This has now fixed. This was
not possible before but is now as the code was recently
simplified.

Fixes #642

Change-Id: I9aab6a02f36ddbc8ed9bce356a2f4ad77cd30cda
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8228
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
  • Loading branch information
mpvl committed Jan 16, 2021
1 parent a086f74 commit 826bd7b
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 7 deletions.
97 changes: 94 additions & 3 deletions cue/testdata/builtins/closed.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,30 @@ inDisjunctions: {
x: syslog: xxx: {}
}

issue642: {
test: close({
a: _
b: x: _
} & {
[string]: y: _
})

test: a: x: _
test: b: x: _
}

// Issue 642
withSubfields: {
test: close({
a: _
b: x: _
[string]: y: _
})

test: a: x: _
test: b: x: _
}

-- out/eval --
Errors:
b: field `x` not allowed:
Expand All @@ -29,13 +53,13 @@ Result:
(_|_){
// [eval]
a: (#struct){
a: (#struct){
a: (struct){
b: (int){ int }
}
}
b: (_|_){
// [eval]
a: (#struct){
a: (struct){
b: (int){ int }
}
x: (_|_){
Expand All @@ -46,7 +70,7 @@ Result:
}
}
c: (#struct){
a: (#struct){
a: (struct){
b: (int){ int }
c: (int){ int }
}
Expand Down Expand Up @@ -118,6 +142,30 @@ Result:
#Def: (#struct){
}
}
issue642: (struct){
test: (#struct){
a: (struct){
y: (_){ _ }
x: (_){ _ }
}
b: (struct){
x: (_){ _ }
y: (_){ _ }
}
}
}
withSubfields: (struct){
test: (#struct){
a: (struct){
y: (_){ _ }
x: (_){ _ }
}
b: (struct){
x: (_){ _ }
y: (_){ _ }
}
}
}
}
-- out/compile --
--- in.cue
Expand Down Expand Up @@ -173,4 +221,47 @@ Result:
}
}
}
issue642: {
test: close(({
a: _
b: {
x: _
}
} & {
[string]: {
y: _
}
}))
test: {
a: {
x: _
}
}
test: {
b: {
x: _
}
}
}
withSubfields: {
test: close({
a: _
b: {
x: _
}
[string]: {
y: _
}
})
test: {
a: {
x: _
}
}
test: {
b: {
x: _
}
}
}
}
3 changes: 3 additions & 0 deletions internal/core/adt/closed.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,9 @@ func (c *closeInfo) isClosed() bool {

func isClosed(v *Vertex) bool {
for _, s := range v.Structs {
if s.IsClosed {
return true
}
for c := s.closeInfo; c != nil; c = c.parent {
if c.isClosed() {
return true
Expand Down
1 change: 0 additions & 1 deletion internal/core/adt/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -1299,7 +1299,6 @@ func (n *nodeContext) addValueConjunct(env *Environment, v Value, id CloseInfo)
n.aStruct = x
n.aStructID = id
if m.NeedClose {
n.node.Closed = true // TODO: remove.
id = id.SpawnRef(x, IsDef(x), x)
id.IsClosed = true
}
Expand Down
9 changes: 6 additions & 3 deletions internal/core/adt/optional.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,16 @@ package adt
func (o *StructInfo) MatchAndInsert(c *OpContext, arc *Vertex) {
env := o.Env

closeInfo := o.CloseInfo
closeInfo.IsClosed = false

// Match normal fields
matched := false
outer:
for _, f := range o.Fields {
if f.Label == arc.Label {
for _, e := range f.Optional {
arc.AddConjunct(MakeConjunct(env, e, o.CloseInfo))
arc.AddConjunct(MakeConjunct(env, e, closeInfo))
}
matched = true
break outer
Expand All @@ -49,7 +52,7 @@ outer:
// }
if matchBulk(c, env, b, arc.Label) {
matched = true
arc.AddConjunct(MakeConjunct(&bulkEnv, b, o.CloseInfo))
arc.AddConjunct(MakeConjunct(&bulkEnv, b, closeInfo))
}
}
if matched {
Expand All @@ -62,7 +65,7 @@ outer:

// match others
for _, x := range o.Additional {
arc.AddConjunct(MakeConjunct(&addEnv, x, o.CloseInfo))
arc.AddConjunct(MakeConjunct(&addEnv, x, closeInfo))
}
}

Expand Down

0 comments on commit 826bd7b

Please sign in to comment.