Skip to content

Commit

Permalink
internal/core/export: bug fixes in definitions
Browse files Browse the repository at this point in the history 
- don't print `_` when not appropriate.
- print constraint information when appropriate

Fixes #882

Change-Id: I474ddf4448b3b7c0ec795688730e9e9b581163dd
Reviewed-on: https://cue-review.googlesource.com/c/cue/+/9485
Reviewed-by: CUE cueckoo <cueckoo@gmail.com>
Reviewed-by: Marcel van Lohuizen <mpvl@golang.org>
  • Loading branch information
@mpvl
mpvl committed Apr 23, 2021
1 parent b5b0429 commit c62b750
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 55 deletions.
44 changes: 33 additions & 11 deletions internal/core/export/export_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ func formatNode(t *testing.T, n ast.Node) []byte {
// TestGenerated tests conversions of generated Go structs, which may be
// different from parsed or evaluated CUE, such as having Vertex values.
func TestGenerated(t *testing.T) {
ctx := cuecontext.New()

testCases := []struct {
in func(ctx *adt.OpContext) (adt.Expr, error)
out string
Expand All @@ -88,7 +90,7 @@ func TestGenerated(t *testing.T) {
}
return convert.GoValueToValue(ctx, in, false), nil
},
out: `{Terminals: [{Name: "Name", Description: "Desc"}]}`,
out: `Terminals: [{Name: "Name", Description: "Desc"}]`,
}, {
in: func(ctx *adt.OpContext) (adt.Expr, error) {
in := &C{
Expand Down Expand Up @@ -134,13 +136,11 @@ func TestGenerated(t *testing.T) {

return n, nil
},
// TODO: should probably print path.
out: `<[l2// undefined field #Terminal] _|_>`,
out: `<[l2// x: undefined field #Terminal] _|_>`,
p: export.Final,
}, {
in: func(ctx *adt.OpContext) (adt.Expr, error) {
c := cuecontext.New()
v := c.CompileString(`
in: func(r *adt.OpContext) (adt.Expr, error) {
v := ctx.CompileString(`
#Provider: {
ID: string
notConcrete: bool
Expand All @@ -155,12 +155,28 @@ func TestGenerated(t *testing.T) {

return n, nil
},
out: `{#Provider: {ID: string, notConcrete: bool, a: int, b: a+1}, providers: {foo: {ID: "12345", notConcrete: bool, a: int, b: a+1}}}`,
out: `#Provider: {ID: string, notConcrete: bool, a: int, b: a+1}, providers: {foo: {ID: "12345", notConcrete: bool, a: int, b: a+1}}`,
}, {
// Issue #882
in: func(r *adt.OpContext) (adt.Expr, error) {
valA := ctx.CompileString(`
#One: { version: string }
`)

valB := ctx.CompileString(`
#One: _
ones: {[string]: #One}
`)
v := valB.Unify(valA)
_, n := value.ToInternal(v)
return n, nil
},
out: `#One: {version: string}, ones: {[string]: #One}`,
p: export.All,
}}
for _, tc := range testCases {
t.Run("", func(t *testing.T) {
r := runtime.New()
ctx := adt.NewContext(r, &adt.Vertex{})
ctx := adt.NewContext((*runtime.Runtime)(ctx), &adt.Vertex{})

v, err := tc.in(ctx)
if err != nil {
Expand All @@ -172,11 +188,17 @@ func TestGenerated(t *testing.T) {
p = export.Simplified
}

expr, err := p.Expr(ctx, "", v)
var n ast.Node
switch x := v.(type) {
case *adt.Vertex:
n, err = p.Def(ctx, "", x)
default:
n, err = p.Expr(ctx, "", v)
}
if err != nil {
t.Fatal("failed export: ", err)
}
got := astinternal.DebugStr(expr)
got := astinternal.DebugStr(n)
if got != tc.out {
t.Errorf("got: %s\nwant: %s", got, tc.out)
}
Expand Down
106 changes: 64 additions & 42 deletions internal/core/export/expr.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,16 @@ 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, false)
e.addExpr(c.c.Env, src, x, false)
}

if src != nil {
for _, a := range src.Arcs {
if x, ok := e.fields[a.Label]; ok {
x.arc = a
e.fields[a.Label] = x
}
}
}

s := x.top().scope
Expand All @@ -107,9 +116,9 @@ func (x *exporter) mergeValues(label adt.Feature, src *adt.Vertex, a []conjunct,
}

// Unify values only for one level.
if len(e.values.Conjuncts) > 0 {
if a := e.values.Conjuncts; len(a) > 0 {
e.values.Finalize(e.ctx)
e.embed = append(e.embed, e.value(e.values, e.values.Conjuncts...))
e.embed = append(e.embed, e.value(e.values, a...))
}

// Collect and order set of fields.
Expand Down Expand Up @@ -196,7 +205,7 @@ func (x *exporter) mergeValues(label adt.Feature, src *adt.Vertex, a []conjunct,
top.fields[f] = fr
}

d.Value = e.mergeValues(f, nil, c, a...)
d.Value = e.mergeValues(f, field.arc, c, a...)

if f.IsDef() {
x.inDefinition--
Expand All @@ -218,10 +227,13 @@ func (x *exporter) mergeValues(label adt.Feature, src *adt.Vertex, a []conjunct,
}
if e.hasEllipsis {
s.Elts = append(s.Elts, &ast.Ellipsis{})
} else if src != nil && src.IsClosedStruct() && e.inDefinition == 0 {
return ast.NewCall(ast.NewIdent("close"), s)
}

// TODO: why was this necessary?
// else if src != nil && src.IsClosedStruct() && e.inDefinition == 0 {
// return ast.NewCall(ast.NewIdent("close"), s)
// }

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

return ast.NewBinExpr(token.AND, e.conjuncts...)
Expand All @@ -240,8 +252,16 @@ type conjuncts struct {
hasEllipsis bool
}

func (c *conjuncts) addConjunct(f adt.Feature, env *adt.Environment, n adt.Node) {
func (c *conjuncts) addValueConjunct(src *adt.Vertex, env *adt.Environment, x adt.Expr) {
switch b, ok := x.(adt.BaseValue); {
case ok && src != nil && isTop(b) && !isTop(src.BaseValue):
// drop top
default:
c.values.AddConjunct(adt.MakeRootConjunct(env, x))
}
}

func (c *conjuncts) addConjunct(f adt.Feature, env *adt.Environment, n adt.Node) {
x := c.fields[f]
v := adt.MakeRootConjunct(env, n)
x.conjuncts = append(x.conjuncts, conjunct{
Expand All @@ -254,6 +274,7 @@ func (c *conjuncts) addConjunct(f adt.Feature, env *adt.Environment, n adt.Node)

type field struct {
docs []*ast.CommentGroup
arc *adt.Vertex
conjuncts []conjunct
}

Expand All @@ -262,7 +283,7 @@ type conjunct struct {
up int32
}

func (e *conjuncts) addExpr(env *adt.Environment, x adt.Expr, isEmbed bool) {
func (e *conjuncts) addExpr(env *adt.Environment, src *adt.Vertex, x adt.Expr, isEmbed bool) {
switch x := x.(type) {
case *adt.StructLit:
e.top().upCount++
Expand Down Expand Up @@ -294,7 +315,7 @@ func (e *conjuncts) addExpr(env *adt.Environment, x adt.Expr, isEmbed bool) {
e.hasEllipsis = true
continue
case adt.Expr:
e.addExpr(env, f, true)
e.addExpr(env, nil, f, true)
continue

// TODO: also handle dynamic fields
Expand All @@ -309,54 +330,44 @@ func (e *conjuncts) addExpr(env *adt.Environment, x adt.Expr, isEmbed bool) {
switch v := x.(type) {
case nil:
default:
e.values.AddConjunct(adt.MakeRootConjunct(env, x)) // GOBBLE TOP
e.addValueConjunct(src, env, x)

case *adt.Vertex:
e.structs = append(e.structs, v.Structs...)

switch y := v.BaseValue.(type) {
case *adt.ListMarker:
a := []ast.Expr{}
for _, x := range v.Elems() {
a = append(a, e.expr(x))
if b, ok := v.BaseValue.(*adt.Bottom); ok {
if !b.IsIncomplete() || e.cfg.Final {
e.addExpr(env, v, b, false)
return
}
if !v.IsClosedList() {
v := &adt.Vertex{}
v.MatchAndInsert(e.ctx, v)
a = append(a, &ast.Ellipsis{Type: e.expr(v)})
}

switch {
default:
for _, c := range v.Conjuncts {
e.addExpr(c.Env, v, c.Expr(), false)
}
e.embed = append(e.embed, ast.NewList(a...))
return

case *adt.StructMarker:
x = nil
case v.IsData():
e.structs = append(e.structs, v.Structs...)

case adt.Value:
if v.IsData() {
e.values.AddConjunct(adt.MakeRootConjunct(env, y))
break
}
for _, c := range v.Conjuncts {
e.values.AddConjunct(c)
if y, ok := v.BaseValue.(adt.Value); ok {
e.addValueConjunct(src, env, y)
}
}

// generated, only consider arcs.
for _, a := range v.Arcs {
a.Finalize(e.ctx) // TODO: should we do this?
for _, a := range v.Arcs {
a.Finalize(e.ctx) // TODO: should we do this?

e.addConjunct(a.Label, env, a)
e.addConjunct(a.Label, env, a)
}
}
// e.exprs = append(e.exprs, e.value(v, v.Conjuncts...))
}

case *adt.BinaryExpr:
switch {
case x.Op == adt.AndOp && !isEmbed:
e.addExpr(env, x.X, false)
e.addExpr(env, x.Y, false)
e.addExpr(env, src, x.X, false)
e.addExpr(env, src, x.Y, false)
case isSelfContained(x):
e.values.AddConjunct(adt.MakeRootConjunct(env, x))
e.addValueConjunct(src, env, x)
default:
if isEmbed {
e.embed = append(e.embed, e.expr(x))
Expand All @@ -368,7 +379,7 @@ func (e *conjuncts) addExpr(env *adt.Environment, x adt.Expr, isEmbed bool) {
default:
switch {
case isSelfContained(x):
e.values.AddConjunct(adt.MakeRootConjunct(env, x))
e.addValueConjunct(src, env, x)
case isEmbed:
e.embed = append(e.embed, e.expr(x))
default:
Expand All @@ -377,6 +388,17 @@ func (e *conjuncts) addExpr(env *adt.Environment, x adt.Expr, isEmbed bool) {
}
}

func isTop(x adt.BaseValue) bool {
switch v := x.(type) {
case *adt.Top:
return true
case *adt.BasicType:
return v.K == adt.TopKind
default:
return false
}
}

// TODO: find a better way to annotate optionality. Maybe a special conjunct
// or store it in the field information?
func isOptional(a []adt.Conjunct) bool {
Expand Down
4 changes: 2 additions & 2 deletions internal/filetypes/types.go
View file

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

0 comments on commit c62b750

Please sign in to comment.