fix: align unexported package member diagnostics

[MeteorsLiu]

No description provided.

[fennoai]

Review summary

Rewrites the go/types diagnostic name X not exported by package … into the gc-style undefined: pkg.X so the go1.26 errorcheck case runtime.go passes, and drops the corresponding xfail.yaml entry. The mechanism is sound: keying by err.Pos (== sel.Sel.Pos()) is exact and collision-free, closure-capture ordering is safe (the map is populated before NewChecker(...).Files(...) runs), and the message-prefix guard makes an unintended rewrite effectively impossible since that message is package-exclusive in go/types.

One correctness concern worth addressing (inline), plus a minor efficiency note below.

Minor: eager, unconditional AST walk on the per-package load path

internal/packages/load.go (~line 373): the ast.Inspect walk and map[token.Pos]string are built for every package on every load, even when the package type-checks cleanly and no message ever needs rewriting. It also records all ident-qualified selectors (v.Field, x.Method, …), not just import-qualified ones, so the map is larger than the single use case requires. This is pure overhead for the common (no-error) case. Since it runs once per package before type-checking, the cost is likely modest relative to type-checking itself — but building the map lazily inside the types.Error branch (only when the target message is actually seen), or restricting insertion to import-qualified selectors, would eliminate it. Non-blocking.

View job run

[fennoai]

Review: link-time DCE for ABI method slots + per-package frontend options

Thanks for this — the DCE analysis is well-structured, the cl.Options refactor that replaces the process-wide Enable* globals is clean and consistently threaded, the sync.Once around llssa.Initialize is safe (init flags are a compile-time constant), and the RewriteMainPrefix removal left no dangling references. Comprehensive tests accompany the new packages.

Findings below, ordered by severity. The correctness-class items (a live method being pruned, or a panic on valid input) matter most because they surface only under -deadcodedrop.

Correctness / robustness

• internal/dcepass/dcepass.go:147 — panic on an unsupported global value is reachable. cloneGlobalValue handles only IsAFunction and IsAGlobalVariable; any other GlobalValue (GlobalAlias, GlobalIFunc) hits panic("dcepass: unsupported global value"). llgo does emit aliases (ssa/decl.go:148 llvm.AddAlias), so if a pruned ABI type descriptor ever references one — directly or through a cloned field — the whole build aborts. Prefer rebinding aliases by name to a dst declaration (as functions are handled) or keeping the value, rather than panicking. (inline)

• internal/dcepass/dcepass.go:114-127 — cloneConst does not recurse into ConstantExpr. It recurses only through ConstantStruct; a ConstantExpr (GEP / bitcast / inttoptr, all produced elsewhere in codegen) falls through to return v, leaving the dst-module initializer pointing back into the source module's value graph. Latent today under opaque pointers where the observed fields are plain global refs, but a typed-pointer build or a codegen change that materializes a field as a constant-expression would produce a cross-module reference. Consider recursing through ConstantExpr/ConstantArray operands, or explicitly asserting the field shapes you support. (inline)

• internal/dcepass/dcepass.go:68-76 — method-array element read as a 4-operand struct without validation. methodArray validates the element type has 4 fields, but the drop branch unconditionally reads orig.Operand(0..3) (and Operand(2/3).Name() when verbose) on each element value. A ConstantAggregateZero element has OperandsCount()==0, so Operand(i) would be out of range. Safe today because llgo always emits non-null ifn/tfn, but it is an unguarded assumption on IR shape. (inline)

• internal/deadcode/analyze.go:46-52 / internal/build/build.go (dceEntryRootCandidates) — missing entry roots are silently dropped. Analyze skips any root name absent from the summary. If a mandatory entry root such as main.main ever fails to resolve (e.g. a metadata-collection regression), the pass silently treats far more slots as dead and prunes live code with no diagnostic. A cheap guard/verbose-warning when main.main/main.init resolve to nothing would turn a silent miscompile into a visible error.

Performance

• internal/deadcode/analyze.go:279-284 — popWork is O(n) per dequeue → O(n²) flood. copy(d.workQueue, d.workQueue[1:]) shifts the entire remaining slice on every pop, so draining N enqueued symbols costs O(N²) over a whole-program analysis. The loop runs to a fixpoint, so FIFO vs LIFO is irrelevant to correctness — popping from the tail (d.workQueue[len-1]; d.workQueue = d.workQueue[:len-1]) is O(1), or use a head index. Single highest-value change in the analysis. (inline)

