Skip to content

Commit 61e0d7f

Browse files
committed
internal/encoding/gotypes: handle cyclic types out of the box
So that the linked list example doesn't need an explicit attribute to use a pointer so the resulting Go type can compile. While here, fix up bits of the code that still named typeFacts variables as "info" from a previous iteration where the type was typeInfo. And we also don't need a strings.Builder in emitTypeReference now. Signed-off-by: Daniel Martí <[email protected]> Change-Id: I79e8381052c0a6eefb3341a08c9de456977b1e86 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1214810 Unity-Result: CUE porcuepine <[email protected]> TryBot-Result: CUEcueckoo <[email protected]> Reviewed-by: Roger Peppe <[email protected]>
1 parent e7d687f commit 61e0d7f

File tree

2 files changed

+59
-39
lines changed

2 files changed

+59
-39
lines changed

cmd/cue/cmd/testdata/script/exp_gengotypes.txtar

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,8 @@ _typeTests: [...#typeTest] & [
163163
{name: "NonEmptyString", fail: both: [1, 2, 3]},
164164
{name: "LinkedList", pass: {item: 12, next: {item: 34}}},
165165
{name: "LinkedList", fail: both: {next: "x"}},
166+
{name: "GraphNode", pass: {edges: [{}, {edges: [{}]}]}},
167+
{name: "GraphNode", fail: both: {edges: {}}},
166168
]
167169
-- cuetest/fail_check.stderr --
168170
fail.both."16_IntList".types.IntList.0: conflicting values "foo" and int (mismatched types string and int):
@@ -185,7 +187,10 @@ fail.both."40_NonEmptyString".types.NonEmptyString: conflicting values string an
185187
./root/types.cue:38:23
186188
fail.both."42_LinkedList".types.LinkedList.next: conflicting values "x" and {item?:_,next?:#linkedList} (mismatched types string and struct):
187189
./cuetest/all.cue:94:50
188-
./root/types.cue:43:14
190+
./root/types.cue:44:14
191+
fail.both."44_GraphNode".types.GraphNode.edges: conflicting values [...#graphNode] and {} (mismatched types list and struct):
192+
./cuetest/all.cue:96:51
193+
./root/types.cue:50:10
189194
fail.both.notList: conflicting values [1,2,3] and {embedded2?:int} (mismatched types list and struct):
190195
./cuetest/all.cue:5:24
191196
./root/root.cue:136:2
@@ -591,14 +596,16 @@ import (
591596
NonEmptyString?: string & != ""
592597
UniqueStrings?: list.UniqueItems & [... string]
593598
LinkedList?: #linkedList
599+
GraphNode?: #graphNode
594600
}
595601

596602
#linkedList: {
597603
item?: _
598-
// TODO: it would be nice to not have to specify optional= here;
599-
// we could determine that this is a recursive type, so a pointer must be used
600-
// as otherwise the Go type tries to have infinite size.
601-
next?: #linkedList @go(,optional=nillable)
604+
next?: #linkedList
605+
}
606+
607+
#graphNode: {
608+
edges?: [...#graphNode]
602609
}
603610
-- root/import.cue --
604611
package root
@@ -707,17 +714,20 @@ type Types struct {
707714
UniqueStrings []string `json:"UniqueStrings,omitempty"`
708715

709716
LinkedList LinkedList `json:"LinkedList,omitempty"`
717+
718+
GraphNode GraphNode `json:"GraphNode,omitempty"`
710719
}
711720

712721
type LinkedList struct {
713722
Item any/* CUE top */ `json:"item,omitempty"`
714723

715-
// TODO: it would be nice to not have to specify optional= here;
716-
// we could determine that this is a recursive type, so a pointer must be used
717-
// as otherwise the Go type tries to have infinite size.
718724
Next *LinkedList `json:"next,omitempty"`
719725
}
720726

727+
type GraphNode struct {
728+
Edges []*GraphNode `json:"edges,omitempty"`
729+
}
730+
721731
// emptyStruct has a doc comment.
722732
type EmptyStruct struct {
723733
}

internal/encoding/gotypes/generate.go

Lines changed: 41 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -292,11 +292,11 @@ func (g *generator) genDef(path cue.Path, val cue.Value) (*generatedDef, error)
292292
def := &generatedDef{inProgress: true}
293293
g.def = def
294294
g.generatedTypes[qpath] = def
295-
info, err := g.emitType(val, optionalZero)
295+
facts, err := g.emitType(val, optionalZero)
296296
if err != nil {
297297
return nil, err
298298
}
299-
g.def.facts = info
299+
g.def.facts = facts
300300
g.def.inProgress = false
301301
g.def = parentDef
302302
return def, nil
@@ -356,10 +356,10 @@ func (g *generator) emitType(val cue.Value, optionalStg optionalStrategy) (typeF
356356
// Note that type references don't get optionalStg,
357357
// as @go(,optional=) only affects fields under the current type expression.
358358
// TODO: support nullable types, such as `null | #SomeReference` and `null | {foo: int}`.
359-
if done, info, err := g.emitTypeReference(val); err != nil {
360-
return info, err
359+
if done, facts, err := g.emitTypeReference(val); err != nil {
360+
return typeFacts{}, err
361361
} else if done {
362-
return info, nil
362+
return facts, nil
363363
}
364364

365365
// Inline types are below.
@@ -443,18 +443,18 @@ func (g *generator) emitType(val cue.Value, optionalStg optionalStrategy) (typeF
443443
}
444444
g.def.printf("%s ", goName)
445445

446-
// Pointers in Go are a prefix in the syntax, but we won't find out the generated type info
446+
// Pointers in Go are a prefix in the syntax, but we won't find out the generated type facts
447447
// until we have emitted its Go source, which we do into the same buffer to avoid copies.
448448
// Luckily, since a pointer is always one byte, and we gofmt the result anyway for nice formatting,
449449
// we can add the pointer first and replace it with whitespace later if not wanted.
450450
ptrOffset := len(g.def.src)
451451
g.def.printf("*")
452452

453-
info, err := g.emitType(val, optionalStg)
453+
facts, err := g.emitType(val, optionalStg)
454454
if err != nil {
455-
return info, err
455+
return facts, err
456456
}
457-
if !usePointer(info, optional, optionalStg) {
457+
if !usePointer(facts, optional, optionalStg) {
458458
g.def.src[ptrOffset] = ' '
459459
}
460460

@@ -522,8 +522,8 @@ func (g *generator) emitType(val cue.Value, optionalStg optionalStrategy) (typeF
522522
return facts, nil
523523
}
524524

525-
func usePointer(info typeFacts, optional bool, strategy optionalStrategy) bool {
526-
if info.isTypeOverride {
525+
func usePointer(facts typeFacts, optional bool, strategy optionalStrategy) bool {
526+
if facts.isTypeOverride {
527527
// @(,type=) overrides any @(,optional=) setting
528528
return false
529529
}
@@ -536,7 +536,7 @@ func usePointer(info typeFacts, optional bool, strategy optionalStrategy) bool {
536536
return false
537537
case optionalNillable:
538538
// Only use a pointer when the type isn't already nillable.
539-
return !info.isNillable
539+
return !facts.isNillable
540540
default:
541541
panic("unreachable")
542542
}
@@ -690,26 +690,19 @@ func goPkgNameForInstance(inst *build.Instance, instVal cue.Value) string {
690690
// emitTypeReference attempts to generate a CUE value as a Go type via a reference,
691691
// either to a type in the same Go package, or to a type in an imported package.
692692
func (g *generator) emitTypeReference(val cue.Value) (bool, typeFacts, error) {
693-
var facts typeFacts
694693
// References to existing names, either from the same package or an imported package.
695694
root, path := val.ReferencePath()
696695
// TODO: surely there is a better way to check whether ReferencePath returned "no path",
697696
// such as a possible path.IsValid method?
698697
if len(path.Selectors()) == 0 {
699-
return false, facts, nil
698+
return false, typeFacts{}, nil
700699
}
701700
inst := root.BuildInstance()
702701
// Go has no notion of qualified import paths; if a CUE file imports
703702
// "foo.com/bar:qualified", we import just "foo.com/bar" on the Go side.
704703
// TODO: deal with multiple packages existing in the same directory.
705704
unqualifiedPath := ast.ParseImportPath(inst.ImportPath).Unqualified().String()
706705

707-
var sb strings.Builder
708-
if root != g.pkgRoot {
709-
sb.WriteString(goPkgNameForInstance(inst, root))
710-
sb.WriteString(".")
711-
}
712-
713706
// As a special case, some CUE standard library types are allowed as references
714707
// even though they aren't definitions.
715708
defsOnly := true
@@ -718,33 +711,50 @@ func (g *generator) emitTypeReference(val cue.Value) (bool, typeFacts, error) {
718711
// Note that CUE represents durations as strings, but Go as int64.
719712
// TODO: can we do better here, such as a custom duration type?
720713
g.def.printf("string /* CUE time.Duration */")
721-
return true, facts, nil
714+
return true, typeFacts{}, nil
722715
case "time.Time":
723716
defsOnly = false
724717
}
725718

726719
name := goNameFromPath(path, defsOnly)
727720
if name == "" {
728-
return false, facts, nil // Not a path we are generating.
721+
return false, typeFacts{}, nil // Not a path we are generating.
729722
}
730723

731-
sb.WriteString(name)
732-
g.def.printf("%s", sb.String())
733-
724+
var facts typeFacts
725+
inProgress := false
734726
// We did use a reference; if the referenced name was from another package,
735727
// we need to ensure that package is imported.
736728
// Otherwise, we need to ensure that the referenced local definition is generated.
737729
// Either way, return the facts about the referenced type.
738730
if root != g.pkgRoot {
739-
// TODO: populate the facts here, which will require generating imported packages first.
740731
g.importCuePkgAsGoPkg[inst.ImportPath] = unqualifiedPath
741-
return true, facts, nil
732+
// TODO: populate the facts here, which will require generating imported packages first.
733+
} else {
734+
def, err := g.genDef(path, cue.Dereference(val))
735+
if err != nil {
736+
return false, typeFacts{}, err
737+
}
738+
facts = def.facts
739+
inProgress = def.inProgress
742740
}
743-
def, err := g.genDef(path, cue.Dereference(val))
744-
if err != nil {
745-
return false, facts, err
741+
// We generate types depth-first; if the type referenced here is still in progress,
742+
// it means that we are in a cyclic type, so be nillable to avoid a Go type of infinite size.
743+
// Note that sometimes we're in a complex type which is already nillable, such as:
744+
//
745+
// #GraphNode: {edges?: [...#GraphNode]}
746+
//
747+
// So we could generate the Go field as `[]GraphNode` rather than `[]*GraphNode`,
748+
// given that Go slices are already nillable, but we currently do use a pointer.
749+
if inProgress && !facts.isNillable {
750+
g.def.printf("*")
751+
facts.isNillable = true // pointers can be nil
752+
}
753+
if root != g.pkgRoot {
754+
g.def.printf("%s.", goPkgNameForInstance(inst, root))
746755
}
747-
return true, def.facts, nil
756+
g.def.printf("%s", name)
757+
return true, facts, nil
748758
}
749759

750760
// emitDocs generates the documentation comments attached to the following declaration.

0 commit comments

Comments
 (0)