v3: clean up transform helpers/inits; cmd/v: fix v3.v bootstrap dispatch on macOS - #28157
Conversation
…nsform Resolve the three XTODO markers in vlib/v3/transform/transform.v: - Drop the arr1/arr2/arr3/arr4 helper wrappers and node_kind_id in favor of plain array literals and int(node.kind) at every call site across the transform module. - Remove the empty map initializers from new_transformer_view; V auto-initializes those struct map fields, so only the AST/type-checker views, used_fns, and the pointer-backed lookup caches need explicit values. - Keep the V3_STR_CAP experimental autostr-cap override (documented in fn.v) and replace the "is this needed" marker with an explanatory comment. No behavior change: transform unit tests pass and the v3 self-host transform phase and peak RSS are unchanged (verified before/after), with a byte-identical prod v3 binary. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The macOS dispatch gates only recognized the v3 bootstrap by the literal path vlib/v3/v3.v, so building it as a bare `v3.v` from inside vlib/v3 (e.g. `../../v -prealloc -prod -o v3 v3.v`) was treated as an ordinary .v build and delegated to the embedded V3 driver instead of the compatibility compiler. Add a shared is_macos_v3_compiler_bootstrap() helper that also resolves a file literally named v3.v through its real path, and use it in both is_macos_v3_relevant_command and macos_v3_force_requested so the two gates cannot drift. The extra real-path lookup only runs for a v3.v target. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 562f9f5c48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // A bare `v3.v` invoked from inside vlib/v3 must resolve to the bootstrap so | ||
| // it builds with the compatibility compiler instead of the embedded V3 driver. | ||
| repo_root := os.dir(os.dir(os.real_path(@FILE))) |
There was a problem hiding this comment.
Ascend one more directory before testing the bare path
With this test located at cmd/v/macos_v3_test.v, two os.dir calls resolve repo_root to <repo>/cmd, so v3_dir becomes <repo>/cmd/vlib/v3. The existence guard is therefore false in a normal checkout, and the key bare and dotted assertions never run, allowing the exact bare-v3.v dispatch regression this test targets to pass unnoticed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — the two os.dir calls resolved repo_root to <repo>/cmd, so the vlib/v3 existence guard was always false and the key assertions never ran.
Fixed in 3b22b60 by dropping the @FILE-relative guard entirely: the test now builds an isolated <tmp>/vlib/v3/v3.v (plus a sibling non-bootstrap v3.v) and chdirs into it, so the real-path resolution is exercised unconditionally regardless of where the test file lives. I also confirmed the test now fails on assert bare if the helper's real-path branch is removed, so the regression can no longer pass unnoticed.
The previous test derived the repo root with two os.dir calls from a file at cmd/v/macos_v3_test.v, which resolves to <repo>/cmd, so the vlib/v3 existence guard was always false and the bare-`v3.v` assertions never ran. Build an isolated <tmp>/vlib/v3/v3.v (and a sibling non-bootstrap v3.v) instead, so the real-path resolution is exercised unconditionally regardless of where the test file lives, and add the negative case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
What
Two related changes that came out of resolving the
XTODOmarkers invlib/v3/transform/transform.vand then verifying self-host performance.1.
v3: transform helper/init cleanup (resolves the 3 XTODOs)arr1/arr2/arr3/arr4wrappers andnode_kind_id, replacing every call site in the transform module with plain array literals ([a, b, ...]) andint(node.kind).map[...]{}initializers fromnew_transformer_view— V auto-initializes those struct map fields, so only the AST/type-checker views,used_fns, and the pointer-backed lookup caches (which would otherwise be nil) need explicit values.V3_STR_CAPexperimental autostr-cap override (documented infn.v) and replace the// XTODO is this neededmarker with an explanatory comment.2.
cmd/v: route a barev3.vbootstrap to the compat compiler on macOSThe macOS dispatch gates only recognized the v3 bootstrap by the literal path
vlib/v3/v3.v. Building it as a barev3.vfrom insidevlib/v3(e.g.../../v -prealloc -prod -o v3 v3.v) was treated as an ordinary.vbuild and delegated to the embedded V3 driver instead of the compatibility compiler (which then crashes and only survives via the auto-fallback retry). A sharedis_macos_v3_compiler_bootstrap()helper now also resolves a file literally namedv3.vthrough its real path, and bothis_macos_v3_relevant_commandandmacos_v3_force_requesteduse it so they can't drift. The extra real-path lookup only runs for av3.vtarget.Verification
v fmt -verifyclean on all changed files;v3builds.Transform unit tests (
v test vlib/v3/transform/) pass 7/7.New dispatcher test
test_macos_v3_compiler_bootstrap_is_detected_from_any_cwd; before/after, the bare-v3.vinvocation goes from "embedded V3 invoked → segfault → fallback" to a clean compat build.No self-host performance regression (
-prod, 5 runs each, baseline @ HEAD vs. this branch):🤖 Generated with Claude Code