Skip to content

ir: give core-qualified heads the builtin ops - #1060

Merged
mparrett merged 2 commits into
mainfrom
wt/ir-qualified-gate
Oct 17, 2026
Merged

mparrett merged 2 commits into
mainfrom
wt/ir-qualified-gate

Conversation

@mparrett

Copy link
Copy Markdown
Collaborator

Fixes the lowered-Go half of #1045. Stacked on #1058 (base wt/aget-opcode), which fixed the bytecode half. It does not depend on #1058's code; it is stacked so the two halves of the issue review together.

intrinsic-builtin-op in build.lg required (nil? (namespace head)), so the core/< and core/inc that syntax-quote emits built as ordinary var calls, as did any explicit clojure.core/<. Under lg compile that turned the issue's qual loop and every dotimes counter into vm.Value locals with a cached-var trampoline per op.

The gate now accepts a qualified head on the same terms #1058 gave the bytecode gate. A qualified symbol cannot be shadowed by a local, so it skips that check and keeps the identity check: resolve of the head must be the clojure.core var of that name. That admits aliases and refuses a qualified head that names another namespace's inc, including through an alias named core. A core/ head that does not resolve falls back to the core var, the same fallback fastOpcodeCoreName has, so the two engines agree. The unqualified path is unchanged; it already did the identity check.

*ir-compile* builds bytecode from the same IR, so on that path qualified heads now get OP_ADD and the other fast opcodes too, as the plain compiler does after #1058.

Verified

qual now lowers like plain: j int64, acc int64, j < arg0, no CachedVarFn. New tests in lisp_lower_go_qualified_gate_test.go cover the repro spelled core/ and clojure.core/, an aliased head, a dotimes counter lowering to int64 through lower-ns-to-go (which macroexpands, as lg compile does), and a qualified non-core inc staying a call, directly and through an alias named core. The four positive tests fail with the old gate restored. pkg/vm, pkg/ir, pkg/compiler, pkg/genmanifest and pkg/rt/... pass; make generate and check-generated are clean; linux, js/wasm, plan9 and wasip1 build.

Not timed: the lowered loop has the same native locals and operators as plain, so the issue's plain numbers are the expectation.

🤖 Generated with Claude Code

@nooga nooga left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 3209553 against #1058. Qualified builtin detection retains the core Var identity check while accepting explicit core names and aliases, so syntax-quoted macro heads receive the same IR builtin operations as unqualified heads. A qualified non-core Var, including an alias named core, remains a call.

Local verification at this exact head: the complete VM, compiler, bytecode and IR package suites pass, including the qualified/aliased gate and dotimes lowering tests. make check-generated passes through full regeneration, committed-artifact comparison, and Go-lowered compilation/native dispatch. The array recur generated-Go probe also matches bytecode. No actionable finding; approving the delta. GitHub currently shows only skipped performance workflows on this head, so normal correctness CI should run before merging the stack.

@nnunley nnunley left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. intrinsic-builtin-op now resolves a qualified head and compares it to the clojure.core var, so core/< from syntax-quote gets the builtin op and other/inc does not. I checked the alias-to-user-ns case (core aliased to a user ns stays a call) and the unqualified path is unchanged. Merge after #1058, which this stacks on.

@nooga nooga left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final head 9290428 matches the bytecode gate's core/ fallback; local suites and generated checks pass. Merge after #1058 — CI only runs fully once it's retargeted to main, so check it then.

Base automatically changed from wt/aget-opcode to main October 17, 2026 19:46
mparrett and others added 2 commits October 17, 2026 12:48
intrinsic-builtin-op refused every namespaced head, so clojure.core/< and
clojure.core/inc from syntax-quote built as var calls. Under lg compile that
left dotimes counters and any macro-emitted arithmetic as vm.Value with a
cached-var trampoline per op (#1045). The bytecode side of the gate was
fixed in the parent commit; this is the IR side.

A qualified head cannot be shadowed by a local, so it skips the shadowing
check and keeps the identity check: it must resolve to the clojure.core var
of that name. Aliases pass, and a qualified head naming another namespace's
var of the same name stays a call.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Syntax-quote emits core/< (clojure.core is an alias of the core ns), so the
tests now cover that spelling as well as clojure.core/<, an alias named core
that points at a user ns, and a dotimes counter that must actually lower to
an int64 rather than merely avoid the var trampoline.

A core/ head that does not resolve now falls back to the core var, as
fastOpcodeCoreName does, so the two engines agree when the current ns has an
alias named core. Unqualified heads skip building a new symbol.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mparrett
mparrett force-pushed the wt/ir-qualified-gate branch from 9290428 to ddfb342 Compare October 17, 2026 19:49
@mparrett

Copy link
Copy Markdown
Collaborator Author

@nooga Thanks for the review! Merging.

@mparrett
mparrett merged commit ff9f31a into main Oct 17, 2026
21 checks passed
@mparrett
mparrett deleted the wt/ir-qualified-gate branch October 17, 2026 20:00
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