build: fix glslang compilation on Windows with MSVC (#11801)
> [!WARNING] > Review/approve this AFTER #11798 and #11800 (this PR stacks on top of rhem... ergo, it includes their commits) > Don't cheat! Start from the oldest one! 😄 I know these are almost one-liners but I am doing this mostly for documentation and karma points. ## Summary - Conditionally skip `linkLibCpp()` on MSVC since Zig's bundled libc++ headers conflict with MSVC's own C++ runtime - Add `-std=c++17` flag for C++17 features (std::variant, std::filesystem, inline variables) that glslang requires ## Context The exact same `linkLibCpp` fix was applied to `simdutf` and `highway` in commitspull/11815/head3d581eb92andb4c529a82but glslang was missed. Without this fix, glslang fails with 297 compilation errors on MSVC. Thanks Claude for the forensic digging. A carpenter should always be thankful for his tools. Even if they are borrowed, maybe even more so. ## Stack Stacked on 011-windows/fix-oniguruma-msvc. ## Discussion points **`-std=c++17` scope:** Currently added unconditionally for all targets. Tested on all three platforms with no regressions, but since this is specifically fixing a Windows/MSVC issue, it could be gated behind `target.result.abi == .msvc`. Donno. The reason it works unconditionally is that Zig's bundled clang already defaults to C++17 on non-MSVC targets, so the flag is a no-op there. Open to either approach. **Other packages with bare `linkLibCpp()`:** The same `linkLibCpp` guard has been applied to `simdutf`, `highway`, `utfcpp`, and now `glslang`. However, `spirv-cross`, `dcimgui`, `harfbuzz`, and `breakpad` still have unconditional `linkLibCpp()` calls. These may need the same treatment when they become buildable on MSVC (some are currently blocked by other issues like freetype's `unistd.h`). Worth tracking as a follow-up? ## Test plan ### test-lib-vt | | Windows | Linux | Mac | |---|---|---|---| | **BEFORE** | 3791/3839 passed, 48 skipped | 3791/3839 passed, 48 skipped | 3807/3839 passed, 32 skipped | | **AFTER** | 3791/3839 passed, 48 skipped | 3791/3839 passed, 48 skipped | 3807/3839 passed, 32 skipped | | **Delta** | no change | no change | no change | ### all tests (`zig build test` / `zig build -Dapp-runtime=none test` on Windows) | | Windows | Linux | Mac | |---|---|---|---| | **BEFORE** | FAIL — 38/51 build steps, 5 failed | 2655/2678 passed, 23 skipped (86/86 steps) | 2655/2662 passed, 7 skipped (160/160 steps) | | **AFTER** | FAIL — 39/51 build steps, 4 failed | 2655/2678 passed, 23 skipped (86/86 steps) | 2655/2662 passed, 7 skipped (160/160 steps) | | **Delta** | +1 build step (glslang unblocked) | no change | no change | - Zero regressions on any platform - Windows improved: glslang now compiles (38 -> 39 steps, 5 -> 4 failures) - Remaining 4 Windows failures (`helpgen`, `framegen`, `freetype`, `translate-c`) are addressed by other PRs in the stack ## What I Learnt - **MSVC's clang doesn't default to C++17.** Zig's bundled clang uses C++17 by default on Linux/Mac, but when targeting MSVC, the C++ standard needs to be specified explicitly. Without `-std=c++17`, features like `std::variant`, `std::filesystem`, and `inline` variables are gated behind `_HAS_CXX17` and won't compile. - **`linkLibCpp` conflicts with MSVC headers.** Zig's `linkLibCpp` passes `-nostdinc++` and adds its own libc++/libc++abi headers, which collide with the C++ headers already provided by the MSVC SDK through `linkLibC`. On MSVC, you don't need `linkLibCpp` at all since the SDK includes both C and C++ headers. I think yesterday we dealt with something similar. Windows is fun. 🫠 Unironically and chronically. - **Grep wider.** The `linkLibCpp` guard was already applied to simdutf, highway, and utfcpp but missed glslang. When a fix follows a repeated pattern across packages, search the whole codebase before declaring it complete.
commit
57b929203b
|
|
@ -51,7 +51,14 @@ fn buildGlslang(
|
|||
.linkage = .static,
|
||||
});
|
||||
lib.linkLibC();
|
||||
lib.linkLibCpp();
|
||||
// On MSVC, we must not use linkLibCpp because Zig unconditionally
|
||||
// passes -nostdinc++ and then adds its bundled libc++/libc++abi
|
||||
// include paths, which conflict with MSVC's own C++ runtime headers.
|
||||
// The MSVC SDK include directories (added via linkLibC) contain
|
||||
// both C and C++ headers, so linkLibCpp is not needed.
|
||||
if (target.result.abi != .msvc) {
|
||||
lib.linkLibCpp();
|
||||
}
|
||||
if (upstream_) |upstream| lib.addIncludePath(upstream.path(""));
|
||||
lib.addIncludePath(b.path("override"));
|
||||
if (target.result.os.tag.isDarwin()) {
|
||||
|
|
@ -65,6 +72,10 @@ fn buildGlslang(
|
|||
"-fno-sanitize=undefined",
|
||||
"-fno-sanitize-trap=undefined",
|
||||
});
|
||||
// MSVC requires explicit std specification otherwise C++17 features
|
||||
// like std::variant, std::filesystem, and inline variables are
|
||||
// guarded behind _HAS_CXX17.
|
||||
try flags.append(b.allocator, "-std=c++17");
|
||||
|
||||
if (target.result.os.tag == .freebsd or target.result.abi == .musl) {
|
||||
try flags.append(b.allocator, "-fPIC");
|
||||
|
|
|
|||
Loading…
Reference in New Issue