internal/packages: eliminate //go:linkname and decouple from x/tools internals - #2369
internal/packages: eliminate //go:linkname and decouple from x/tools internals#2369cpunion wants to merge 3 commits into
Conversation
8c00388 to
dc4629e
Compare
LLGo baseline benchmarks
Program measurements
Core language and compiler benchmarks
Compared with |
There was a problem hiding this comment.
Review: refactor internal/packages to drop //go:linkname internals
This is a solid, well-motivated refactor. It removes the fragile //go:linkname hooks into golang.org/x/tools internals (defaultDriver, newLoader, loadFromExportData, parseFiles, typesinternalSetUsesCgo) and the private loader/loaderPackage types, replacing them with a self-contained driver: call the public packages.Load with an adjusted Mode, then parse + typecheck bottom-up via packages.Visit. This is much less likely to break across x/tools upgrades. internal/packages tests pass locally.
I verified the main worry — that dropping the export-data fallback would break NeedImports-without-NeedDeps loads — and it does not reproduce: the go list driver still returns dependency packages with populated CompiledGoFiles, so packages.Visit reaches and typechecks them from source (confirmed internal/bytealg loads with complete import types, IllTyped=false, 0 errors). So that path is fine.
A few findings below, ordered by impact. The wasm-sizes one (P1) is the only likely functional regression.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
11d8b59 to
adb0bec
Compare
Summary
Eliminate all
//go:linknamedirectives and private struct mirroring ininternal/packages/load.go, replacing them with standard publicgolang.org/x/tools/go/packages.Loadand a self-contained, post-ordergo/types.NewCheckertypechecker.This resolves fragility across
golang.org/x/toolsupgrades (e.g., #2368) and achieves a cleaner import pipeline with reduced binary sizes for compiled programs.Key Changes
Decouple from
x/toolsPrivate Internals://go:linknamehooks (defaultDriver,newLoader,loadFromExportData,parseFiles,typesinternalSetUsesCgo).loader,loaderPackage,loadRecursiveEx,refineEx).packages.Loadhandlesgo listdriver invocation, module metadata, embed discovery, and file resolution.Self-Contained Bottom-Up Typechecking:
packages.Visit.go/types.NewCheckerwith a customimporterFuncensuring complete type information across dependencies.cfg.Envtarget resolution).Dedupercaching (safely guarded onpkg.Types.Complete()) andpreloadtypes.SizesoverridesllgoFilespatch file injectionGoVersionpropagation (requestingNeedModule)*os.PathErrorpropagationBinary Size & Dead Code Elimination (DCE) Improvement:
types.Inforecords, allowing LLGo's backend SSA dead-code elimination and symbol pruning to be more effective.fmtprintf: -832 B on Linux, -272 B on macOS) with zero regression in performance.Code Quality & Maintenance:
LoadMode,Config,Visit,LoadEx).Verification
go test -v ./internal/packages(PASS)go test -v ./internal/build -run TestWasm(PASS)bash test/buildcache/test.sh(18/18 Native, WASM, ESP32-C3 cache tests PASS)