internal/core/eval: fix result of closed embedding
This brings the implementation in line with the
spec.
Includes a small change to the formulation
in the spec to clarify things. But it doesn't
aim to change the meaning.
Change-Id: I35dadb4af53ea3f008e39dea92ccfef0fb33f375
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8061
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
diff --git a/cue/testdata/definitions/embed.txtar b/cue/testdata/definitions/embed.txtar
index 201af61..1d045d6 100644
--- a/cue/testdata/definitions/embed.txtar
+++ b/cue/testdata/definitions/embed.txtar
@@ -13,6 +13,11 @@
#TypeMeta: {}
+recloseSimple: {
+ #foo: {}
+ a: {#foo} & {b: int}
+}
+
// Reclosing
reclose1: {
#D: {
@@ -47,7 +52,8 @@
}
reclose3: {
- #Step: (#A | #B) & {
+ #Step: {
+ (#A | #B)
#Common
}
#Common: {
@@ -71,9 +77,14 @@
-- out/eval --
Errors:
reclose1.z: field `d` not allowed:
- ./in.cue:23:8
- ./in.cue:28:6
- ./in.cue:29:6
+ ./in.cue:28:8
+ ./in.cue:33:6
+ ./in.cue:34:6
+recloseSimple.a: field `b` not allowed:
+ ./in.cue:16:9
+ ./in.cue:17:6
+ ./in.cue:17:7
+ ./in.cue:17:16
Result:
(_|_){
@@ -95,6 +106,21 @@
}
#TypeMeta: (#struct){
}
+ recloseSimple: (_|_){
+ // [eval]
+ #foo: (#struct){
+ }
+ a: (_|_){
+ // [eval]
+ b: (_|_){
+ // [eval] recloseSimple.a: field `b` not allowed:
+ // ./in.cue:16:9
+ // ./in.cue:17:6
+ // ./in.cue:17:7
+ // ./in.cue:17:16
+ }
+ }
+ }
reclose1: (_|_){
// [eval]
#D: (#struct){
@@ -113,9 +139,9 @@
c: (int){ int }
d: (_|_){
// [eval] reclose1.z: field `d` not allowed:
- // ./in.cue:23:8
- // ./in.cue:28:6
- // ./in.cue:29:6
+ // ./in.cue:28:8
+ // ./in.cue:33:6
+ // ./in.cue:34:6
}
}
}
@@ -182,6 +208,14 @@
replicas: int
}
#TypeMeta: {}
+ recloseSimple: {
+ #foo: {}
+ a: ({
+ 〈1;#foo〉
+ } & {
+ b: int
+ })
+ }
reclose1: {
#D: {
x: int
@@ -215,9 +249,10 @@
}
}
reclose3: {
- #Step: ((〈0;#A〉|〈0;#B〉) & {
+ #Step: {
+ (〈1;#A〉|〈1;#B〉)
〈1;#Common〉
- })
+ }
#Common: {
Name: string
}
diff --git a/doc/ref/spec.md b/doc/ref/spec.md
index 395730b..a5a1dec 100644
--- a/doc/ref/spec.md
+++ b/doc/ref/spec.md
@@ -1306,8 +1306,8 @@
A struct may contain an _embedded value_, an operand used as a declaration.
An embedded value of type struct is unified with the struct in which it is
embedded, but disregarding the restrictions imposed by closed structs.
-So if an embedding contains a closed struct, the corresponding resulting struct
-will also be closed, but may have fields that are not allowed if
+So if an embedding resolves to a closed struct, the corresponding enclosing
+struct will also be closed, but may have fields that are not allowed if
normal rules for closed structs were observed.
If an embedded value is not of type struct, the struct may only have
diff --git a/internal/core/adt/closed.go b/internal/core/adt/closed.go
index 1f8ba57..f68bc61 100644
--- a/internal/core/adt/closed.go
+++ b/internal/core/adt/closed.go
@@ -118,6 +118,24 @@
return c
}
+// SpawnGroup is used for structs that contain embeddings that may end up
+// closing the struct. This is to force that `b` is not allowed in
+//
+// a: {#foo} & {b: int}
+//
+func (c CloseInfo) SpawnGroup(x Expr) CloseInfo {
+ embedded := false
+ if c.closeInfo != nil {
+ embedded = c.embedded
+ }
+ c.closeInfo = &closeInfo{
+ parent: c.closeInfo,
+ location: x,
+ embedded: embedded,
+ }
+ return c
+}
+
func (c CloseInfo) SpawnRef(arc *Vertex, isDef bool, x Expr) CloseInfo {
embedded := false
if c.closeInfo != nil {
diff --git a/internal/core/adt/closed_test.go b/internal/core/adt/closed_test.go
index d3485ee..9436997 100644
--- a/internal/core/adt/closed_test.go
+++ b/internal/core/adt/closed_test.go
@@ -301,6 +301,46 @@
},
required: true,
}, {
+ desc: "local closing of def",
+ // #test: {
+ // #def
+ // a: 1
+ // b: 1
+ // }
+ // #test: {
+ // c: 1
+ // d: 1
+ // }
+ // #def: {
+ // c: 1
+ // e: 1
+ // }
+ n: func() *adt.Vertex {
+ var (
+ root adt.CloseInfo
+ test = root.SpawnRef(nil, true, mkRef("#test"))
+ // isolate local struct.
+ spawned = test.SpawnRef(nil, false, mkRef("dummy"))
+ embed = spawned.SpawnEmbed(mkRef("dummy"))
+ def = embed.SpawnRef(nil, true, mkRef("#def"))
+ )
+ return &adt.Vertex{
+ Structs: []*adt.StructInfo{
+ mkStruct(spawned, "{a: 1, b: 1}"),
+ mkStruct(test, "{c: 1, d: 1}"),
+ mkStruct(def, "{c: 1, e: 1}"),
+ },
+ }
+ },
+ tests: []test{
+ {"a", true},
+ {"d", false},
+ {"c", true},
+ {"e", true},
+ {"f", false},
+ },
+ required: true,
+ }, {
desc: "branching",
// test: #foo
// #foo: {
@@ -372,11 +412,11 @@
// -----
for _, tc := range testCases {
t.Run(tc.desc, func(t *testing.T) {
+ n := tc.n()
for _, sub := range tc.tests {
t.Run(sub.f, func(t *testing.T) {
f := adt.MakeIdentLabel(r, sub.f, "")
- n := tc.n()
ok, required := adt.Accept(ctx, n, f)
if ok != sub.found || required != tc.required {
t.Errorf("got (%v, %v); want (%v, %v)",
diff --git a/internal/core/adt/eval.go b/internal/core/adt/eval.go
index 16a45bc..2e7abc1 100644
--- a/internal/core/adt/eval.go
+++ b/internal/core/adt/eval.go
@@ -1569,14 +1569,16 @@
childEnv.Deref = env.Deref
}
+ s.Init()
+
+ if s.HasEmbed {
+ closeInfo = closeInfo.SpawnGroup(nil)
+ }
+
parent := n.node.AddStruct(s, childEnv, closeInfo)
closeInfo.IsClosed = false
parent.Disable = true // disable until processing is done.
- hasEmbed := false
-
- s.Init()
-
for _, d := range s.Decls {
switch x := d.(type) {
case *Field:
@@ -1602,8 +1604,6 @@
n.ifClauses = append(n.ifClauses, envYield{childEnv, x, closeInfo, nil})
case Expr:
- hasEmbed = true
-
// add embedding to optional
// TODO(perf): only do this if addExprConjunct below will result in
@@ -1626,7 +1626,7 @@
}
}
- if !hasEmbed {
+ if !s.HasEmbed {
n.aStruct = s
n.aStructID = closeInfo
}
diff --git a/internal/core/adt/expr.go b/internal/core/adt/expr.go
index d8f5118..c607a8a 100644
--- a/internal/core/adt/expr.go
+++ b/internal/core/adt/expr.go
@@ -46,6 +46,7 @@
Bulk []*BulkOptionalField
Additional []Expr
+ HasEmbed bool
IsOpen bool // has a ...
initialized bool
@@ -95,8 +96,10 @@
case *DynamicField:
o.Dynamic = append(o.Dynamic, x)
- case *ForClause, Yielder, Expr:
- // Set hasEmbed, or maybe add embeddings.
+ case Expr:
+ o.HasEmbed = true
+
+ case *ForClause, Yielder:
case *BulkOptionalField:
o.Bulk = append(o.Bulk, x)
