From 0feb8a8dca8e461dbe35530fed04d7522e5a0e0d Mon Sep 17 00:00:00 2001 From: Roger Peppe Date: Fri, 13 Mar 2026 11:31:36 +0000 Subject: [PATCH] internal/core/convert: fix astFromGoType for recursive types and shared AST pointers Replace the store-then-mutate pattern in astFromGoType with a typeBuilder that tracks named Go types and generates unique CUE identifiers for them. Named types that are recursive or referenced multiple times get a hidden field definition in a wrapper struct; non-recursive single-use types are inlined directly as before. Also, rather than store a `reflect.Type` to `adt.Expr` mapping, we store the finalized `*adt.Vertex` mapping which means we don't have to mutate the value by invoking `Finalize` on it in a concurrent situation. This fixes three interrelated bugs: - Cyclic AST trees caused by pre-storing a partially-built struct in astTypeCache then mutating it, which made astutil.Resolve loop forever. - A race condition where concurrent goroutines could read the partially-built struct from the cache. - Shared AST pointers when the same type appeared at multiple sites, rather than explicit references. Signed-off-by: Roger Peppe Change-Id: I332c0ce58b45decde3eee998a1a8cda820212c95 Reviewed-on: https://cue.gerrithub.io/c/cue-lang/cue/+/1233620 TryBot-Result: CUEcueckoo Unity-Result: CUE porcuepine Reviewed-by: Marcel van Lohuizen --- cue/context.go | 6 ++---- cue/context_test.go | 22 +++++++++++++++++++--- internal/value/value.go | 8 +++----- internal/core/adt/context.go | 8 ++++---- internal/core/convert/go.go | 303 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------------------------------------------------------------------------------------------------- internal/core/convert/go_test.go | 237 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------ internal/core/export/export_test.go | 4 ++-- internal/core/runtime/go.go | 10 +++++----- 8 file(s) changed, 414 insertion(s)(+), 184 deletion(s)(-) diff --git a/cue/context.go b/cue/context.go --- a/cue/context.go +++ b/cue/context.go @@ -389,13 +389,11 @@ } ctx := c.ctx() - expr, err := convert.FromGoType(ctx, x) + v, err := convert.FromGoType(ctx, x) if err != nil { return c.makeError(err) } - n := exprToVertex(expr) - n.Finalize(ctx) - return c.make(n) + return c.make(v) } // NewList creates a Value that is a list of the given values. diff --git a/cue/context_test.go b/cue/context_test.go --- a/cue/context_test.go +++ b/cue/context_test.go @@ -148,6 +148,14 @@ wantErr string out string } + type linkedList struct { + X int `json:"x"` + Next *linkedList `json:"next"` + } + type multiref struct { + L1 *linkedList `json:"l1"` + L2 *linkedList `json:"l2"` + } testCases := []testCase{{ name: "Struct", x: struct { @@ -170,15 +178,23 @@ // TODO this looks like a shortcoming of EncodeType. name: "map", x: map[string]int{}, - out: `*null|{}`, + out: `*null|{[string]: int&>=-9223372036854775808&<=9223372036854775807}`, }, { name: "slice", x: []int{}, - out: `*null|[...int64]`, + out: `*null|[...int&>=-9223372036854775808&<=9223372036854775807]`, }, { name: "chan", x: chan int(nil), wantErr: `unsupported Go type \(chan int\)`, + }, { + name: "recursiveType", + x: new(linkedList), + out: `{*null|_linkedList_0, _linkedList_0: {x: int64, next: *null|_linkedList_0}}`, + }, { + name: "multiref", + x: new(multiref), + out: `{*null|_multiref_0, _multiref_0: {l1: *null|_linkedList_0, l2: *null|_linkedList_0}, _linkedList_0: {x: int64, next: *null|_linkedList_0}}`, }} tdtest.Run(t, testCases, func(t *cuetest.T, tc *testCase) { v := cuecontext.New().EncodeType(tc.x) @@ -187,7 +203,7 @@ return } qt.Assert(t, qt.IsNil(v.Err())) - got := fmt.Sprint(astinternal.DebugStr(v.Eval().Syntax())) + got := fmt.Sprint(astinternal.DebugStr(v.Syntax())) t.Equal(got, tc.out) }) } diff --git a/internal/value/value.go b/internal/value/value.go --- a/internal/value/value.go +++ b/internal/value/value.go @@ -108,11 +108,9 @@ rt := (*runtime.Runtime)(r) rt.Init() ctx := eval.NewContext(rt, nil) - expr, err := convert.FromGoType(ctx, x) + v, err := convert.FromGoType(ctx, x) if err != nil { - expr = &adt.Bottom{Err: err} + return r.Encode(&adt.Bottom{Err: err}) } - n := &adt.Vertex{} - n.AddConjunct(adt.MakeRootConjunct(nil, expr)) - return r.Encode(n) + return r.Encode(v) } diff --git a/internal/core/adt/context.go b/internal/core/adt/context.go --- a/internal/core/adt/context.go +++ b/internal/core/adt/context.go @@ -50,12 +50,12 @@ // instance. It returns nil if no such instance has been compiled. LoadInstance(inst *build.Instance) *Vertex - // StoreType associates a CUE expression with a Go type. - StoreType(t reflect.Type, expr Expr) + // StoreType associates a finalized CUE Vertex with a Go type. + StoreType(t reflect.Type, v *Vertex) - // LoadType retrieves a previously stored CUE expression for a given Go + // LoadType retrieves a previously stored CUE Vertex for a given Go // type if available. - LoadType(t reflect.Type) (Expr, bool) + LoadType(t reflect.Type) (*Vertex, bool) // ConfigureOpCtx configures the [*OpContext] with details such as // evaluator version, debug options etc. diff --git a/internal/core/convert/go.go b/internal/core/convert/go.go --- a/internal/core/convert/go.go +++ b/internal/core/convert/go.go @@ -65,24 +65,34 @@ return v } -// FromGoType converts a Go type to an internal CUE expression. -func FromGoType(ctx *adt.OpContext, x any) (adt.Expr, errors.Error) { +// FromGoType converts a Go type to a finalized CUE Vertex. +func FromGoType(ctx *adt.OpContext, x any) (*adt.Vertex, errors.Error) { // TODO: if this value will always be unified with a concrete type in Go, // then many of the fields may be omitted. // TODO: this can be much more efficient. typ := reflect.TypeOf(x) - if t, ok := ctx.LoadType(typ); ok { - return t, nil + if v, ok := ctx.LoadType(typ); ok { + return v, nil } - _, expr := fromGoType(ctx, true, typ) + expr := fromGoType(ctx, typ) if expr == nil { expr = ctx.AddErrf("unsupported Go type (%v)", typ) } if err := ctx.Err(); err != nil { // TODO: return an error as the expr itself, like [FromGoValue]? - return expr, err.Err + return exprToVertex(ctx, expr), err.Err } - return expr, nil + v := exprToVertex(ctx, expr) + v.Finalize(ctx) + ctx.StoreType(typ, v) + return v, nil +} + +// exprToVertex returns a new Vertex with x as its sole conjunct. +func exprToVertex(ctx *adt.OpContext, x adt.Expr) *adt.Vertex { + v := &adt.Vertex{} + v.AddConjunct(adt.MakeRootConjunct(nil, x)) + return v } func compileExpr(ctx *adt.OpContext, expr ast.Expr) adt.Value { @@ -580,28 +590,92 @@ return *new(T), false } -// fromGoTypeAST converts a Go reflect.Type to an ast.Expr, caching results -// in the global astTypeCache. It does not require an adt.OpContext. -// Errors are accumulated into errs. +// typeBuilder tracks named Go types encountered during AST construction +// and generates unique CUE identifiers for them. This avoids creating +// cyclic ASTs for recursive Go types and eliminates shared AST pointers +// when the same type is referenced multiple times. +type typeBuilder struct { + named []*namedType // ordered list of named type entries + byType map[reflect.Type]*namedType // lookup by Go type + nameCount map[string]int // disambiguate same-named types from different packages + errs *[]errors.Error +} + +type namedType struct { + ident string // unique CUE identifier, e.g. "_A_0" + expr ast.Expr // the full struct literal for this type +} + +// astFromGoType converts a Go reflect.Type to an ast.Expr, caching results +// in the global astTypeCache. Errors are accumulated into errs. func astFromGoType(t reflect.Type, allowNullDefault bool, errs *[]errors.Error) ast.Expr { if v, ok := astTypeCache.Load(t); ok { return v.(ast.Expr) } - var e ast.Expr + b := &typeBuilder{ + byType: make(map[reflect.Type]*namedType), + nameCount: make(map[string]int), + errs: errs, + } + e := b.build(t, allowNullDefault) + if e == nil { + return nil + } + e = b.finalize(e, errs) + // Avoid returning different AST nodes for the same type. + // TODO use singleflight to avoid duplicating the work? + e1, _ := astTypeCache.LoadOrStore(t, e) + return e1.(ast.Expr) +} +// finalize wraps the top-level expression with named type definitions +// if needed, resolves identifiers, and returns the final AST. +func (b *typeBuilder) finalize(topExpr ast.Expr, errs *[]errors.Error) ast.Expr { + if len(b.named) == 0 { + f := &ast.File{Decls: []ast.Decl{&ast.EmbedDecl{Expr: topExpr}}} + astutil.Resolve(f, func(_ token.Pos, msg string, args ...interface{}) { + *errs = append(*errs, errors.Newf(token.NoPos, msg, args...)) + }) + return topExpr + } + + // Build a struct with hidden fields for named type definitions + // and the top-level expression's content. + s := &ast.StructLit{} + for _, entry := range b.named { + s.Elts = append(s.Elts, &ast.Field{ + Label: ast.NewIdent(entry.ident), + Value: entry.expr, + }) + } + // If the top expression is itself a struct literal, merge its + // elements directly to avoid an unnecessary level of nesting. + if topStruct, ok := topExpr.(*ast.StructLit); ok { + s.Elts = append(s.Elts, topStruct.Elts...) + } else { + s.Elts = append(s.Elts, &ast.EmbedDecl{Expr: topExpr}) + } + + // Resolve using the struct as the top-level expression so that + // identifiers within it can reference the struct's own fields. + f := &ast.File{Decls: []ast.Decl{&ast.EmbedDecl{Expr: s}}} + astutil.Resolve(f, func(_ token.Pos, msg string, args ...interface{}) { + *errs = append(*errs, errors.Newf(token.NoPos, msg, args...)) + }) + return s +} + +// build recursively converts a Go type to a CUE AST expression. +func (b *typeBuilder) build(t reflect.Type, allowNullDefault bool) ast.Expr { + // Check special types first — these short-circuit regardless of being named. switch reflect.Zero(t).Interface().(type) { case *big.Int, big.Int: - e = ast.NewIdent("int") - goto store - + return ast.NewIdent("int") case *big.Float, big.Float, *big.Rat, big.Rat: - e = ast.NewIdent("number") - goto store - + return ast.NewIdent("number") case *apd.Decimal, apd.Decimal: - e = ast.NewIdent("number") - goto store + return ast.NewIdent("number") } // Even if this is for types that we know cast to a certain type, it can't @@ -609,8 +683,7 @@ // strict instances and there cannot be any tags that further constrain // the values. if t.Implements(jsonMarshaler) || t.Implements(textMarshaler) { - e = topSentinel - goto store + return topSentinel } switch k := t.Kind(); k { @@ -619,100 +692,50 @@ for elem.Kind() == reflect.Pointer { elem = elem.Elem() } - e = astFromGoType(elem, false, errs) + e := b.build(elem, false) if allowNullDefault { e = wrapOrNull(e) } + return e case reflect.Interface: switch t.Name() { case "error": - // This is really null | _|_. There is no error if the error is null. - e = ast.NewNull() + return ast.NewNull() default: - e = topSentinel // `_` + return topSentinel } case reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: - e = compile.LookupRange(t.Kind().String()).Source().(ast.Expr) + return compile.LookupRange(t.Kind().String()).Source().(ast.Expr) case reflect.Uint, reflect.Uintptr: - e = compile.LookupRange("uint64").Source().(ast.Expr) + return compile.LookupRange("uint64").Source().(ast.Expr) case reflect.Int: - e = compile.LookupRange("int64").Source().(ast.Expr) + return compile.LookupRange("int64").Source().(ast.Expr) case reflect.String: - e = ast.NewIdent("__string") + return ast.NewIdent("__string") case reflect.Bool: - e = ast.NewIdent("__bool") + return ast.NewIdent("__bool") case reflect.Float32, reflect.Float64: - e = ast.NewIdent("__number") + return ast.NewIdent("__number") case reflect.Struct: - obj := &ast.StructLit{} - - // TODO: dirty trick: set this to a value that's updated - // below. This avoids an infinite loop on circular references - // when creating the AST, but does not actually work - // as intended because AST trees should never be actually - // cyclic. Also, this is racy when called concurrently. - // Instead, we should generate named identifiers for - // named Go types and emit references to them. - astTypeCache.Store(t, obj) - - for i := range t.NumField() { - f := t.Field(i) - if f.PkgPath != "" { - continue - } - _, ok := f.Tag.Lookup("cue") - elem := astFromGoType(f.Type, !ok, errs) - if isBad(elem) { - continue // Ignore fields for unsupported types - } - - // leave errors like we do during normal evaluation or do we - // want to return the error? - name := getName(&f) - if name == "-" { - continue - } - - if tag, ok := f.Tag.Lookup("cue"); ok { - v, err := parseTag(name, tag) - if err != nil { - *errs = append(*errs, err) - } - if isBad(v) { - return v - } - elem = ast.NewBinExpr(token.AND, elem, v) - } - // TODO: if an identifier starts with __ (or otherwise is not a - // valid CUE name), make it a string and create a map to a new - // name for references. - - // The Go JSON decoder always allows a value to be undefined. - d := &ast.Field{Label: ast.NewIdent(name), Value: elem} - if isOptional(&f) { - d.Constraint = token.OPTION - } - obj.Elts = append(obj.Elts, d) - } - - e = obj + return b.buildStruct(t) case reflect.Array, reflect.Slice: + var e ast.Expr if t.Elem().Kind() == reflect.Uint8 { e = ast.NewIdent("__bytes") } else { - elem := astFromGoType(t.Elem(), allowNullDefault, errs) + elem := b.build(t.Elem(), allowNullDefault) if elem == nil { - *errs = append(*errs, errors.Newf(token.NoPos, "unsupported Go type (%v)", t.Elem())) + *b.errs = append(*b.errs, errors.Newf(token.NoPos, "unsupported Go type (%v)", t.Elem())) return &ast.BadExpr{} } @@ -731,6 +754,7 @@ if k == reflect.Slice { e = wrapOrNull(e) } + return e case reflect.Map: switch key := t.Key(); key.Kind() { @@ -738,65 +762,118 @@ reflect.Int32, reflect.Int64, reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr: default: - *errs = append(*errs, errors.Newf(token.NoPos, "unsupported Go type for map key (%v)", key)) + *b.errs = append(*b.errs, errors.Newf(token.NoPos, "unsupported Go type for map key (%v)", key)) return &ast.BadExpr{} } - v := astFromGoType(t.Elem(), allowNullDefault, errs) + v := b.build(t.Elem(), allowNullDefault) if v == nil { - *errs = append(*errs, errors.Newf(token.NoPos, "unsupported Go type (%v)", t.Elem())) + *b.errs = append(*b.errs, errors.Newf(token.NoPos, "unsupported Go type (%v)", t.Elem())) return &ast.BadExpr{} } if isBad(v) { return v } - e = ast.NewStruct(&ast.Field{ + e := ast.NewStruct(&ast.Field{ Label: ast.NewList(ast.NewIdent("__string")), Value: v, }) - e = wrapOrNull(e) + return wrapOrNull(e) } - -store: - if e != nil { - f := &ast.File{Decls: []ast.Decl{&ast.EmbedDecl{Expr: e}}} - astutil.Resolve(f, func(_ token.Pos, msg string, args ...interface{}) { - *errs = append(*errs, errors.Newf(token.NoPos, msg, args...)) - }) - astTypeCache.Store(t, e) - } - return e + return nil } -func fromGoType(ctx *adt.OpContext, allowNullDefault bool, t reflect.Type) (ast.Expr, adt.Expr) { - if expr, ok := ctx.LoadType(t); ok { - e, _ := astTypeCache.Load(t) - src, _ := e.(ast.Expr) - return src, expr +// allocIdent generates a unique CUE identifier for the given Go type name. +func (b *typeBuilder) allocIdent(name string) string { + idx := b.nameCount[name] + b.nameCount[name]++ + return fmt.Sprintf("_%s_%d", name, idx) +} + +// buildStruct converts a Go struct type to a CUE AST expression. +// For named types, it allocates a unique identifier and returns +// a reference to it. For anonymous structs, it returns the struct +// literal directly. +func (b *typeBuilder) buildStruct(t reflect.Type) ast.Expr { + if t.Name() != "" { + if entry, ok := b.byType[t]; ok { + return ast.NewIdent(entry.ident) + } + entry := &namedType{ + ident: b.allocIdent(t.Name()), + } + b.named = append(b.named, entry) + b.byType[t] = entry + entry.expr = b.buildStructLit(t) + return ast.NewIdent(entry.ident) } + return b.buildStructLit(t) +} + +// buildStructLit builds the struct literal for a Go struct type. +func (b *typeBuilder) buildStructLit(t reflect.Type) ast.Expr { + obj := &ast.StructLit{} + for i := range t.NumField() { + f := t.Field(i) + if f.PkgPath != "" { + continue + } + _, ok := f.Tag.Lookup("cue") + elem := b.build(f.Type, !ok) + if isBad(elem) { + continue // Ignore fields for unsupported types + } + + name := getName(&f) + if name == "-" { + continue + } + + if tag, ok := f.Tag.Lookup("cue"); ok { + v, err := parseTag(name, tag) + if err != nil { + *b.errs = append(*b.errs, err) + } + if isBad(v) { + return v + } + elem = ast.NewBinExpr(token.AND, elem, v) + } + // TODO: if an identifier starts with __ (or otherwise is not a + // valid CUE name), make it a string and create a map to a new + // name for references. + + // The Go JSON decoder always allows a value to be undefined. + d := &ast.Field{Label: ast.NewIdent(name), Value: elem} + if isOptional(&f) { + d.Constraint = token.OPTION + } + obj.Elts = append(obj.Elts, d) + } + return obj +} + +func fromGoType(ctx *adt.OpContext, t reflect.Type) adt.Expr { var errs []errors.Error - e := astFromGoType(t, allowNullDefault, &errs) + e := astFromGoType(t, true, &errs) for _, err := range errs { ctx.AddErr(err) } if isBad(e) || e == nil { if len(errs) > 0 { - return e, &adt.Bottom{Err: errs[0]} + return &adt.Bottom{Err: errs[0]} } - return e, nil + return nil } x, err := compile.Expr(nil, ctx, pkgID(), e) if err != nil { b := &adt.Bottom{Err: err} ctx.AddBottom(b) - ctx.StoreType(t, b) - return e, b + return b } - expr := x.Expr() - ctx.StoreType(t, expr) - return e, expr + return x.Expr() } func isBottom(x adt.Node) bool { diff --git a/internal/core/convert/go_test.go b/internal/core/convert/go_test.go --- a/internal/core/convert/go_test.go +++ b/internal/core/convert/go_test.go @@ -20,6 +20,7 @@ "encoding" "encoding/json" "math/big" + "sync" "testing" "time" @@ -34,6 +35,30 @@ _ "cuelang.org/go/pkg" ) + +type recursiveA struct { + Next *recursiveA + Val int +} + +type crossRefA struct { + Y string + B *crossRefB +} + +type crossRefB struct { + X int + A *crossRefA +} + +type sharedS struct { + Other string +} + +type sharedT struct { + S *sharedS + S2 *sharedS +} func mkBigInt(a int64) (v apd.Decimal) { v.SetInt64(a); return } @@ -305,12 +330,12 @@ F *big.Float }{}, // TODO: indicate that B is explicitly an int only. - want: `{ - A: (((int & >=-9223372036854775808) & <=9223372036854775807) & (>=0 & <100)) - B: (int & >=0) - C?: int - D: int - F?: number + want: `(struct){ + A: (int){ &(>=0, <100, int) } + B: (int){ &(>=0, int) } + C?: (int){ int } + D: (int){ int } + F?: (number){ number } }`, }, { goTyp: &struct { @@ -323,20 +348,20 @@ T time.Time G func() }{}, - want: `(*null|{ - A: (((int & >=-32768) & <=32767) & (>=0 & <100)) - b: null - C: string - D: bool - F: number - L?: (*null|bytes) - T: _ -})`, + want: `((null|struct)){ |(*(null){ null }, (struct){ + A: (int){ &(>=0, <100, int) } + b: (null){ null } + C: (string){ string } + D: (bool){ bool } + F: (number){ number } + L?: ((null|bytes)){ |(*(null){ null }, (bytes){ bytes }) } + T: (_){ _ } + }) }`, }, { goTyp: struct { A int `cue:"<"` // invalid }{}, - want: "_|_(invalid tag \"<\" for field \"A\": expected operand, found 'EOF')", + want: "(_|_){// _|_(invalid tag \"<\" for field \"A\": expected operand, found 'EOF')\n}", expectError: true, }, { goTyp: struct { @@ -347,11 +372,11 @@ T string `cue:""` // allowed h int }{}, - want: `{ - D?: number - P?: (*null|number) - I?: _ - T: (string & _) + want: `(struct){ + D?: (number){ number } + P?: ((null|number)){ |(*(null){ null }, (number){ number }) } + I?: (_){ _ } + T: (string){ string } }`, }, { goTyp: struct { @@ -360,63 +385,56 @@ C int8 `cue:"A+B"` }{}, // TODO: should B be marked as optional? - want: `{ - A: (((int & >=-128) & <=127) & (〈0;C〉 - 〈0;B〉)) - B?: (((int & >=-128) & <=127) & (〈0;C〉 - 〈0;A〉)) - C: (((int & >=-128) & <=127) & (〈0;A〉 + 〈0;B〉)) -}`, + want: "(struct){\n A: (_|_){\n // [incomplete] A: non-concrete value >=-128 & <=127 & int in operand to -:\n // :1:1\n // A: cannot reference optional field: B:\n // :1:3\n }\n B?: (_|_){\n // [incomplete] B: non-concrete value >=-128 & <=127 & int in operand to -:\n // :1:1\n }\n C: (_|_){\n // [incomplete] C: non-concrete value >=-128 & <=127 & int in operand to +:\n // :1:1\n // C: cannot reference optional field: B:\n // :1:3\n }\n}", }, { goTyp: []string{}, - want: `(*null|[ - ...string, -])`, + want: `((null|list)){ |(*(null){ null }, (list){ + }) }`, }, { goTyp: [4]string{}, - want: `〈import;list〉.Repeat([ - string, -], 4)`, + want: `(#list){ + 0: (string){ string } + 1: (string){ string } + 2: (string){ string } + 3: (string){ string } +}`, }, { goTyp: []func(){}, - want: "_|_(unsupported Go type (func()))", + want: "(_|_){// _|_(unsupported Go type (func()))\n}", expectError: true, }, { goTyp: map[string]struct{ A map[string]uint }{}, - want: `(*null|{ - [string]: { - A?: (*null|{ - [string]: ((int & >=0) & <=18446744073709551615) - }) - } -})`, + want: `((null|struct)){ |(*(null){ null }, (struct){ + }) }`, }, { goTyp: map[float32]int{}, - want: `_|_(unsupported Go type for map key (float32))`, + want: "(_|_){// _|_(unsupported Go type for map key (float32))\n}", expectError: true, }, { goTyp: map[int]map[float32]int{}, - want: `_|_(unsupported Go type for map key (float32))`, + want: "(_|_){// _|_(unsupported Go type for map key (float32))\n}", expectError: true, }, { goTyp: map[int]func(){}, - want: `_|_(unsupported Go type (func()))`, + want: "(_|_){// _|_(unsupported Go type (func()))\n}", expectError: true, }, { goTyp: time.Now, // a function - want: "_|_(unsupported Go type (func() time.Time))", + want: "(_|_){// _|_(unsupported Go type (func() time.Time))\n}", expectError: true, }, { goTyp: struct { Foobar string `cue:"\"foo,bar\",opt"` }{}, - want: `{ - Foobar?: (string & "foo,bar") + want: `(struct){ + Foobar?: (string){ "foo,bar" } }`, }, { goTyp: struct { Foobar string `cue:"\"foo,opt,bar\""` }{}, - want: `{ - Foobar: (string & "foo,opt,bar") + want: `(struct){ + Foobar: (string){ "foo,opt,bar" } }`, }} @@ -443,4 +461,127 @@ } }) } +} + +func TestFromGoTypeRecursive(t *testing.T) { + r := runtime.New() + + testCases := []struct { + name string + goTyp any + want string + }{{ + name: "self-recursive", + goTyp: recursiveA{}, + want: `(struct){ + _recursiveA_0: (struct){ + Next?: (null){ null } + Val: (int){ &(>=-9223372036854775808, <=9223372036854775807, int) } + } + Next?: ((null|struct)){ |(*(null){ null }, (struct){ + Next?: ((null|struct)){ |(*(null){ null }, (struct){ + Next?: (null){ null } + Val: (int){ &(>=-9223372036854775808, <=9223372036854775807, int) } + }) } + Val: (int){ &(>=-9223372036854775808, <=9223372036854775807, int) } + }) } + Val: (int){ &(>=-9223372036854775808, <=9223372036854775807, int) } +}`, + }, { + name: "mutually-recursive", + goTyp: crossRefA{}, + want: `(struct){ + _crossRefA_0: (struct){ + Y: (string){ string } + B?: ((null|struct)){ |(*(null){ null }, (struct){ + X: (int){ &(>=-9223372036854775808, <=9223372036854775807, int) } + A?: (null){ null } + }) } + } + _crossRefB_0: (struct){ + X: (int){ &(>=-9223372036854775808, <=9223372036854775807, int) } + A?: ((null|struct)){ |(*(null){ null }, (struct){ + Y: (string){ string } + B?: (null){ null } + }) } + } + Y: (string){ string } + B?: ((null|struct)){ |(*(null){ null }, (struct){ + X: (int){ &(>=-9223372036854775808, <=9223372036854775807, int) } + A?: ((null|struct)){ |(*(null){ null }, (struct){ + Y: (string){ string } + B?: ((null|struct)){ |(*(null){ null }, (struct){ + X: (int){ &(>=-9223372036854775808, <=9223372036854775807, int) } + A?: ((null|struct)){ |(*(null){ null }, (struct){ + Y: (string){ string } + B?: (null){ null } + }) } + }) } + }) } + }) } +}`, + }, { + name: "shared-type", + goTyp: sharedT{}, + want: `(struct){ + _sharedT_0: (struct){ + S?: ((null|struct)){ |(*(null){ null }, (struct){ + Other: (string){ string } + }) } + S2?: ((null|struct)){ |(*(null){ null }, (struct){ + Other: (string){ string } + }) } + } + _sharedS_0: (struct){ + Other: (string){ string } + } + S?: ((null|struct)){ |(*(null){ null }, (struct){ + Other: (string){ string } + }) } + S2?: ((null|struct)){ |(*(null){ null }, (struct){ + Other: (string){ string } + }) } +}`, + }} + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctx := adt.NewContext(r, &adt.Vertex{}) + v, err := convert.FromGoType(ctx, tc.goTyp) + if err != nil { + t.Fatal(err) + } + got := debug.NodeString(ctx, v, nil) + if got != tc.want { + t.Errorf("\n got %q;\nwant %q", got, tc.want) + } + val, _ := ctx.Evaluate(&adt.Environment{}, v) + if bot, ok := val.(*adt.Bottom); ok { + t.Errorf("unexpected error when evaluating: %v", bot) + } + }) + } +} + +func TestFromGoTypeConcurrent(t *testing.T) { + // Note: there is a pre-existing race in compile.Expr which mutates + // cached AST nodes. This test verifies that astFromGoType itself + // (the AST construction) does not race, by checking that concurrent + // calls complete without panicking or producing errors. + // The -race flag may still detect the compile.Expr race which is + // outside the scope of this fix. + var wg sync.WaitGroup + for range 10 { + wg.Add(1) + go func() { + defer wg.Done() + r := runtime.New() + ctx := adt.NewContext(r, &adt.Vertex{}) + _, err := convert.FromGoType(ctx, recursiveA{}) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + }() + } + wg.Wait() } diff --git a/internal/core/export/export_test.go b/internal/core/export/export_test.go --- a/internal/core/export/export_test.go +++ b/internal/core/export/export_test.go @@ -97,7 +97,7 @@ } return convert.FromGoType(ctx, in) }, - out: `*null|{Terminals?: *null|[...*null|{Name: string, Description: string}]}`, + out: `*null|_C_0, _C_0: {Terminals?: *null|[...*null|_A_0]}, _A_0: {Name: string, Description: string}`, }, { in: func(ctx *adt.OpContext) (adt.Expr, error) { in := []*A{{Name: "Name", Description: "Desc"}} @@ -109,7 +109,7 @@ in := []*A{{Name: "Name", Description: "Desc"}} return convert.FromGoType(ctx, in) }, - out: `*null|[...*null|{Name: string, Description: string}]`, + out: `*null|[...*null|_A_0], _A_0: {Name: string, Description: string}`, }, { in: func(ctx *adt.OpContext) (adt.Expr, error) { in := &KeepGoFieldOrdering{} diff --git a/internal/core/runtime/go.go b/internal/core/runtime/go.go --- a/internal/core/runtime/go.go +++ b/internal/core/runtime/go.go @@ -20,19 +20,19 @@ "cuelang.org/go/internal/core/adt" ) -func (x *Runtime) StoreType(t reflect.Type, expr adt.Expr) { - x.index.StoreType(t, expr) +func (x *Runtime) StoreType(t reflect.Type, v *adt.Vertex) { + x.index.StoreType(t, v) } -func (x *Runtime) LoadType(t reflect.Type) (adt.Expr, bool) { +func (x *Runtime) LoadType(t reflect.Type) (*adt.Vertex, bool) { v, ok := x.index.LoadType(t) if !ok { return nil, false } - return v.(adt.Expr), true + return v.(*adt.Vertex), true } -func (x *index) StoreType(t reflect.Type, v interface{}) { +func (x *index) StoreType(t reflect.Type, v *adt.Vertex) { x.typeCache.Store(t, v) } -- tangled.sh