Skip to content

fix(gnotypes): prevent init panic on Go 1.26+ from recursive builtin type stubs#57

Open
tbruyelle wants to merge 3 commits into
mainfrom
fix/gnotypes-init-panic-go126
Open

fix(gnotypes): prevent init panic on Go 1.26+ from recursive builtin type stubs#57
tbruyelle wants to merge 3 commits into
mainfrom
fix/gnotypes-init-panic-go126

Conversation

@tbruyelle

Copy link
Copy Markdown
Collaborator

Go 1.26's go/types now treats the self-referential predeclared type stubs in builtin.gno (type bool bool, type comparable interface{ comparable }, ...) as a fatal "invalid recursive type" error that aborts type resolution for the whole file. This leaves ~23 builtin funcs -- including gno's cross, revive and istypednil -- with a nil Type(), and isMethod's unchecked fn.Type().(*types.Signature) then panics at package init. The panic kills the gnopls binary on startup, so the language server never answers a single request (definition, hover, ...) on any file.

On Go 1.24/1.25 the same recursive-type error was reported but non-fatal, so the funcs still got their signatures -- hence the regression only surfaced after building with the 1.26 toolchain.

  • builtin.gno: drop the self-referential predeclared scalar type stubs and the comparable stub. They are pure documentation: each is already provided by types.Universe and skipped during registration, so removing them changes nothing but lets the file type-check cleanly on all toolchains.
  • builtin.go: harden isMethod and the builtin registration loop to log-and-skip a nil signature instead of panicking, so a future toolchain change degrades gracefully instead of taking down the whole server.

Verified on Go 1.26.4 and 1.25.0: gnotypes/resolver tests pass, builtins register, gnopls definition on the realm builtin type and cross-package imports resolve, and gnopls check on an interrealm-v2 realm is clean.

…type stubs

Go 1.26's go/types now treats the self-referential predeclared type stubs in
builtin.gno (`type bool bool`, `type comparable interface{ comparable }`, ...)
as a fatal "invalid recursive type" error that aborts type resolution for the
whole file. This leaves ~23 builtin funcs -- including gno's cross, revive and
istypednil -- with a nil Type(), and isMethod's unchecked
fn.Type().(*types.Signature) then panics at package init. The panic kills the
gnopls binary on startup, so the language server never answers a single request
(definition, hover, ...) on any file.

On Go 1.24/1.25 the same recursive-type error was reported but non-fatal, so the
funcs still got their signatures -- hence the regression only surfaced after
building with the 1.26 toolchain.

- builtin.gno: drop the self-referential predeclared scalar type stubs and the
  comparable stub. They are pure documentation: each is already provided by
  types.Universe and skipped during registration, so removing them changes
  nothing but lets the file type-check cleanly on all toolchains.
- builtin.go: harden isMethod and the builtin registration loop to log-and-skip
  a nil signature instead of panicking, so a future toolchain change degrades
  gracefully instead of taking down the whole server.

Verified on Go 1.26.4 and 1.25.0: gnotypes/resolver tests pass, builtins
register, `gnopls definition` on the realm builtin type and cross-package
imports resolve, and `gnopls check` on an interrealm-v2 realm is clean.
Add a `toolchain go1.26.4` directive so CI and `go install` build with the
Go 1.26 toolchain that exposes the gnotypes init panic fixed in the previous
commit. Without this, a machine with an older default toolchain could build a
working binary while 1.26 users get a crash-on-startup server, and CI would
stay green against the very version that breaks.

The `go 1.25.0` minimum language version is left unchanged.
@gfanton

gfanton commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Tested on Go 1.26.4 and 1.25.10 — LGTM. Removing the recursive stubs fixes it at the source (repro'd: old file → 23 nil-typed funcs, this PR → 0), and the nil guards are solid. gnotypes + resolver tests pass on both.

Two minor notes:

  • builtin.go:118 (IsGnoBuiltin) still does the unguarded obj.Type().(*types.Signature) — same pattern you hardened in isMethod. Not reachable in practice, just a consistency call.
  • On the description: I couldn't reproduce the "only on 1.26" part — the recursive error is non-fatal on both 1.25.10 and 1.26.4 in a normal build; the nil cascade only shows when the importer can't resolve (likely the shipped binary without module cache), which looks version-independent. Fix is correct regardless.

(ai agent)

Apply the same defensive handling as isMethod to IsGnoBuiltin's
obj.Type().(*types.Signature) assertion: an unresolved func signature now
returns false instead of panicking. Addresses PR review feedback (the last
unguarded instance of this pattern).
@tbruyelle

Copy link
Copy Markdown
Collaborator Author

Thanks for the review!

builtin.go:118 (IsGnoBuiltin) still does the unguarded obj.Type().(*types.Signature)

Good catch — fixed in a follow-up commit (2cab3c5, pushing shortly), same guard as isMethod.

I couldn't reproduce the "only on 1.26" part … which looks version-independent.

I re-ran the actual go test ./pkg/gnotypes/ on the pre-fix source, cache-busted (-count=1), same machine and module cache, varying only the toolchain:

toolchain result
go1.25.0 ok
go1.25.10 ok
go1.26.4 panic

So for the real code path it is version-dependent. The subtlety: newBuiltinDef calls conf.Check with no conf.Error handler, so go/types runs in abort-on-first-error (bailout) mode — and that bailout path is what changed in 1.26. It now bails on the recursive-bool error before the builtin func signatures are resolved, leaving ~23 of them nil. If you set an error collector (as a standalone repro naturally does), both toolchains resolve fully → 0 nil funcs → "non-fatal on both", which is exactly what you observed.

Your importer point is a real, related failure mode though: an importer that can't resolve import "cmp" (e.g. a shipped binary on a Go-less host) would hit the same abort-on-first-error nil cascade, version-independently. The nil guards turn that into graceful log-and-skip instead of a panic. If we want the funcs to fully resolve even in that case, I can add a conf.Error collector in newBuiltinDef on top — happy to do that in this PR if you think it's worth it.

@notJoon

notJoon commented Jul 7, 2026

Copy link
Copy Markdown
Member

this change finally makes me free from the eternal pain point

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants