Skip to content

llgo: fix tflag for named #86

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
visualfc wants to merge 1 commit into
base: main
Choose a base branch
from
Open

llgo: fix tflag for named #86

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 5 additions & 11 deletions rtype_llgo.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func setTypeName(t *rtype, pkgpath string, name string) {
_, f := path.Split(pkgpath)
name = f + "." + name
}
t.TFlag |= tflagNamed
t.TFlag = (t.TFlag & ^tflagExtraStar) | tflagNamed
t.Str_ = name
if t.TFlag&tflagUncommon == tflagUncommon {
t.Uncommon().PkgPath_ = pkgpath
Expand Down Expand Up @@ -374,19 +374,16 @@ func createMethod(typ reflect.Type, ptyp reflect.Type, m Method, index int) (mty
return
}

//go:linkname DirectIfaceData github.com/goplus/llgo/runtime/internal/runtime.DirectIfaceData

@fennoai fennoai Bot Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DirectIfaceData is declared here via //go:linkname but has no caller anywhere in the repository (git grep DirectIfaceData returns only this declaration). As it stands this is dead code. If it is scaffolding for a follow-up change, consider adding it in the same commit that uses it, or add a brief comment explaining its purpose; otherwise it is liable to confuse future readers since it looks like a deliberately exposed symbol with no consumer.

func DirectIfaceData(typ *abi.Type) bool

func (ctx *Context) hasImethod(typ reflect.Type, method Method) bool {
if ctx.fnHasImethod != nil {
return ctx.fnHasImethod(typ, method)
}
return true
}

//go:linkname gcEnable C.GC_enable
func gcEnable()

//go:linkname gcDisable C.GC_disable
func gcDisable()

func (ctx *Context) setMethodSet(typ reflect.Type, methods []Method, sortMethods bool) error {
if sortMethods {
sort.Slice(methods, func(i, j int) bool {
Expand Down Expand Up @@ -425,9 +422,6 @@ func (ctx *Context) setMethodSet(typ reflect.Type, methods []Method, sortMethods
ms := rtypeMethods(rt)
pms := rtypeMethods(prt)

gcDisable()
defer gcEnable()

var index int
for i, m := range methods {
if m.FuncId > 0 {
Expand Down Expand Up @@ -572,7 +566,7 @@ func SetUnderlying(typ reflect.Type, styp reflect.Type) {
st.Out = ost.Out
}
rt.Size_ = ort.Size_
rt.TFlag |= tflagUncommon | tflagExtraStar | tflagNamed
rt.TFlag |= tflagUncommon /*| tflagExtraStar*/ | tflagNamed

@gemini-code-assist gemini-code-assist Bot Jun 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Avoid leaving commented-out code in the source. It is cleaner to completely remove tflagExtraStar if it is no longer needed here.

Suggested change
rt.TFlag |= tflagUncommon /*| tflagExtraStar*/ | tflagNamed
rt.TFlag |= tflagUncommon | tflagNamed

@fennoai fennoai Bot Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two concerns on this line:

  1. Incomplete vs. setTypeName. setTypeName now actively clears the bit (t.TFlag = (t.TFlag & ^tflagExtraStar) | tflagNamed), but here you only stop setting it. If rt.TFlag already has tflagExtraStar set — e.g. it was OR-ed in from ort.TFlag in newType at rtype_llgo.go:281 — the star bit survives SetUnderlying, producing a wrong String()/Name() for the type. To mirror setTypeName and be robust, clear it explicitly:
rt.TFlag = (rt.TFlag | tflagUncommon | tflagNamed) & ^tflagExtraStar
  1. Commented-out code. /*| tflagExtraStar*/ leaves intent ambiguous (temporary? permanent?). Prefer removing it and adding a one-line comment explaining why the llgo build omits tflagExtraStar here. (Also raised by gemini-code-assist in this thread.)

rt.Kind_ = ort.Kind_
rt.Align_ = ort.Align_
rt.FieldAlign_ = ort.FieldAlign_
Expand Down
Loading