From 880b903088c1a41b2f17d540f22c82a6d1b5d209 Mon Sep 17 00:00:00 2001 From: ZhouGuangyuan Date: Tue, 4 Aug 2026 08:41:38 +0800 Subject: [PATCH 1/2] ssa: make recursive type conversion order-independent --- ssa/type_cvt.go | 172 ++++++++++++++++++++++++++++++++++---- ssa/type_cvt_test.go | 195 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 353 insertions(+), 14 deletions(-) create mode 100644 ssa/type_cvt_test.go diff --git a/ssa/type_cvt.go b/ssa/type_cvt.go index bd8ab36ed4..a5302a77b4 100644 --- a/ssa/type_cvt.go +++ b/ssa/type_cvt.go @@ -28,15 +28,27 @@ import ( // ----------------------------------------------------------------------------- type goTypes struct { - typs map[unsafe.Pointer]unsafe.Pointer - typbg sync.Map + // typs and cvtneed are owned by the single lowering goroutine for one + // Program. typbg is populated during concurrent package syntax preloading, + // before lowering starts, so it remains a sync.Map. + typs map[unsafe.Pointer]unsafe.Pointer + cvtneed map[*types.Named]conversionRequirement + typbg sync.Map } func newGoTypes() goTypes { typs := make(map[unsafe.Pointer]unsafe.Pointer) - return goTypes{typs: typs} + return goTypes{typs: typs, cvtneed: make(map[*types.Named]conversionRequirement)} } +type conversionRequirement uint8 + +const ( + conversionUnknown conversionRequirement = iota + conversionNotNeeded + conversionNeeded +) + type Background int const ( @@ -101,7 +113,7 @@ func (p goTypes) cvtType(typ types.Type) (raw types.Type, cvt bool) { } return p.cvtStruct(t) case *types.Named: - if v, ok := p.typbg.Load(namedLinkname(t)); ok && v.(Background) == InC { + if !p.shouldConvertNamed(t) { break } return p.cvtNamed(t) @@ -161,37 +173,169 @@ func namedLinkname(t *types.Named) string { return obj.Name() } +func (p goTypes) shouldConvertNamed(t *types.Named) bool { + v, ok := p.typbg.Load(namedLinkname(t)) + return !ok || v.(Background) != InC +} + func (p goTypes) cvtNamed(t *types.Named) (raw *types.Named, cvt bool) { if v, ok := p.typs[unsafe.Pointer(t)]; ok { raw = (*types.Named)(v) cvt = t != raw return } + // Decide whether the complete recursive type graph needs conversion before + // installing the recursion placeholder. Previously the placeholder was the + // original type. For mutually recursive named types, that made the result + // depend on which member of the cycle happened to be converted first: a + // closure reachable through a later member could leave an earlier member + // permanently cached in its unconverted form. + if !p.namedNeedsTypeConversion(t) { + p.typs[unsafe.Pointer(t)] = unsafe.Pointer(t) + return t, false + } n := t.NumMethods() methods := make([]*types.Func, n) for i := 0; i < n; i++ { m := t.Method(i) // don't need to convert method signature methods[i] = m } - named := types.NewNamed(t.Obj(), types.Typ[types.Int], methods) + origin := types.NewNamed(t.Obj(), types.Typ[types.Int], methods) if tp := t.TypeParams(); tp != nil { list := make([]*types.TypeParam, tp.Len()) for i := 0; i < tp.Len(); i++ { param := tp.At(i) list[i] = types.NewTypeParam(param.Obj(), param.Constraint()) } - named.SetTypeParams(list) + origin.SetTypeParams(list) } - p.typs[unsafe.Pointer(t)] = unsafe.Pointer(t) - if tund, cvt := p.cvtType(t.Underlying()); cvt { - named.SetUnderlying(tund) - if typ, ok := Instantiate(named, t); ok { - named = typ.(*types.Named) + named := origin + if typ, ok := Instantiate(origin, t); ok { + named = typ.(*types.Named) + } + // Publish the converted placeholder before descending so every back-edge in + // the cycle observes the same conversion decision. + p.typs[unsafe.Pointer(t)] = unsafe.Pointer(named) + tund, _ := p.cvtType(t.Underlying()) + // Generic instances derive their underlying type lazily from the origin. + // Fill the origin before any caller observes named.Underlying(), so a + // recursive My[T] back-edge resolves to the converted My[args] instance. + origin.SetUnderlying(tund) + return named, true +} + +type conversionNeedState struct { + visiting bool + seen bool +} + +type conversionNeedQuery map[*types.Named]conversionNeedState + +func (p goTypes) namedNeedsTypeConversion(t *types.Named) bool { + if requirement := p.cvtneed[t]; requirement != conversionUnknown { + return requirement == conversionNeeded + } + query := make(conversionNeedQuery) + needed := p.needsTypeConversion(t, query) + if !needed { + // A complete negative query proves that every named type it reached is + // also conversion-free. Negative results observed only on a cycle + // back-edge are never stored here. + for named, state := range query { + if state.seen { + p.cvtneed[named] = conversionNotNeeded + } } - p.typs[unsafe.Pointer(t)] = unsafe.Pointer(named) - return named, true } - return t, false + return needed +} + +// needsTypeConversion reports whether cvtType changes any part of typ. The +// recursion set deliberately belongs to one query: a cycle back-edge alone is +// not a conversion, but another member of that cycle may still require one. +// Keep its traversal and conversion predicates in lock-step with cvtType. +func (p goTypes) needsTypeConversion(typ types.Type, query conversionNeedQuery) bool { + if _, ok := cvtGoSSAOpaqueType(typ); ok { + return true + } + switch t := typ.(type) { + case *types.Basic: + return false + case *types.Pointer: + return p.needsTypeConversion(t.Elem(), query) + case *types.Interface: + for i := 0; i < t.NumExplicitMethods(); i++ { + sig := t.ExplicitMethod(i).Type().(*types.Signature) + if p.needsTypeConversion(sig.Params(), query) || p.needsTypeConversion(sig.Results(), query) { + return true + } + } + for i := 0; i < t.NumEmbeddeds(); i++ { + if p.needsTypeConversion(t.EmbeddedType(i), query) { + return true + } + } + return false + case *types.Slice: + return p.needsTypeConversion(t.Elem(), query) + case *types.Map: + return p.needsTypeConversion(t.Key(), query) || p.needsTypeConversion(t.Elem(), query) + case *types.Struct: + if IsClosure(t) { + return false + } + for i := 0; i < t.NumFields(); i++ { + if p.needsTypeConversion(t.Field(i).Type(), query) { + return true + } + } + return false + case *types.Named: + if !p.shouldConvertNamed(t) { + return false + } + if requirement := p.cvtneed[t]; requirement != conversionUnknown { + return requirement == conversionNeeded + } + state := query[t] + state.seen = true + if state.visiting { + query[t] = state + return false + } + state.visiting = true + query[t] = state + ret := p.needsTypeConversion(t.Underlying(), query) + state = query[t] + state.visiting = false + query[t] = state + if ret { + p.cvtneed[t] = conversionNeeded + } + return ret + case *types.Signature: + return true + case *types.Array: + return p.needsTypeConversion(t.Elem(), query) + case *types.Chan: + return p.needsTypeConversion(t.Elem(), query) + case *types.Tuple: + for i := 0; i < t.Len(); i++ { + if p.needsTypeConversion(t.At(i).Type(), query) { + return true + } + } + return false + case *types.TypeParam: + return false + case *types.Alias: + return p.needsTypeConversion(types.Unalias(t), query) + case *types.Union: + // cvtUnion currently always creates a raw union. + return true + default: + panic(fmt.Sprintf("needsTypeConversion: unexpected type - %T", typ)) + } } func Instantiate(orig types.Type, t *types.Named) (types.Type, bool) { diff --git a/ssa/type_cvt_test.go b/ssa/type_cvt_test.go new file mode 100644 index 0000000000..f599b024e2 --- /dev/null +++ b/ssa/type_cvt_test.go @@ -0,0 +1,195 @@ +/* + * Copyright (c) 2026 The XGo Authors (xgo.dev). All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package ssa + +import ( + "go/ast" + "go/parser" + "go/token" + "go/types" + "testing" +) + +func TestNamedTypeConversionIsIndependentOfTraversalOrder(t *testing.T) { + pkg := types.NewPackage("example.com/cycle", "cycle") + a := types.NewNamed(types.NewTypeName(token.NoPos, pkg, "A", nil), types.Typ[types.Int], nil) + b := types.NewNamed(types.NewTypeName(token.NoPos, pkg, "B", nil), types.Typ[types.Int], nil) + a.SetUnderlying(types.NewStruct([]*types.Var{ + types.NewField(token.NoPos, pkg, "B", types.NewPointer(b), false), + }, nil)) + b.SetUnderlying(types.NewStruct([]*types.Var{ + types.NewField(token.NoPos, pkg, "A", types.NewPointer(a), false), + types.NewField(token.NoPos, pkg, "F", types.NewSignatureType(nil, nil, nil, nil, nil, false), false), + }, nil)) + + convert := func(first, second *types.Named) (*types.Named, *types.Named) { + cvt := newGoTypes() + cvt.cvtNamed(first) + rawSecond, _ := cvt.cvtNamed(second) + rawFirst, _ := cvt.cvtNamed(first) + if first == a { + return rawFirst, rawSecond + } + return rawSecond, rawFirst + } + aFromA, bFromA := convert(a, b) + aFromB, bFromB := convert(b, a) + + for name, got := range map[string]*types.Named{ + "A after A-first conversion": aFromA, + "B after A-first conversion": bFromA, + "A after B-first conversion": aFromB, + "B after B-first conversion": bFromB, + } { + original := a + if name[0] == 'B' { + original = b + } + if got == original { + t.Errorf("%s retained the unconverted recursive type", name) + } + } + if got, want := types.TypeString(aFromA, nil), types.TypeString(aFromB, nil); got != want { + t.Errorf("A conversion depends on traversal order:\nA-first: %s\nB-first: %s", got, want) + } + if got, want := types.TypeString(bFromA, nil), types.TypeString(bFromB, nil); got != want { + t.Errorf("B conversion depends on traversal order:\nA-first: %s\nB-first: %s", got, want) + } + assertCycle := func(name string, rawA, rawB *types.Named) { + t.Helper() + aStruct := rawA.Underlying().(*types.Struct) + if got := aStruct.Field(0).Type().(*types.Pointer).Elem(); got != rawB { + t.Errorf("%s: converted A points to %v, want converted B", name, got) + } + bStruct := rawB.Underlying().(*types.Struct) + if got := bStruct.Field(0).Type().(*types.Pointer).Elem(); got != rawA { + t.Errorf("%s: converted B points to %v, want converted A", name, got) + } + if got, ok := bStruct.Field(1).Type().(*types.Struct); !ok || !IsClosure(got) { + t.Errorf("%s: converted B.F type = %v, want closure", name, got) + } + } + assertCycle("A-first", aFromA, bFromA) + assertCycle("B-first", aFromB, bFromB) +} + +func TestRecursiveGenericNamedTypeConversion(t *testing.T) { + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, "generic.go", `package generic +type My[T any] struct { + F func(T) + Next *My[T] +} +`, 0) + if err != nil { + t.Fatal(err) + } + pkg, err := (&types.Config{}).Check("example.com/generic", fset, []*ast.File{file}, nil) + if err != nil { + t.Fatal(err) + } + origin := pkg.Scope().Lookup("My").Type() + instantiated, err := types.Instantiate(nil, origin, []types.Type{types.Typ[types.Int]}, false) + if err != nil { + t.Fatal(err) + } + original := instantiated.(*types.Named) + + cvt := newGoTypes() + raw, changed := cvt.cvtNamed(original) + if !changed || raw == original { + t.Fatal("recursive generic type was not converted") + } + underlying, ok := raw.Underlying().(*types.Struct) + if !ok { + t.Fatalf("converted generic underlying = %T, want *types.Struct", raw.Underlying()) + } + if closure, ok := underlying.Field(0).Type().(*types.Struct); !ok || !IsClosure(closure) { + t.Fatalf("converted generic F = %v, want closure", underlying.Field(0).Type()) + } + next := underlying.Field(1).Type().(*types.Pointer).Elem() + if next != raw { + t.Fatalf("converted generic Next points to %v, want converted instance %v", next, raw) + } +} + +func TestTypeConversionRequirementShapes(t *testing.T) { + sig := types.NewSignatureType(nil, nil, nil, nil, nil, false) + sigParam := types.NewSignatureType(nil, nil, nil, + types.NewTuple(types.NewVar(token.NoPos, nil, "f", sig)), nil, false) + method := types.NewFunc(token.NoPos, nil, "M", sigParam) + methodInterface := types.NewInterfaceType([]*types.Func{method}, nil) + methodInterface.Complete() + embeddedInterface := types.NewInterfaceType(nil, []types.Type{methodInterface}) + embeddedInterface.Complete() + typeParam := types.NewTypeParam( + types.NewTypeName(token.NoPos, nil, "T", nil), types.Universe.Lookup("any").Type()) + alias := types.NewAlias(types.NewTypeName(token.NoPos, nil, "Alias", nil), sig) + union := types.NewUnion([]*types.Term{types.NewTerm(false, types.Typ[types.Int])}) + + tests := []struct { + name string + typ types.Type + want bool + }{ + {name: "basic", typ: types.Typ[types.Int]}, + {name: "pointer", typ: types.NewPointer(sig), want: true}, + {name: "interface method", typ: methodInterface, want: true}, + {name: "embedded interface", typ: embeddedInterface, want: true}, + {name: "slice", typ: types.NewSlice(sig), want: true}, + {name: "map key", typ: types.NewMap(sig, types.Typ[types.Int]), want: true}, + {name: "map value", typ: types.NewMap(types.Typ[types.Int], sig), want: true}, + {name: "closure", typ: newGoTypes().cvtClosure(sig)}, + {name: "struct", typ: types.NewStruct([]*types.Var{types.NewField(token.NoPos, nil, "F", sig, false)}, nil), want: true}, + {name: "signature", typ: sig, want: true}, + {name: "array", typ: types.NewArray(sig, 1), want: true}, + {name: "channel", typ: types.NewChan(types.SendRecv, sig), want: true}, + {name: "tuple", typ: types.NewTuple(types.NewVar(token.NoPos, nil, "F", sig)), want: true}, + {name: "type parameter", typ: typeParam}, + {name: "alias", typ: alias, want: true}, + {name: "union", typ: union, want: true}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + cvt := newGoTypes() + query := make(conversionNeedQuery) + if got := cvt.needsTypeConversion(test.typ, query); got != test.want { + t.Fatalf("needsTypeConversion(%v) = %v, want %v", test.typ, got, test.want) + } + }) + } +} + +func TestRecursiveNamedTypesWithoutConversionKeepTheirIdentity(t *testing.T) { + pkg := types.NewPackage("example.com/plaincycle", "plaincycle") + a := types.NewNamed(types.NewTypeName(token.NoPos, pkg, "A", nil), types.Typ[types.Int], nil) + b := types.NewNamed(types.NewTypeName(token.NoPos, pkg, "B", nil), types.Typ[types.Int], nil) + a.SetUnderlying(types.NewStruct([]*types.Var{ + types.NewField(token.NoPos, pkg, "B", types.NewPointer(b), false), + }, nil)) + b.SetUnderlying(types.NewStruct([]*types.Var{ + types.NewField(token.NoPos, pkg, "A", types.NewPointer(a), false), + }, nil)) + + cvt := newGoTypes() + if got, changed := cvt.cvtNamed(a); changed || got != a { + t.Fatalf("plain recursive A conversion = (%v, %v), want original type", got, changed) + } + if got, changed := cvt.cvtNamed(b); changed || got != b { + t.Fatalf("plain recursive B conversion = (%v, %v), want original type", got, changed) + } +} From 06b5378478de95164e190b43102a96a5bd8887ac Mon Sep 17 00:00:00 2001 From: ZhouGuangyuan Date: Thu, 6 Aug 2026 00:08:56 +0800 Subject: [PATCH 2/2] test(ssa): keep conversion predicate in sync --- ssa/type_cvt_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ssa/type_cvt_test.go b/ssa/type_cvt_test.go index f599b024e2..bd10a5a13d 100644 --- a/ssa/type_cvt_test.go +++ b/ssa/type_cvt_test.go @@ -170,6 +170,9 @@ func TestTypeConversionRequirementShapes(t *testing.T) { if got := cvt.needsTypeConversion(test.typ, query); got != test.want { t.Fatalf("needsTypeConversion(%v) = %v, want %v", test.typ, got, test.want) } + if _, got := newGoTypes().cvtType(test.typ); got != test.want { + t.Fatalf("cvtType(%v) changed = %v, want %v", test.typ, got, test.want) + } }) } }