llgo: SetUnderlying remove named func to closure cached

[visualfc]

No description provided.

[gemini-code-assist]

Code Review

This pull request modifies rtype_llgo.go to clear cached named functions from namedFuncMap when setting an underlying type. The review feedback points out performance inefficiencies in the cache-clearing loop, specifically the repeated evaluation of typ.String() and the use of fmt.Sprint for string conversion. It suggests hoisting the string evaluation outside the loop and using a type assertion to improve performance and reduce heap allocations.

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.

[gemini-code-assist]

Performance & Efficiency Issues

1. Repeated typ.String() evaluation: typ.String() is called inside the namedFuncMap.Range callback. This means it is evaluated repeatedly for every single entry in the map. Since typ does not change during the iteration, its string representation should be computed once outside the loop.
2. Inefficient String Conversion (fmt.Sprint): Using fmt.Sprint(v) to get the string representation of v (which is a pointer to abi.FuncType) is slow and causes unnecessary heap allocations because it uses reflection under the hood. Since v implements fmt.Stringer (via the promoted String() method from abi.Type), we can use a direct type assertion to interface{ String() string } and call String() directly, which is much faster and avoids allocations.

Suggested change

	// remove named func to closure cached
	namedFuncMap.Range(func(k, v any) bool {
	if typ.String() == fmt.Sprint(v) {
	namedFuncMap.Delete(k)
	return false
	}
	return true
	})
	typStr := typ.String()
	namedFuncMap.Range(func(k, v any) bool {
	if stringer, ok := v.(interface{ String() string }); ok && typStr == stringer.String() {
	namedFuncMap.Delete(k)
	return false
	}
	return true
	})

[fennoai]

Review summary

This PR adds closure-cache eviction to the reflect.Func branch of SetUnderlying, links the llgo runtime's reflect.namedFuncMap via //go:linkname, and removes a stale commented-out tflagExtraStar flag. The core mechanism (matching a named func type against cached entries by their string form, since abi.FuncType embeds abi.Type and thus promotes String()) is coherent. A few refinements below; the string-identity approach also carries some correctness/concurrency caveats worth confirming against the runtime.

Notes not tied to a single diff line

• String-based type identity is fragile. Eviction relies on typ.String() == fmt.Sprint(v). Two distinct func types can share the same string form, and Type.String() prepends * when TFlagExtraStar is set (internal/abi/type_llgo.go:416-420), so string equality is not a reliable proxy for type identity. If the runtime exposes a way to match by key/pointer identity instead, that would be more robust. Please confirm the value stored in reflect.namedFuncMap stringifies to exactly what typ.String() returns for the intended matches.
• Concurrent mutation of a runtime-shared map. Range/Delete on sync.Map are individually safe, but the surrounding in-place rewrite of rt is not atomic with respect to the runtime. If any goroutine consults these types concurrently with SetUnderlying, it may observe a half-updated state. If SetUnderlying is only expected to run during single-threaded type setup, a short comment stating that invariant would help.

[fennoai]

typ.String() is invoked inside the Range callback, so it is recomputed and re-allocated for every entry scanned. Hoist it to a local before the Range:

name := typ.String()
namedFuncMap.Range(func(k, v any) bool {
    if name == fmt.Sprint(v) {
        namedFuncMap.Delete(k)
        return false
    }
    return true
})

Separately, fmt.Sprint(v) runs reflection-based formatting and allocates once per entry on every Func SetUnderlying. Since abi.FuncType embeds abi.Type (promoting String()), calling v.(interface{ String() string }).String() — or comparing typed identity if available — avoids the fmt reflection path.

[fennoai]

The return false stops iteration after the first match, assuming at most one cached entry maps to a given named func type. If more than one encoding can exist for the same type, remaining stale entries would be leaked. If the single-match invariant holds, a brief comment documenting it would help; otherwise return true to continue scanning.

[fennoai]

Comment reads awkwardly ("remove named func to closure cached"). Consider rewording for clarity, e.g. // evict the cached closure entry for this named func type so the redefined type is not stale.