support llgo NamedTypeOf#83
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for the llgo compiler by updating build tags across several files in the internal/abi package and introducing a new type_llgo.go file. The review feedback suggests optimizing the Methods and ExportedMethods implementations in the new file by using unsafe.Slice to avoid manual slice allocations and element copying.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![fennoai[bot]](https://avatars.githubusercontent.com/in/1924301?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review: support llgo
This PR refactors the internal ABI type system to support the llgo compiler by introducing internal/abi/type_llgo.go with llgo-specific type definitions, guarded by //go:build llgo, and adjusting other files to use type aliases from the internal/abi package.
The structural approach (a new build-tag-guarded file with llgo-specific ABI definitions) is sound. However, the llgo build appears to be incomplete: several type definitions required by unguarded reflectx package files are absent from type_llgo.go, and there are field name/type mismatches between the two builds that would prevent compilation under llgo.
Missing type definitions in type_llgo.go
reflectx/type.go (no build tag) aliases:
type name = abi.Name
type nameOff = abi.NameOff
type typeOff = abi.TypeOff
type textOff = abi.TextOffNone of abi.Name, abi.NameOff, abi.TypeOff, or abi.TextOff are defined in type_llgo.go. Every file that uses these aliases (linkname.go, name.go, rtype.go, reflectx.go, methodof.go, method.go) will fail to compile under llgo.
Field name / type mismatches between builds
Type.Str_(llgo,string) vsType.Str(non-llgo,NameOff): code inrtype.go,reflectx.go, andmethod.gowritesrt.Str = resolveReflectName(...)— no such field in llgo.Type.PtrToThis_(llgo,*Type) vsType.PtrToThis(non-llgo,TypeOff):methodof.goassignsrt.PtrToThis = resolveReflectType(prt)(aTypeOff) — type mismatch in llgo.FuncType(llgo) hasIn []*Type,Out []*Type; (non-llgo) hasInCount uint16,OutCount uint16.rtype.goaccessesst.InCountandst.OutCountdirectly.UncommonType.PkgPath_(llgo,string) vsPkgPath(non-llgo,NameOff):methodof.goandreflectx.goassign aNameOff(int32) to this field.Method(llgo):Name_ string,Mtyp_ *FuncType,Ifn_ Text,Tfn_ Text.methodof.gosetspms[i].Name,.Mtyp,.Tfn,.Ifn(no underscore) — none of these fields exist in llgo'sMethod.
None of the affected files (rtype.go, reflectx.go, methodof.go, method.go, linkname.go) carry a //go:build !llgo guard.
TFlag bit 7 collision
type_llgo.go defines TFlagGCMaskOnDemand TFlag = 1 << 7. reflectx/type.go defines tflagUserMethod tflag = 1 << 7. Under an llgo build both flags occupy the same bit: any type the llgo runtime marks with TFlagGCMaskOnDemand will be misidentified by reflectx as having user-injected methods, and any type reflectx marks via setMethodSet will cause the runtime to attempt on-demand GC mask computation on a value that is not **byte.
GcSlice missing nil/flag guards
The non-llgo type.go implementation of GcSlice guards against TFlagGCMaskOnDemand being set (where GCData is **byte, not *byte) and against nil GCData. The llgo version unconditionally calls unsafe.Slice(t.GCData, int(end)), which panics on nil GCData and reinterprets a **byte as a *byte when the on-demand flag is set.
| // If the KindGCProg bit is set in kind, GCData is a GC program. | ||
| // Otherwise it is a ptrmask bitmap. See mbitmap.go for details. | ||
| GCData *byte | ||
| Str_ string // string form |
There was a problem hiding this comment.
Str_ is a string here, but in the non-llgo build Type.Str is NameOff (int32). Files without a !llgo build guard — rtype.go, reflectx.go, method.go — write rt.Str = resolveReflectName(...) which returns a NameOff. This will not compile under llgo. Similarly PtrToThis_ here is *Type, while code assigns rt.PtrToThis = resolveReflectType(prt) (a TypeOff). These fields either need to be renamed to match the access sites, or those files need //go:build !llgo guards.
| Float64: "float64", | ||
| Complex64: "complex64", | ||
| Complex128: "complex128", | ||
| Array: "array", |
There was a problem hiding this comment.
Method fields here are Name_ string, Mtyp_ *FuncType, Ifn_ Text, Tfn_ Text, but methodof.go (no build tag) accesses them as .Name, .Mtyp, .Ifn, .Tfn. This will not compile under llgo. Either rename the fields to match the access sites, or add a //go:build !llgo constraint to methodof.go.
| // compile time. If this flag is set, the GCData field effectively | ||
| // has type **byte instead of *byte. The runtime will store a | ||
| // pointer to the GC pointer bitmask in *GCData. | ||
| TFlagGCMaskOnDemand TFlag = 1 << 7 |
There was a problem hiding this comment.
TFlagGCMaskOnDemand = 1 << 7 collides with tflagUserMethod tflag = 1 << 7 defined in reflectx/type.go. Under an llgo build, any type the runtime marks with TFlagGCMaskOnDemand will be incorrectly treated by reflectx as having user-injected methods, and any type reflectx marks via setMethodSet will corrupt the GC on-demand metadata bit. tflagUserMethod should use a bit not occupied by any TFlag constant in either build.
| return lastDot(p.Name_) == -1 | ||
| } | ||
|
|
||
| // Name returns the tag string for method. |
There was a problem hiding this comment.
Comment says "Name returns the tag string for method" — should be "Name returns the name string for method". The same copy-paste mistake appears on the Imethod.Name() comment below.
| } | ||
|
|
||
| // IsClosure reports whether t is closure struct | ||
| func (t *Type) IsClosure() bool { |
There was a problem hiding this comment.
Methods() allocates a new []Method and copies every element via a manual pointer-arithmetic loop — O(n) heap allocation per call. The non-llgo equivalent returns a zero-copy slice header over the existing memory with no allocation. This function is called on hot paths (method lookup, interface satisfaction). Consider the same zero-copy approach:
return (*[1 << 16]Method)(addChecked(unsafe.Pointer(t), uintptr(t.Moff), "t.mcount > 0"))[:t.Mcount:t.Mcount]| } | ||
| return &(*u)(unsafe.Pointer(t)).u | ||
| case Interface: | ||
| type u struct { |
There was a problem hiding this comment.
The TODO(xsw): misunderstand comment signals unresolved uncertainty about the TFlagExtraStar semantics in the llgo context. This should be clarified before merging — either confirm the implementation is correct and remove the TODO, or document the known divergence from standard semantics.
| } | ||
|
|
||
| func (t *Type) GcSlice(begin, end uintptr) []byte { | ||
| return unsafe.Slice(t.GCData, int(end))[begin:] |
There was a problem hiding this comment.
unsafe.Slice(t.GCData, int(end)) panics if t.GCData is nil (valid for types with no pointer data). The non-llgo implementation guards against this case. Add a nil check. Additionally guard against the TFlagGCMaskOnDemand case where GCData is actually **byte rather than *byte, otherwise the unsafe reinterpretation produces a wrong memory view.
1 participant
No description provided.