fix(vm): warn-on-core-shadow never fired for compiled defs

[nnunley]

Summary

The warn-on-core-shadow check has never fired for user code. Every compiled (def …) and (defn …) bypassed it.

Stacked on #733.

The defect

Namespace.Def compares the name against clojure.core's refers and prints the Clojure-parity warning. Its own comment describes the case it exists for:

User code that uses the default (ns ...) form gets clojure.core auto-refered :all, so it does warn on shadow.

That case never reaches it. The def special form interns through Namespace.LookupOrAdd (pkg/compiler/compiler.go:1925), not through Def, so only Go-side ns.Def callers ever tripped the warning — the inverse of what the comment claimed.

Observable before this change:

$ cat 01_shadow_emits_warning.lg
(ns test.shadow-emits)
(defn nth [a b c] :x)
(println "result:" (nth 1 2 3))

$ lg 01_shadow_emits_warning.lg
result: :x        # no warning on stderr

Fix

Factor the check into warnOnCoreShadow and call it from both intern paths. Guards are unchanged: core itself, suppressShadowWarn, :exclude, unmapped, private core vars, and a name this namespace already owns. In LookupOrAdd the call sits on the interning branch only, so a repeated def of our own var stays silent.

What turning it on surfaced

One unsuppressed intentional shadow, during boot:

WARNING: read-string already refers to: #'clojure.core/read-string in namespace: edn, being replaced by: #'edn/read-string

edn/read-string shadowing clojure.core/read-string is the point of that namespace. Declared with (:refer-clojure :exclude [read-string]), matching how io/slurp and io/spit are handled through ns.Exclude on the Go side. Consumers are unaffected: (:require [edn :as e]), (:require [edn :refer [read-string]]), and :refer-clojure :exclude plus :refer all resolve as before.

Why it went unnoticed

test/namespace_shadow_warning_test/run.sh asserts on stderr, so it cannot live in the Go table tests. Nothing ran it — not make, not CI — so its first case had been failing silently. It is now make shadow-warning-test, invoked from make test.

Validation

• Harness 5/5, where case 1 previously failed.
• go test ./pkg/... ./test passes, with zero stray WARNING: lines anywhere in the run.
• Bundle and lowered tree regenerated for the edn change; make check-generated clean.

[mparrett]

The mechanism matters beyond parity: this warning is the guard against a generated //lg:native def shadowing a fast-path closure — the exact shape of the #656 reduce regression — and compiled defs are where those shadows originate, so the check being dead there was the guard's blind spot. The factoring keeps all six suppression guards and the warn-once-per-ns behavior, LookupOrAdd is compile-time-only so there's no dispatch-path cost, and the edn :refer-clojure :exclude matches the io/slurp precedent.

One finding, P2, non-blocking: the harness still doesn't run in CI. go.yml invokes go test directly in every job and never runs make test, so shadow-warning-test — now wired into make — remains un-executed exactly where "nothing ran it" bit before. Either add a step to go.yml (it already carries bespoke steps for the e2e lowering tests, so this matches the pattern) or wrap the harness in a Go test that shells out to run.sh so ./... picks it up. As written, a future regression in the warning would again pass CI silently.

[nnunley]

Performance: branch vs main

The ratchet's baseline (docs/perf/baseline.json) was captured on go1.26.3 and the current toolchain is go1.26.5, so make bench-ratchet reports regressions on main itself. A control run on unmodified main (af4b7f4) reproduces them with byte-identical allocation counts, which scopes them out of this change:

benchmark	control = main	this branch
pkg/compiler.BenchmarkInitFromLGB	+42.6%	+33.2%
pkg/ir.BenchmarkIRCompile [bytecode]	+9.9%	+10.4%
pkg/ir.BenchmarkIRCompile [gogen_ir]	−13.7%	−23.2%

Branch measured against main on the same toolchain and machine, anchor-normalised:

benchmark	main ×	branch ×	Δ vs main	wall
pkg/compiler.BenchmarkInitFromLGB	1 307 085	1 220 929	−6.6%	1.36 ms
pkg/ir.BenchmarkIRCompile [bytecode]	14 537 718	14 607 273	+0.5%	16.2 ms
pkg/ir.BenchmarkIRCompile [gogen_ir]	10 624 979	9 461 319	−11.0%	10.5 ms

Allocation counts are identical between control and branch — 7028 → 26118, 100159 → 114256, 111864 → 151751 — so none of the allocation growth in the ratchet output originates here.

Interleaved A/B

scripts/ab_repeat.py --n 7, profile pr-fast, order BH HB BH HB BH HB BH, 53 benchmarks on Apple M3 / go1.26.5:

would-gate (any comparable family's median > budget):
  budget  6%: clean
  budget  8%: clean
  budget 10%: clean

Every family median lands within ±0.8% (largest: VectorCreation/ArrayVector/100 +0.75%, VectorConj/ArrayVector/1000 +0.71%, FuncInvoke/Closure +0.22%). The harness classifies the run as a no-op.

Limits of the above

pr-fast covers pkg/vm under -tags gogen_ir only. It does not include BenchmarkInitFromLGB or BenchmarkIRCompile, so the two rows above are single-run measurements. Both point in the improvement direction, and neither is confirmed by repetition; I would not claim them as results.

Separately, docs/perf/baseline.json needs re-derivation against the current toolchain before the ratchet can gate anything — related to #663.

[nnunley]

Updated at 82f95da29f21. The stderr shell harness is now wrapped by a normal Go test, so direct go test ./... CI builds a temporary lg and executes all five warning cases. The shell runner honors quoted absolute, relative, and bare LG paths; the duplicate Make invocation was removed while retaining the manual target. Rebase integration also declares ir.zipper/next intentional and renames the private generator run! helper, leaving generation warning-clean. Final pre-push and generated gates pass.

[mparrett]

Approved. The core-shadow warning fix behaves as intended, and the focused harness, short package suite, VM race tests, vet, and generated-artifact checks pass.

Non-blocking P2 follow-up: test/namespace_shadow_warning_harness_test.go unconditionally executes Bash. On Windows or Plan 9, go test ./... will fail before exercising the warning behavior. Please consider implementing the assertions in Go or guarding/skipping the wrapper when Bash is unavailable.

The branch still needs rebasing because the generated manifest files currently conflict with main.

[mparrett]

@nnunley — approved from my side. It has picked up conflicts with main since; rebase when you get a chance and I'll merge it.

[mparrett]

Following up on my rebase note, because "conflicts" undersells what is happening and I would rather you did not chase it.

I fetched this head and ran git merge-tree --write-tree --name-only origin/main against it twice. With the repo's merge drivers registered it merges clean. With merge.sums.driver and merge.lgb.driver disabled, one path conflicts: pkg/rt/generated.sums, the one-line digest. That is the whole conflict. core_compiled.lgb merges without help.

GitHub does not run custom merge drivers on its server-side merge, so this PR will read CONFLICTING again the next time main regenerates, however recently you rebased. Two things follow. Locally, make install-hooks registers the sums driver and a plain-git rebase then resolves it for you, but the Makefile notes that jj does not run merge drivers either, so under jj it stays a make generate after the rebase. And the recurrence is structural rather than anything about this branch: it is what #635 and #683 are for.

So the rebase is still worth doing to get the merge box green, but treat the red as an artifact of the driver gap rather than a signal about the change. Same picture on #781.

[nooga]

LGTM, mparrett's approval stands and his CI-coverage nit is fixed in the follow-up commit. Only the generated manifest conflicts. Merging after a rebase.