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

Commit

Permalink
internal/core/export: don't embed proper conjuncts
Browse files Browse the repository at this point in the history
This now makes the distinction between conjuncts
and embeddings, fixing a bug that resulted from the
slight semantics changes in embeddings introduced in
https://cue-review.googlesource.com/c/cue/+/8229

Note that scalar values can still always be embedded
as it doesn't affect closedness.
Also recersively nested embeddings can be merged.
Nested closed conjunctions need to remain conjunctions.

Change-Id: I14c1a30802132effe0c6c27a4a8e363236df5771
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/8363
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
  • Loading branch information
mpvl committed Jan 29, 2021
1 parent 10a48e9 commit d2aa995
Show file tree
Hide file tree
Showing 8 changed files with 190 additions and 39 deletions.
52 changes: 33 additions & 19 deletions internal/core/export/expr.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ func (x *exporter) mergeValues(label adt.Feature, src *adt.Vertex, a []conjunct,
for _, c := range a {
e.top().upCount = c.up
x := c.c.Expr()
e.addExpr(c.c.Env, x)
e.addExpr(c.c.Env, x, false)
}

s := x.top().scope
Expand All @@ -109,7 +109,7 @@ func (x *exporter) mergeValues(label adt.Feature, src *adt.Vertex, a []conjunct,
// Unify values only for one level.
if len(e.values.Conjuncts) > 0 {
e.values.Finalize(e.ctx)
e.exprs = append(e.exprs, e.value(e.values, e.values.Conjuncts...))
e.embed = append(e.embed, e.value(e.values, e.values.Conjuncts...))
}

// Collect and order set of fields.
Expand All @@ -135,21 +135,25 @@ func (x *exporter) mergeValues(label adt.Feature, src *adt.Vertex, a []conjunct,
})

if len(e.fields) == 0 && !e.hasEllipsis {
switch len(e.exprs) {
switch len(e.embed) + len(e.conjuncts) {
case 0:
if len(e.structs) > 0 {
return s
}
return ast.NewIdent("_")
case 1:
return e.exprs[0]
if len(e.conjuncts) == 1 {
return e.conjuncts[0]
}
return e.embed[0]
case 2:
// Simplify.
return ast.NewBinExpr(token.AND, e.exprs...)
e.conjuncts = append(e.conjuncts, e.embed...)
return ast.NewBinExpr(token.AND, e.conjuncts...)
}
}

for _, x := range e.exprs {
for _, x := range e.embed {
s.Elts = append(s.Elts, &ast.EmbedDecl{Expr: x})
}

Expand Down Expand Up @@ -201,15 +205,18 @@ func (x *exporter) mergeValues(label adt.Feature, src *adt.Vertex, a []conjunct,
return ast.NewCall(ast.NewIdent("close"), s)
}

return s
e.conjuncts = append(e.conjuncts, s)

return ast.NewBinExpr(token.AND, e.conjuncts...)
}

// Conjuncts if for collecting values of a single vertex.
type conjuncts struct {
*exporter
// Values is used to collect non-struct values.
values *adt.Vertex
exprs []ast.Expr
embed []ast.Expr
conjuncts []ast.Expr
structs []*adt.StructInfo
fields map[adt.Feature]field
attrs []*ast.Attribute
Expand Down Expand Up @@ -238,7 +245,7 @@ type conjunct struct {
up int32
}

func (e *conjuncts) addExpr(env *adt.Environment, x adt.Expr) {
func (e *conjuncts) addExpr(env *adt.Environment, x adt.Expr, isEmbed bool) {
switch x := x.(type) {
case *adt.StructLit:
e.top().upCount++
Expand All @@ -250,7 +257,7 @@ func (e *conjuncts) addExpr(env *adt.Environment, x adt.Expr) {
// Only add if it only has no bulk fields or elipsis.
if isComplexStruct(x) {
_, saved := e.pushFrame(nil)
e.exprs = append(e.exprs, e.adt(x, nil))
e.embed = append(e.embed, e.adt(x, nil))
e.popFrame(saved)
return
}
Expand All @@ -269,7 +276,7 @@ func (e *conjuncts) addExpr(env *adt.Environment, x adt.Expr) {
e.hasEllipsis = true
continue
case adt.Expr:
e.addExpr(env, f)
e.addExpr(env, f, true)
continue

// TODO: also handle dynamic fields
Expand Down Expand Up @@ -301,7 +308,7 @@ func (e *conjuncts) addExpr(env *adt.Environment, x adt.Expr) {
v.MatchAndInsert(e.ctx, v)
a = append(a, &ast.Ellipsis{Type: e.expr(v)})
}
e.exprs = append(e.exprs, ast.NewList(a...))
e.embed = append(e.embed, ast.NewList(a...))
return

case *adt.StructMarker:
Expand All @@ -322,20 +329,27 @@ func (e *conjuncts) addExpr(env *adt.Environment, x adt.Expr) {

case *adt.BinaryExpr:
switch {
case x.Op == adt.AndOp:
e.addExpr(env, x.X)
e.addExpr(env, x.Y)
case x.Op == adt.AndOp && !isEmbed:
e.addExpr(env, x.X, false)
e.addExpr(env, x.Y, false)
case isSelfContained(x):
e.values.AddConjunct(adt.MakeRootConjunct(env, x))
default:
e.exprs = append(e.exprs, e.expr(x))
if isEmbed {
e.embed = append(e.embed, e.expr(x))
} else {
e.conjuncts = append(e.conjuncts, e.expr(x))
}
}

default:
if isSelfContained(x) {
switch {
case isSelfContained(x):
e.values.AddConjunct(adt.MakeRootConjunct(env, x))
} else {
e.exprs = append(e.exprs, e.expr(x))
case isEmbed:
e.embed = append(e.embed, e.expr(x))
default:
e.conjuncts = append(e.conjuncts, e.expr(x))
}
}
}
Expand Down
49 changes: 46 additions & 3 deletions internal/core/export/testdata/adt.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,18 @@ l1: [3, ...int]
l2: [...int]
l3: []
l4: [1, 2]
l5: [1, 3] & {
[1, 3]

#foo: int
}

#foo: int
l6: [1, #foo] & {
[1, 3]

#foo: int
}

n1: 1.0
n10: 10
Expand All @@ -48,9 +60,9 @@ e6: !t
e7: !t || !false
e8?: !false

m1: {
m1: {[string]: uint} & {
// this is a pattern constraint
{[string]: int}
{[string]: int} & {[string]: int64}
// foo is an optional field
foo?: 3

Expand Down Expand Up @@ -134,6 +146,15 @@ l1: [3, ...int]
l2: [...int]
l3: []
l4: [1, 2]
l5: [1, 3] & {
[1, 3]
#foo: int
}
#foo: int
l6: [1, #foo] & {
[1, 3]
#foo: int
}
n1: 1.0
n10: 10
t: true
Expand All @@ -145,9 +166,13 @@ e5: e1 + e2 - e3
e6: !t
e7: !t || !false
e8?: !false
m1: {
m1: {
[string]: int & >=0
} & {
{
[string]: int
} & {
[string]: int64
}

// foo is an optional field
Expand Down Expand Up @@ -205,6 +230,15 @@ errorListDef: {
[l4]
[l4 0]
[l4 1]
[l5]
[l5 #foo]
[l5 0]
[l5 1]
[#foo]
[l6]
[l6 #foo]
[l6 0]
[l6 1]
[n1]
[n10]
[t]
Expand Down Expand Up @@ -281,6 +315,15 @@ _|_ // e3: index out of range [2] with length 2
l2: []
l3: []
l4: [1, 2]
l5: {
#foo: int
[1, 3]
}
#foo: int
l6: {
#foo: int
[1, 3]
}
n1: 1.0
n10: 10

Expand Down
4 changes: 1 addition & 3 deletions internal/core/export/testdata/docs.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,7 @@ bar: {
// comment from bar on field 2
field2: int
}
baz: {
bar

baz: bar & {
// comment from baz on field 1
field1: int
field2: int
Expand Down
3 changes: 1 addition & 2 deletions internal/core/export/testdata/hidden.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,7 @@ package foo

import "acme.com/pkg"

a: {
pkg.#A
a: pkg.#A & {
_val: 3
_#val: 6
}
Expand Down
3 changes: 1 addition & 2 deletions internal/core/export/testdata/issue662.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
#LineConfig: {
lineColor?: string
}
#GraphFieldConfig: {
#LineConfig
#GraphFieldConfig: #LineConfig & {
drawStyle?: int
}
-- out/doc --
Expand Down
102 changes: 102 additions & 0 deletions internal/core/export/testdata/merge.txtar
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
-- in.cue --
#A: {
_
#a: string
}

#E: {_}
#F: {_}
#G: {_}

#B: #A & {
{ 2, #def: 3 }
2
#E & #F
#f: int
{
_
#bar: int
#G
}

}
-- out/definition --
#A: {
#a: string
}
#E: {}
#F: {}
#G: {}
#B: #A & {
#E & #F
#G
2
#f: int
#def: 3
#bar: int
}
-- out/doc --
[]
[#A]
[#A #a]
[#E]
[#F]
[#G]
[#B]
[#B #a]
[#B #def]
[#B #bar]
[#B #f]
-- out/value --
== Simplified
{}
== Raw
{
#A: {
#a: string
}
#E: {}
#F: {}
#G: {}
#B: {
2
#a: string
#f: int
#def: 3
#bar: int
}
}
== Final
{}
== All
{
#A: {
#a: string
}
#E: {}
#F: {}
#G: {}
#B: {
2
#a: string
#f: int
#def: 3
#bar: int
}
}
== Eval
{
#A: {
#a: string
}
#E: {}
#F: {}
#G: {}
#B: {
2
#a: string
#f: int
#def: 3
#bar: int
}
}
4 changes: 2 additions & 2 deletions internal/filetypes/types.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit d2aa995

Please sign in to comment.