cmd: validate explicit C compiler before dispatch#27871
Conversation
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e470ea5181
ℹ️ 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".
| if builder.should_find_windows_host_c_compiler(prefs) && prefs.ccompiler_set_by_flag | ||
| && prefs.ccompiler != 'msvc' { |
There was a problem hiding this comment.
On Windows this probe runs immediately after parsing, before the dispatch below decides whether the command is help, version, or an external tool such as fmt. As a result, v -cc missing help, v -cc missing version, or a stale VFLAGS=-cc missing now exits with a missing-C-compiler error even though those commands do not compile user code and previously reached their normal handlers; scope the early check to build/source/unknown-command paths that actually need this precedence.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b28e283703
ℹ️ 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".
| if builder.should_find_windows_host_c_compiler(prefs) && prefs.ccompiler_set_by_flag | ||
| && prefs.ccompiler != 'msvc' { | ||
| mut probe := builder.Builder{ | ||
| pref: unsafe { prefs } |
There was a problem hiding this comment.
On Windows this $if windows block is type-checked from module main, but Builder.pref is a private field declared in vlib/v/builder/builder.v; cross-module struct initialization of private fields is rejected by the checker. As a result, rebuilding the compiler on Windows fails here before the new -cc diagnostic can run, so the probe needs to be exposed through a builder-side helper or otherwise avoid setting private fields from cmd/v.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Thanks for the review. This is not a visibility issue: Builder.pref is declared under pub mut: in vlib/v/builder/builder.v, so it is accessible from cmd/v.
I rebuilt the compiler from this PR on Windows successfully (v -g -keepc -o vreview.exe cmd/v) and ran v test-all on the current commit. No checker error occurs for this struct initialization.
2 participants
What
Validate an explicitly selected C compiler before command dispatch on Windows.
Why
An invalid
-ccvalue could reach command handling first when no command or an invalid command was supplied, hiding the useful missing-compiler diagnostic. This reuses the existing Windows C-compiler validation and adds a regression test for both cases.Fixes #27868
Tests
v test-allv test cmd/v/v_windows_test.v