Skip to content

Commit

Permalink
internal/core/adt: fix handling of ellipsis in evalv3
Browse files Browse the repository at this point in the history
There were two fields in the closeContext that
duplicated functionality. By unifying the two we
now handle ... correctly.

Fixes #3572

Signed-off-by: Marcel van Lohuizen <mpvl@gmail.com>
Change-Id: Ia7241af91186431a775d0ca430060710de9fb537
Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1204259
Reviewed-by: Roger Peppe <rogpeppe@gmail.com>
TryBot-Result: CUEcueckoo <cueckoo@cuelang.org>
Unity-Result: CUE porcuepine <cue.porcuepine@gmail.com>
  • Loading branch information
mpvl committed Nov 19, 2024
1 parent a2490a2 commit c97d709
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 74 deletions.
71 changes: 8 additions & 63 deletions cue/testdata/builtins/closed.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,6 @@ Errors:
b.x: field not allowed:
./in.cue:1:4
./in.cue:5:9
issue3572.t1.a: field not allowed:
./in.cue:45:7
./in.cue:45:23
issue3572.t2.a: field not allowed:
./in.cue:47:6
./in.cue:49:12

Result:
(_|_){
Expand Down Expand Up @@ -205,49 +199,31 @@ Result:
}
}
}
issue3572: (_|_){
// [eval]
t1: (_|_){
// [eval]
a: (_|_){
// [eval] issue3572.t1.a: field not allowed:
// ./in.cue:45:7
// ./in.cue:45:23
}
issue3572: (struct){
t1: (#struct){
a: (int){ 5 }
}
e: (#struct){
}
t2: (_|_){
// [eval]
a: (_|_){
// [eval] issue3572.t2.a: field not allowed:
// ./in.cue:47:6
// ./in.cue:49:12
}
t2: (#struct){
a: (int){ 5 }
}
}
}
-- diff/-out/evalalpha<==>+out/eval --
diff old new
--- old
+++ new
@@ -1,8 +1,13 @@
@@ -1,7 +1,6 @@
Errors:
b.x: field not allowed:
- ./in.cue:1:10
- ./in.cue:5:4
+ ./in.cue:1:4
./in.cue:5:9
+issue3572.t1.a: field not allowed:
+ ./in.cue:45:7
+ ./in.cue:45:23
+issue3572.t2.a: field not allowed:
+ ./in.cue:47:6
+ ./in.cue:49:12

Result:
(_|_){
@@ -14,20 +19,19 @@
@@ -14,20 +13,19 @@
}
b: (_|_){
// [eval]
Expand All @@ -273,7 +249,7 @@ diff old new
}
}
inDisjunctions: (struct){
@@ -60,42 +64,50 @@
@@ -60,42 +58,50 @@
}) }
}
syslog: (#struct){
Expand Down Expand Up @@ -356,37 +332,6 @@ diff old new
string: (#struct){
a: (#struct){
b: (bool){ true }
@@ -131,14 +143,25 @@
}
}
}
- issue3572: (struct){
- t1: (#struct){
- a: (int){ 5 }
+ issue3572: (_|_){
+ // [eval]
+ t1: (_|_){
+ // [eval]
+ a: (_|_){
+ // [eval] issue3572.t1.a: field not allowed:
+ // ./in.cue:45:7
+ // ./in.cue:45:23
+ }
}
e: (#struct){
}
- t2: (#struct){
- a: (int){ 5 }
+ t2: (_|_){
+ // [eval]
+ a: (_|_){
+ // [eval] issue3572.t2.a: field not allowed:
+ // ./in.cue:47:6
+ // ./in.cue:49:12
+ }
}
}
}
-- diff/todo/p3 --
Reordering
Let differs.
Expand Down
2 changes: 1 addition & 1 deletion internal/core/adt/conjunct.go
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ loop2:
}
}
if hasEllipsis {
ci.cc.hasEllipsis = true
ci.cc.isTotal = true
}
if !hasEmbed {
n.aStruct = s
Expand Down
2 changes: 1 addition & 1 deletion internal/core/adt/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -615,7 +615,7 @@ func (m *mermaidContext) pstr(cc *closeContext) string {
addFlag(cc.isEmbed, 'E')
addFlag(cc.isClosed, 'c')
addFlag(cc.isClosedOnce, 'C')
addFlag(cc.hasEllipsis, 'o')
addFlag(cc.isTotal, 'o')
flags.WriteByte(cc.arcType.String()[0])
io.Copy(w, flags)

Expand Down
8 changes: 1 addition & 7 deletions internal/core/adt/fields.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,6 @@ type closeContext struct {
// definition. This value propagates to itself and parents through isDef.
isDefOrig bool

// hasEllipsis indicates whether the node contains an ellipsis.
hasEllipsis bool

// hasTop indicates a node has at least one top conjunct.
hasTop bool

Expand Down Expand Up @@ -598,7 +595,7 @@ func (c *closeContext) decDependent(ctx *OpContext, kind depKind, dependant *clo

p := c.parent

if c.isDef && !c.hasEllipsis && (!c.hasTop || c.hasNonTop) {
if c.isDef && !c.isTotal && (!c.hasTop || c.hasNonTop) {
c.isClosed = true
if p != nil {
p.isDef = true
Expand Down Expand Up @@ -637,9 +634,6 @@ func (c *closeContext) decDependent(ctx *OpContext, kind depKind, dependant *clo
return
}

if c.hasEllipsis {
p.hasEllipsis = true
}
if c.hasTop {
p.hasTop = true
}
Expand Down
1 change: 0 additions & 1 deletion internal/core/adt/overlay.go
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,6 @@ func (ctx *overlayContext) initCloneCC(x *closeContext) {
o.disjunctCount = x.disjunctCount
o.isDef = x.isDef
o.isDefOrig = x.isDefOrig
o.hasEllipsis = x.hasEllipsis
o.hasTop = x.hasTop
o.hasNonTop = x.hasNonTop
o.isClosedOnce = x.isClosedOnce
Expand Down
2 changes: 1 addition & 1 deletion internal/core/adt/unify.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,7 @@ func (v *Vertex) unify(c *OpContext, needs condition, mode runMode) bool {

// validationCompleted
if n.completed&(subFieldsProcessed) != 0 {
n.node.HasEllipsis = n.node.cc().hasEllipsis
n.node.HasEllipsis = n.node.cc().isTotal

// The next piece of code used to address the following case
// (order matters)
Expand Down

0 comments on commit c97d709

Please sign in to comment.