Skip to content

ssa: panic with runtime type assertion errors#1892

Draft
cpunion wants to merge 5 commits into
xgo-dev:mainfrom
cpunion:codex/goroot-interface-coverage
Draft

ssa: panic with runtime type assertion errors#1892
cpunion wants to merge 5 commits into
xgo-dev:mainfrom
cpunion:codex/goroot-interface-coverage

Conversation

@cpunion
Copy link
Copy Markdown
Collaborator

@cpunion cpunion commented May 22, 2026

Summary

  • lower failed non-comma-ok type assertions through a runtime TypeAssertionError helper instead of panic(string)
  • add stable test/go coverage for recovered runtime.Error values on interface-to-interface and interface-to-concrete assertions
  • remove fixed fixedbugs/issue16130.go GOROOT xfails; leave issue32187 comparison xfails untouched

Testing

  • go test ./test/go -run 'TestInterfaceAssert|TestNilInterfaceSameTypeAssertPanics|TestInterfaceCompareUncomparable' -count=1\n- go test ./test/go -count=1\n- (cd runtime && go test ./internal/runtime -run 'TestDoesNotExist' -count=1)\n- LLGO_ROOT="$PWD" go run -tags=dev ./cmd/llgo test -run 'TestInterfaceAssert|TestNilInterfaceSameTypeAssertPanics|TestInterfaceCompareUncomparable' ./test/go\n- go test ./ssa -count=1\n- go test ./test/goroot -run TestGoRootRunCases/fixedbugs/issue16130.go -count=1 -args -goroot "$(go env GOROOT)" -dirs fixedbugs -case '^fixedbugs/issue16130\.go$' -xfail /tmp/llgo-no-xfail.yaml\n- go test ./test/goroot -run TestGoRootRunCases/fixedbugs/issue16130.go -count=1 -args -goroot "$(go env GOROOT)" -dirs fixedbugs -case '^fixedbugs/issue16130\.go$'\n\n## Notes\nGOROOT CI is slow/disabled, so this PR does not depend on automatic full GOROOT runs. The targeted local GOROOT check used the current machine GOROOT (go env GOROOT = /opt/homebrew/Cellar/go@1.24/1.24.11/libexec). Normal PR CI should still run from the fork branch.\n\nA non-targeted local LLGO_ROOT="$PWD" go run -tags=dev ./cmd/llgo test ./test/go run reached test execution but failed on darwin temp-dir cleanup with operation not permitted in existing temp-dir cleanup paths; the focused interface assertion llgo test above passed.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 22, 2026

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 80.70175% with 11 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
ssa/package.go 45.45% 4 Missing and 2 partials ⚠️
cl/import.go 81.25% 2 Missing and 1 partial ⚠️
ssa/abitype.go 90.90% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@cpunion
Copy link
Copy Markdown
Collaborator Author

cpunion commented May 24, 2026

Update for latest interface/nointerface coverage (head c9e354c):

Scope:

  • interface/nointerface only; no chan/nil/print/recover/GC/liveness/finalizer/goroutine changes.
  • //go:nointerface methods are recorded during AST directive scan and omitted from runtime ABI method tables, while direct method calls/method expressions remain available.

GOROOT xfails removed after targeted local verification:

  • typeparam/mdempsky/15.go (//go:nointerface, generics, promoted methods)
  • fixedbugs/issue47928.go (promoted //go:nointerface method)
  • typeparam/mdempsky/16.go (interface type assertion panic text; already fixed by this PR's earlier assertion-error work)

Coverage:

  • Added test/go/nointerface_test.go plus llgo/non-llgo expectation files. Host Go's default toolchain does not filter nointerface without the fieldtrack experiment, while llgo should filter under the llgo tag.

Local tests:

  • go test ./test/go -count=1
  • go test ./ssa -count=1
  • go test ./cl -count=1
  • go test ./test/goroot -run TestGoRootRunCases -count=1 -args -goroot "$(go env GOROOT)" -dirs typeparam/mdempsky,fixedbugs -case '^(typeparam/mdempsky/(15|16)|fixedbugs/issue47928)\.go$' -directive-mode runlike -run-timeout 120s

Not covered here:

  • Remaining non-nointerface xfails such as typeparam/nested.go and unrelated chan/nil/print/recover/GC cases are intentionally left for their owning domains.

@cpunion
Copy link
Copy Markdown
Collaborator Author

cpunion commented May 24, 2026

Update for commit 64ae33c:\n\n- Added reflect/interface type identity coverage for reflect.Type.Method and MethodByName: Method.Func.Interface() now asserts to and calls func(*T) / func(T) using the canonical LLGo closure-backed function type.\n- Fixed reflect.Type interface invokes to request method ABI type initialization, included closure ABI symbols for reflect method FuncOf lookup, and boxed Type.Method Func values as LLGo closure values while preserving public reflect Type identity.\n- recover.go classification: without #1882 it still stops at the existing recover-frame failure (spurious recover). With #1882's recover-frame fix layered locally, the previous func(*T1) vs func(*T1) type identity failure is gone; the next observed failure is missing recover 10, which is recover semantics and intentionally not covered here. No recover.go xfail removed.\n- Local tests: go test ./test/go -count=1; go run ./cmd/llgo test -run TestReflectTypeMethodFuncInterfaceTypeIdentity -count=1 ./test/go; go test ./ssa -count=1; go test ./cl -count=1; go test ./internal/build -count=1; focused goroot recover.go with existing xfail passed.

@cpunion
Copy link
Copy Markdown
Collaborator Author

cpunion commented May 24, 2026

Coverage update for fixedbugs/issue26094.go (commit 5733be6aa6aa6f2bb3782a00ae49e7bc8c960989):

  • Classified as the same local type identity / interface assertion root as this draft PR: different scopes can produce the same printed type name, but interface assertion must compare scoped type identity and panic with different scopes.
  • Added stable test/go coverage in TestInterfaceAssertRejectsSameNameTypesFromDifferentScopes for local-vs-local and local-vs-package same-name type assertions.
  • Removed only the now-covered fixedbugs/issue26094.go goroot xfail entries for darwin/arm64 and linux/amd64. No nil/chan/print/recover/GC/finalizer/goroutine cases were changed.

Focused local validation:

  • go test ./test/go -run 'TestInterfaceAssert(RejectsSameNameTypesFromDifferentScopes|To.*Panics)' -count=1\n- go test ./test/goroot -run TestGoRootRunCases/fixedbugs/issue26094.go -count=1 -args -goroot /Users/lijie/sdk/go1.26.0 -dirs fixedbugs -case '^fixedbugs/issue26094\\.go$' -progress 30s\n- go test ./test/go -count=1\n- go test ./ssa/... ./cl/... ./runtime/... -count=1 reported ssa and cl OK; the command itself returned FAIL because ./runtime/... is a separate module path from the repo root.\n- (cd runtime && go test ./... -count=1) passed.

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.

2 participants