• internal/meta/global.go:264-272 — OrdinaryEdges grows via append with no pre-size on the hot flood path, unlike the structurally identical TypeChildren (line 282) which uses make([]Symbol, len(local)). Pre-size to len(edges) for consistency. Separately, markUsedInIface re-enqueues an already-reachable symbol, causing flood to re-walk its ordinary edges + demands purely to reach the usedInIface branch — a separate "already flooded" guard would avoid that duplicate work.

Minor / docs

• internal/dcepass/dcepass.go:20-21 — doc says "clears IFn/TFn" but the code replaces both with the unreachableMethod stub (the slots stay populated, pointing at a trap). "redirects IFn/TFn to an unreachable stub" describes it accurately. (inline)

• internal/dcepass/dcepass.go:53 — redundant map write. emitTypeOverride sets e.values[srcType] = dstType, but ensureOverrideGlobal (called the line above) already assigns the same key. The line-53 write is dead.

• cl/compile.go:2117-2119 — comment steers multi-package drivers to a now-deprecated function. The block comment says multi-package drivers "use NewPackageExWithEmbed", which this PR marks Deprecated: use NewPackageExWithEmbedMetaOptions. Point the guidance at the new entry point.

No blocking issues; the panic-robustness and the O(n²) dequeue are the two I'd most want addressed before this runs on large programs.

Findings without inline locations

• internal/dcepass/dcepass.go:147: cloneGlobalValue handles only functions and global variables; any other GlobalValue (GlobalAlias, GlobalIFunc) reaches this panic. llgo does emit aliases (ssa/decl.go:148), so if a pruned ABI type descriptor references one — directly or via a cloned field — the build aborts on valid input. Consider rebinding aliases by name to a dst declaration (like functions above) or falling back to keeping the value, rather than panicking.
• internal/dcepass/dcepass.go:120: cloneConst recurses only through ConstantStruct. A ConstantExpr (GEP/bitcast/inttoptr) falls through to return v here, leaving the dst-module initializer referencing the source module's value graph. Latent under opaque pointers (observed fields are plain global refs), but a typed-pointer build or a codegen change that materializes a field as a constant-expression would produce a cross-module reference. Consider recursing through ConstantExpr/ConstantArray operands, or asserting the shapes you support.
• internal/dcepass/dcepass.go:70: The drop branch unconditionally reads orig.Operand(0..3) (and Operand(2/3).Name() when verbose). methodArray validated the element type has 4 fields, but not that each element value is a materialized 4-operand ConstantStruct — a ConstantAggregateZero element has OperandsCount()==0 and would fault here. Safe today since ifn/tfn are always non-null, but it's an unguarded assumption on IR shape.
• internal/deadcode/analyze.go:283: copy(d.workQueue, d.workQueue[1:]) shifts the whole remaining slice on every pop, making flood() O(n²) over a whole-program analysis. The loop runs to a fixpoint so ordering is irrelevant — popping from the tail is O(1):
• internal/dcepass/dcepass.go:20: Nit: "clears IFn/TFn" — the code replaces both fields with the unreachableMethod stub (slots stay populated, pointing at a trap) rather than clearing them. "redirects IFn/TFn to an unreachable stub" would be accurate.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[fennoai]

Review (corrected): align unexported-package-member diagnostics

⚠️ My earlier review on this PR reviewed the wrong diff — it compared the head branch against the current main tip and so pulled in dozens of already-merged, unrelated commits (the DCE feature, the cl.Options refactor, etc.). Please disregard that review. This PR's real base is 2310ff87 and it changes only 3 files. This review supersedes it.

The actual change makes llgo report undefined: alias.hidden (matching gc) instead of name hidden not exported by package ... when source references an unexported member of another package, and removes the now-passing runtime.go go1.26 errorcheck case from xfail.yaml. It's focused and well-tested (load_test.go asserts both the new message and the absence of the old one).

Only minor observations — nothing blocking.

Observations

• internal/packages/load.go:291 — the rewrite is coupled to go/types' exact error wording. The trigger is strings.HasPrefix(err.Msg, "name "+name+" not exported by package "). If a future Go toolchain rephrases that diagnostic, the rewrite silently stops firing and the original (non-gc) message resurfaces. The failure mode is graceful (you fall back to the upstream message, and the goroot errorcheck test would flag the regression), so this is acceptable — but a brief comment noting the dependency on the go/types string would help the next reader understand why the match is so literal.

