v3: make -cc msvc work again - #28922
Conversation
`-cc msvc` failed for every program: the driver passed `cl` the gcc-style
command line it builds for gcc/clang, and the generated C relies on GNU
extensions that MSVC does not implement.
Driver:
- translate the gcc-style arguments into `cl` options (`/Fe`, `/Fo`, `/c`,
`/I`, `/D`, `-l` -> `.lib`, linker options after `/link`, the default
Windows libraries), for the program and for third-party objects
- build third-party objects per compilation, since `cl` has no `-M`
- keep MSVC builds in one translation unit, skip the `-dM` macro probe
Generated C:
- lower GNU statement expressions into ordinary statements before `cl`
runs (cgen.msvc_compat_c_source). Their locals are renamed apart and
hoisted; `&&`, `||`, `?:`, loop conditions, `for` post expressions and
`else if` chains keep their evaluation semantics. The same pass drops
same-type struct casts, turns `{}` into `{0}` and drops compound
literals from static initializers.
- MSVC preamble: no GNU attributes, `_Thread_local`/`_Atomic` spellings,
V's WinAPI atomics header instead of <stdatomic.h>, Interlocked
prealloc atomics, plain sum type tag reads.
Fixes #28919
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![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: c4dccf7fe0
ℹ️ 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".
| '-x' { | ||
| i++ | ||
| continue |
There was a problem hiding this comment.
-x
When a native source uses -x c++ with a .c filename, this discards the language selection, leaves has_cpp false, and later adds /std:c11; MSVC then treats the file as C rather than C++. The driver deliberately tracks an active -x language for native inputs, so translate c++ to /TP (and suppress C11 mode) instead of dropping it. See MSVC's source-type options.
Useful? React with 👍 / 👎.
| } else if arg.starts_with('/') && !os.exists(arg) { | ||
| // An MSVC option given directly, for example with `-cflags`. | ||
| compile << arg |
There was a problem hiding this comment.
/link
When users pass a native linker switch such as /LIBPATH:C:\libs or /NODEFAULTLIB:foo through -cflags, this branch puts it in compile, before the source inputs and the generated /link; cl therefore interprets or rejects it as a compiler option instead of forwarding it to link.exe. Classify linker-only slash switches into link, and handle a direct /link marker without placing it before the input files. See MSVC's /link option.
Useful? React with 👍 / 👎.
`v -cc msvc -os windows -o v_win.c` output is also compiled with MinGW GCC (Cross CI). Choose between V's WinAPI atomics header and the builtin atomic helpers in the preprocessor, so the file works with both. MSVC backtraces call the dbghelp API, which needs its declarations under `/we4013`; pass the frame addresses as the `DWORD64` it expects.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d9d371214
ℹ️ 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".
| b.step_parallel('cgen', cgen_was_parallel) | ||
| } | ||
| if effective_c_compiler == 'msvc' && !cache_state.manager.enabled && !c_to_stdout { | ||
| msvc_lower_c_file(cc_src) or { |
There was a problem hiding this comment.
typeof before invoking MSVC
When a program calls .pointers() on a fixed array, gen_fixed_array_pointers_expr emits a GNU-style typeof(...) declaration (vlib/v/gen/c/array.v:975), but this compatibility pass leaves that token unchanged. The resulting source is then compiled with /std:c11; Microsoft documents C typeof as requiring /std:clatest, so these otherwise valid V programs fail under -cc msvc. Add an MSVC-specific declaration in cgen or teach this pass to replace the emitted typeof.
Useful? React with 👍 / 👎.
A literal-output program (hello world) gets a reduced set of runtime seeds. Under MSVC the backtrace code is reachable from it, and slices its symbol names with `string.substr`, which only appears after markused; the C then called an undeclared `string__substr`. math.bits uses MSVC intrinsics such as `_udiv128`, declared by <intrin.h>.
- Block-scope `extern` declarations and function prototypes inside a
statement expression name global symbols; renaming them broke links
(`builtin__none__` in the ownership drop code).
- `cJSON_ArrayForEach(item, array) { ... }` is a macro that expands to a
loop header. Parse `name(...) { ... }` as a header with a block body, so
statement expressions in the body are not hoisted out of the loop (JSON
decoding of fixed arrays returned zeroes).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: baceb320e2
ℹ️ 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".
| // `_Atomic` needs `/experimental:c11atomics`; with `/volatile:ms`, volatile | ||
| // accesses have the acquire/release semantics V relies on. | ||
| g.writeln('#if defined(_MSC_VER) && !defined(__clang__)') | ||
| g.writeln('#define __attribute__(x)') |
There was a problem hiding this comment.
For any V type declared with @[aligned] or @[aligned: N], emit_struct still emits a GNU __attribute__((aligned(...))) suffix, so this macro silently removes the requested alignment when compiling with MSVC. This contradicts the existing MSVC expectation in vlib/v/gen/c/testdata/aligned_attr_msvc_windows.c.must_have, which requires __declspec(align(...)), and can leave stack variables and array elements under-aligned. Emit the MSVC alignment spelling before globally erasing GNU attributes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7494bbb874
ℹ️ 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 host_os == 'windows' { | ||
| eprintln('`-cc msvc` could not find `${c_compiler}`. Run V from a Visual Studio Developer Command Prompt (or after `vcvars64.bat`), so that `cl` and its INCLUDE/LIB environment are available.') | ||
| } else { | ||
| eprintln('`-cc msvc` can only compile on Windows; use `-o file.c` to generate C for MSVC on ${host_os}.') |
There was a problem hiding this comment.
On a non-Windows host when the original command did not specify -os windows, following this diagnostic with -cc msvc -o file.c still generates C for the host target, including its platform-specific V sources and declarations, so the result cannot be compiled by Windows MSVC. The suggested command needs to include -os windows (or otherwise force the target) to produce the promised MSVC-compatible Windows source.
Useful? React with 👍 / 👎.
The MSVC preamble defines `__attribute__` away, which also dropped the `__attribute__((aligned(N)))` suffix of `@[aligned]` structs. For MSVC, emit `__declspec(align (N))` between the tag and the name instead (16 for a bare `@[aligned]`, GCC's meaning on x86-64 and arm64), as in testdata/aligned_attr_msvc_windows.c.must_have. Map `__alignof__`, used for heap copies of such structs, to MSVC's `__alignof`.
Off Windows, the missing-`cl` diagnostic suggested `-cc msvc -o file.c`, which still generates C for the host target. Suggest `-os windows` (and `-arch amd64` when the architecture would follow a non-x64 host) unless the build already targets Windows.
1 participant
Fixes #28919.
-cc msvcfailed for every program, for the two reasons in the issue: the V3 driver handedclthe gcc-style command line it builds for gcc/clang, and the generated C uses GNU extensions that MSVC lacks, statement expressions above all (cmd/valone has ~14k of them).Driver (
vlib/v/driver/msvc.v)msvc_cl_argstranslates the gcc-style arguments intocloptions:/Fe//Fo//c//LD,/I,/D,-O*→/O1//O2,-l→.lib,-L/-Wl,...→ options after/link, and the default Windows libraries. Options with no MSVC meaning (-std=gnu11,-W...,-f...,-m...) are dropped./nologo /volatile:ms /bigobj /MD /we4013 /utf-8 /std:c11 /D_CRT_DECLARE_NONSTDC_NAMES=1, a 32 MiB stack, and the subsystem. The last define is needed because/std:c11defines__STDC__, which hides the POSIX CRT names V calls.#flag x.o: libgc, cJSON, ...) are compiled incl's default mode, as V1 did, once per build, becauseclhas no-Mdependency output to validate a cached object.-dMmacro probe and keep the full runtime seeds: under MSVC, the backtrace code of even a hello world slices strings.clgets an explanation (use a Developer Command Prompt /vcvars64.bat).Generated C
cgen.msvc_compat_c_source(vlib/v/gen/c/msvc_compat.v) runs on the generated C of MSVC builds only. It hoists statement expressions into ordinary statements before the statement that uses them. Their locals are renamed to unique names, so hoisting cannot collide with or shadow anything;externdeclarations and prototypes keep their names. Conditionally evaluated operands keep their semantics:&&/||: a flag guards the operand's statements.?:: branch statements go into anif/else, with their declarations hoisted.while/do/forconditions andforpost expressions: the loop is restructured, andcontinuebecomes agotowhere needed.else ifchains: a "branch taken" flag keeps the chain flat, since MSVC limits block nesting to 128. The deepest nesting incmd/v's C is 14.cJSON_ArrayForEach(...) { ... }keep their bodies.The same pass drops same-type struct casts, turns
{}into{0}, and removes compound literals from static initializers.MSVC preamble:
__declspec(thread)for_Thread_local, andvolatilefor_Atomic(as V1 did);atomic.hinstead of<stdatomic.h>, chosen by the preprocessor so the same C still builds with MinGW;Interlocked*prealloc atomics;<intrin.h>and<dbghelp.h>;@[aligned]structs use__declspec(align (N)), and__alignof__maps to__alignof.GCC/Clang/TCC output is unchanged. Every new C construct is behind
_MSC_VER, or is only produced for-cc msvc.Testing
There is no MSVC on the machine this was written on, so the Windows MSVC CI of this PR is the first real
clrun.What was checked locally:
cmd/v, after lowering, compiles with-Werror=gnu-statement-expression. The resulting compiler builds itself and generates C forcmd/vbyte-identical to the unlowered compiler's.cl→ clang on macOS (translated options, statement expressions as errors):vlib/v/tests: 2270 passed. 16 failed; 10 of those also fail with the normal compiler, and the other 6 were 3 macOS Objective-C/Metal tests, 1 empty-struct size check (expects MSVC's 1 byte), and 2 real bugs, now fixed.vlib/json: 52/52.cl→ MinGW GCC 14 (implicit declarations are errors, like/we4013): 338 of 349 examples/vlib/vlib/v/testsfiles build and link for Windows. All 11 failures are missing sqlite/wkhtmltox headers, or also fail with plain MinGW. The-cc msvc -os windowsC ofcmd/valso builds with MinGW, as Cross CI does.vlib/v/gen/c/msvc_compat_test.vandvlib/v/driver/msvc_test.v.preamble_test.vandcross_output_codegen_test.vnow expect the Windows atomics header guard to include MSVC.Known limitations
clmust already be onPATH(a Developer Command Prompt /vcvars64, orilammy/msvc-dev-cmdin CI). V1's registry/vswhere lookup is not ported.