From c62b750e927f510853cfece6ebe20fc04f0a92b9 Mon Sep 17 00:00:00 2001 From: Marcel van Lohuizen Date: Fri, 23 Apr 2021 14:08:41 +0200 Subject: [PATCH] internal/core/export: bug fixes in definitions - 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 Reviewed-by: Marcel van Lohuizen --- internal/core/export/export_test.go | 44 +++++++++--- internal/core/export/expr.go | 106 +++++++++++++++++----------- internal/filetypes/types.go | 4 +- 3 files changed, 99 insertions(+), 55 deletions(-) diff --git a/internal/core/export/export_test.go b/internal/core/export/export_test.go index 41d40ebb5..c61699f81 100644 --- a/internal/core/export/export_test.go +++ b/internal/core/export/export_test.go @@ -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 @@ -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{ @@ -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 @@ -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 { @@ -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) } diff --git a/internal/core/export/expr.go b/internal/core/export/expr.go index 77709545c..627d57cd3 100644 --- a/internal/core/export/expr.go +++ b/internal/core/export/expr.go @@ -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 @@ -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. @@ -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-- @@ -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...) @@ -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{ @@ -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 } @@ -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++ @@ -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 @@ -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)) @@ -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: @@ -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 { diff --git a/internal/filetypes/types.go b/internal/filetypes/types.go index b2948eabb..1bb1ac0c6 100644 --- a/internal/filetypes/types.go +++ b/internal/filetypes/types.go @@ -41,5 +41,5 @@ func cuegenMake(name string, x interface{}) cue.Value { return v } -// Data size: 1709 bytes. -var cuegenInstanceData = []byte("\x01\x1f\x8b\b\x00\x00\x00\x00\x00\x00\xff\xacX_\x8f\u0736\x11\x97\xce.P\x11i\xdf\xf2X`,\x03Azpu\u0203\x11c\x81\x83\xe1\xc4v\u15e6(\u04a7 8p\xa5\xd1.\x1b\x89TI*\xb9Cn\x1f\u06a6i\xbfQ\xbfF?Q\xae\x18\x92\x12%\xad\xee\xeck|/\xb7;?\xcep\xe6G\xce\x1f\xee\xafn\xfeu\x92\x9e\xdc\xfc;Io\xfe\x9e$\x9f\xde\xfc\xedA\x9a~ \xa4\xb1\\\x96\xf8\x92[N\xf2\xf4A\xfa\xf0OJ\xd9\xf4$I\x1f\xfe\x91\xdb}\xfaA\x92\xfe\xe2\xb5h\u04247?&I\xf2\x9b\x9b\u007f\x9e\xa4\u9bff\xfa\xba\ucc68E\x134\u007fL\u049b\x1f\x92\xe4\xe3\x9b\u007f\xfc\x03o\x91\f=tB\x96$\xc9O\x1f\xfe\x97y\v\xeb\x94\u0491\xf3\x03\xcbT\xdf\xd9\xe1\xe2x\xaf\x9e=}\xffn={z_\xbf\xf0[*\x06\xff\xffu\xbe\xf8\xec\xc5\xfb\x0f\xe3\xb3\x17o\t\xa3\x16\x94\xf9\xd38*\xac\u007fV\x18O?\xfd\xfc\xd9{OMg\xf5\x9e\xf99\xb4\xbbWC\x9aB\xcb;\xe3;KL]\xaae\xa16z\xa8\xd3T\x13\xad\xa0R\xb8\xc8\xf0<\x9f\xb6\xdc\v\x96\xe54)\x8cBj\xbe$`\xb1\x10D9\t\x06\xa0\t\xc8\b4\x844UT\x9a#\xf2V$\x14\x8fh\x8d\x04l,\x11+\x80\xbd\xb4s\xc0\xe2\xa5%`\xa7bt\x0e\xd8)\x12wZY5\xf5\xd7\t\x9c%\xbc\xb4\x03:Z\x9a\xa3\u06c9\xcf\x11e\x19\xf5\x97/^~\xb1\x01\n\xc4\xe0_\x9f8Q^\f\n\xa3\xd2V\xc8n\vgg\xb0\x15\x92\xeb\xabn;\xce\r\u00f4\x04BV\xa2\xf4-\xca\x1f \xdd\x06n]\x9f\xd3\xd8i4(iv\x01NG\xbb\u04fc-\xd88km\xe0\xd1y\x9e{\x93\x12\xe6S\x16ThQ\xb7\x93\xa1\xa4Dm\xb9\x90\x83\x1d0{\xd57\x15\xb5\xc2\xd9hrv\x06\xaf\x95\x86a\x9e}\x02\xaeV\xb4\xfcj\xb1\x128\xb5eSj\xb1\xf5\xfe\xf9\x1b\xfc\x04\xbe\u06cbr\x0f\xc2\x1alj\xd7F\xb9$\xd5R\xc9oQ[\xdf\u007f9|\xfe\xe7WA\xa3`\x8b\x01q\x9c\xf9\xdcX8\xbd\xb4A^\xbb\xf9t6?\x0es\xd8bj\xcbk\xa5\xfcm\xf6S\xa7\xd7\xca\xfd\xc6y8\x0e:+\x9f]\xa5j[\x9a\xd5\x1a!\u044b\xad:\xce+\x02\\Fy3>\x99\xbd\xf5\xd12\xa5\xf0N\xf3n?C\x9d\u0103\x15\xdf\u0360\x8a\xef\x06\xc0\xf2\x05b\x83AW/\xbeg\xd3j\u6299\x03)\xca#4\x84\x1e\xe0f\x15o\xfc\x02J\xb1#\xdce\xa8\x83\xdd\xe5?\xc2}\x06\xb9\x05c\x86\x1c-\x8a\xa9F\v]\xb2t[\x1a\x8f\xdd\u060e\xc2\xeeQ\x13\xd1C.\x84t\x81\xc1\xc4\x13P3\x9ce\xddv\x03\xa7\xf3]\xfc_>dZ\u038e\x87\x8b\x9c\xf6\x87kXS|t~\xb7\xaa\x13\x87(W\x03\xcc\xc7\x03s~\xc4C\xdb\x1d\x11\x12\\\xa5\xa7\xc1mnf\xe3\x1d\u02f2\x86\xbbMvD_\xe8r\xa4\xfa^\xac\x0e\x8f\xab`\x97\x86/\x8f\x1f\xa9\xbb\xb9le\xc3\xd9\xcc\x15\xee\xd94/\x8e\f\xc5\x05\xefbNu(y'n\xb1\x15\xd0w0\xe43\u0775\xda\xf1\xad\x16Z.\x95Z\xde4\x1e,\xe0\x8d\x85J\xa1\x01\xa9,\bY6}\x85\xfe\xa9\xa8t\vo^\x16\u032ds\x0e\xb9\x87*=\xc9\xcf\xc7\xd7\xeaX\x89\x9c\xf7\xd4r/\xd6\xea\x04\x8c^\x06*\xe0\x1ar7\u0378OC\x9dX\xbc\xa1\x96\x03\xd3\xfc%\xb6\x9cB\xe6\xef\xbe%:\u007f\x01~<\x83\u007f\v\x1f-%,[\xbc\x0f\x97\xf6\xe6/\xc5%:\u007f\x1f.\xd0\x03Ul9\u031f\xd3q\u822f\xc0\xd1\xd1~\xebQE\xfbG\xa58\x1e\x80\xe7\x9aX\xa7\x12\xec\xff\xbb\xdc]\xbc\xc7\xc9\xe7#\xce\u05f9\xbe\u04db\x05\x8f\xeb\xfc\xad\xf3\x16\xe3\x99u\x0fS\xb8\x18&\xb1=:\x8fWh\xf8m`\xaa<\xed0\xf4\x18\xd8-yyt\x1e\x1a\xd2\xdc\xdb\xc1\xad\u064f\x11c\\\xd3\x1f!V\x03X\xe5e\xf4\xeb\xc0\xe6\xf3\xf1\xd8\xed\x86$\x88\x11\xc4^\x17\x9f3\x8bl\xf1I\x02\xd7\u00f9M\x87\xf9\xc1\x8f\xe9\f\x1f\x8d\xc7F8'w\xe6\x06\xa5\xa1\xb7<\ufb6b\xfe\x8c\vc\xf7X]\x17}\xb0\xaa\xbd\xcb`\\8\u9e0b\xc4y\xa7.=\xb3~K\u02de\xd0\x1a\xf7\x1d\x9a\xe5\xddf\xa6-u\xcdJ\xecc\v\xe7\x8f\x02=\xb0y\xf1\xbfG\x01vo\x1d\xdf\xd9\xe6\xbb,[\u056d\x04\xde\u0654\xdeYk\x95\xac\xe5\x159\xb0$\xf9_\x00\x00\x00\xff\xff\x13*\xa9\xaf`\x16\x00\x00") +// Data size: 1707 bytes. +var cuegenInstanceData = []byte("\x01\x1f\x8b\b\x00\x00\x00\x00\x00\x00\xff\xacX_\x8f\u0736\x11\x97\xce.P\x11i\xdf\xf2X`,\x03Azpu\u0203\x11c\x81\x83\xe1\xc4v\u15e6(\u04a7 8p\xa5\xd1.\x1b\x89TI*\xb9Cn\x1f\u06a6i?P\xbfF\xbfR\xae\x18\x92\x12%\xad\xee\xeck|/\xb7;?\xcep\xe6G\xce\x1f\xee\xafn\xfeu\x92\x9e\xdc\xfc;Io\xfe\x9e$\x9f\xde\xfc\xedA\x9a~ \xa4\xb1\\\x96\xf8\x92[N\xf2\xf4A\xfa\xf0OJ\xd9\xf4$I\x1f\xfe\x91\xdb}\xfaA\x92\xfe\xe2\xb5h\u04247?&I\xf2\x9b\x9b\u007f\x9e\xa4\u9bff\xfa\xba\ucc68E\x134\u007fL\u049b\x1f\x92\xe4\xe3\x9b\u007f\xfc\x03o\x91\f=tB\x96$\xc9O\x1f\xfe\x97{\xf1\x960jAi?\x8d\xa3\xc2\xfag\x85\xf1\xf4\xd3\u03df\xbd\xf7\xd4tV\uf65fC\xaf{5\xa4)\xb4\xbc3\xbe\xad\xc4\u0525B\x16\n\xa3\x87:M\x05\xd1\n\xaa\x83\x8b\f\xcf\xf3i\xbf\xbd`YNc\xc2(\xa4\xceK\x02\x16\vA\x94\x93`\x00\x9a\x80\x8c@CHSE\xa59\"oEB\xf1\x88\xd6H\xc0\xc6\x12\xb1\x02\xd8K;\a,^Z\x02v*F\u701d\"q\xa7\x95US\u007f\x9d\xc0Y\xc2K;\xa0\xa3\xa59\xba\x9d\xf8\x1cQ\x96Qs\xf9\xe2\xe5\x17\x1b\xa0@\f\xfe\xf5\x89\x13\xe5\u01600*m\x85\xec\xb6pv\x06[!\xb9\xbe\xea\xb6\xe3\xd00\x8cJ d%J\u07df\xfc\x01\xd2m\xe0\xd659\x8d\x9dF\x83\x92\x06\x17\xe0t\xb4;\xcd\u06c2\x8d\x83\xd6\x06\x1e\x9d\xe7\xb97)a>bA\x85\x16u;\x99HJ\u0516\v9\xd8\x01\xb3W}SQ\x1f\x9c\xcd%gg\xf0Zi\x18\x86\xd9'\xe0jE\u02ef\x16+\x81SO6\xa5\x16[\uf7ff\xc1O\u0efd(\xf7 \xac\xc1\xa6v=\x94KR-\x95\xfc\x16\xb5\xf5\u0357\xc3\xe7\u007f~\x154\n\xb6\x98\x0e\u01c1\xcf\u0344\xd3K\x1b\xe4\xb5\x1bNg\xc3\xe30\x84-F\xb6\xbcV\xca\xdff?rz\xad\xdco\x9c\x87\u3833\xf2\xd9U\xaa\xb6\xa5A\xad\x11\x12\xbd\u062a\xe3\xbc\"\xc0e\x947\xe3\x93\xd9[\x1f-S\n\xef4\xef\xf63\xd4I\xd3]\xab\x1d\x1fj\xa1\xe5R\xa9\xe5M\xe3\xc1\x02\xdeX\xa8\x14\x1a\x90\u0282\x90e\xd3W\xe8\u07c9J\xb7\xf0\xe6e\xc1\xdc:\xe7\x90{\xa5\xd2{\xfc||\xaa\x8e\x95\xc8yO-\xf7b\xadN\xc0\xe8e\xa0\x02\xae!w\u04cc\xfb4\u0509\xc5\x03j90\u035fa\xcb)d\xfe\xe8[\xa2\xf3\xe7\xdf\xc73\xf8\xb7\xf0\xd1R\u00b2\xc5\xe3pio\xfeL\\\xa2\xf3\xc7\xe1\x02=P\u0156\xc3\xfc9\x1d\x87\x8e\xf8\n\x1c\x1d\xed\xb7\x1eU\xb4\u007fT\x8a\xe3\x01x\xae\x89u*\xc1\xfe\xbf\xcb\xdd\xc5c\x9c|>\xe2|\x9d\xeb;\xbdY\xf0\xb8\xce\xdf:o1\x9eY\xf70\x85\x8ba\x12\u06e3\xf3x\x85\x86\x1f\x06\xa6\xca\xd3\x0eC\x8f\x81\u0752\x97G\xe7\xa1!\u037d\x1d\u071a\xfd\x121\xc65\xfd\x05b5\x80U^F\xbf\x0el>\x1f\x8f\xddnH\x82\x18A\xecu\xf19\xb3\xc8\x16\x9f$p=\x9c\xdbt\x98\x1f\xfc\x98\xce\xf0\xd1xl\x84srgnP\x1az\xcb\xf3\u07ba\xea\u03f80v\x8f\xd5u\xd1\a\xab\u06bb\f\u0185\x93\x8e\xbbH\x9cw\xea\xd23\ubdf4\xec\t\xadq\u07e1Y\xdemf\xdaR\u05ec\xc4>\xb6p\xfe(\xd0\x03\x9b\x17\xff{\x14`\xf7\xd6\xf1\x9dm\xbe\u02f2U\xddJ\xe0\x9dM\u9775V\xc9Z^\x91\x03K\x92\xff\x05\x00\x00\xff\xff\xa6\xedNA]\x16\x00\x00")