• internal/packages/load.go:383-385 — packageSelectors records every ident-qualified selector, not just package qualifiers (x.field, v.Method, and pkg.Name all land in the map). This is harmless because the rewrite at line 289-293 only activates on the name X not exported by package message, which go/types emits exclusively for package-qualified access to an unexported identifier — so a non-package qualifier can never reach the rewrite. Worth being aware of if that message guard is ever loosened; today it is safe by construction.

• internal/packages/load.go:260,289 — nil-map read is safe. packageSelectors is declared nil at line 260 and only populated at line 376, after parse/driver errors but before type-checking. The line-289 lookup runs only for types.Error (emitted during type-checking, after population), and a read from a nil map returns the zero value regardless — so there is no ordering hazard or panic risk. Noting it only to confirm it was checked.

Overall this looks correct and appropriately scoped. No changes required.

[fennoai]

This rewrite is coupled to go/types' exact diagnostic wording (name X not exported by package ). If a future Go toolchain rephrases that message, the rewrite silently stops firing and the original message resurfaces. The fallback is graceful and the goroot errorcheck test would catch the regression, so this is fine — a short comment noting the dependency on the go/types string would help the next reader understand why the match is so literal.

[fennoai]

This records every ident-qualified selector (x.field, v.Method, pkg.Name), not only package qualifiers. Harmless today because the rewrite only activates on the name X not exported by package message, which go/types emits exclusively for package-qualified unexported access — so a non-package qualifier can never reach it. Just flagging it in case that message guard is ever loosened.

[github-actions]

LLGo baseline benchmarks

5e23576c2fc7 | workflow run | long-term charts

Program measurements

Platform	Workload	File size	vs main	Build	vs main	Run	vs main
Linux	cprintf	18544 B	new	354.594 ms	new	1.358 ms	new
Linux	fmtprintf	2217608 B	new	3.234 s	new	2.711 ms	new
Linux	println	71504 B	new	348.636 ms	new	1.709 ms	new
macOS	cprintf	84672 B	new	353.821 ms	new	2.456 ms	new
macOS	fmtprintf	2361520 B	new	2.622 s	new	17.106 ms	new
macOS	println	125712 B	new	326.187 ms	new	3.345 ms	new

Core language and compiler benchmarks

Platform	Benchmark	ns/op	vs main
Linux	BenchmarkLookupPCRandom	12.450 ns/op	new
Linux	BenchmarkMergeCompilerFlags	143.600 ns/op	new
Linux	BenchmarkMergeLinkerFlags	93.720 ns/op	new
Linux	BenchmarkChannelBuffered	36.510 ns/op	new
Linux	BenchmarkChannelHandoff	24177 ns/op	new
Linux	BenchmarkDefer	42.440 ns/op	new
Linux	BenchmarkDirectCall	1.757 ns/op	new
Linux	BenchmarkGlobalRead	1.759 ns/op	new
Linux	BenchmarkGlobalWrite	2.806 ns/op	new
Linux	BenchmarkGoroutine	36128 ns/op	new
Linux	BenchmarkInterfaceCall	8.789 ns/op	new
Linux	BenchmarkRuntimeGetG	1.758 ns/op	new
macOS	BenchmarkLookupPCRandom	12.610 ns/op	new
macOS	BenchmarkMergeCompilerFlags	118.700 ns/op	new
macOS	BenchmarkMergeLinkerFlags	74.580 ns/op	new
macOS	BenchmarkChannelBuffered	21.130 ns/op	new
macOS	BenchmarkChannelHandoff	7185 ns/op	new
macOS	BenchmarkDefer	26.430 ns/op	new
macOS	BenchmarkDirectCall	1.003 ns/op	new
macOS	BenchmarkGlobalRead	0.961 ns/op	new
macOS	BenchmarkGlobalWrite	0.947 ns/op	new
macOS	BenchmarkGoroutine	27261 ns/op	new
macOS	BenchmarkInterfaceCall	4.122 ns/op	new
macOS	BenchmarkRuntimeGetG	2.109 ns/op	new

No main baseline exists yet; all metrics are marked new.

Warning

• The rendered benchmark data could not be pushed.