[brechtsanders]

Fix to use use proper flag instead of MSVC's /W0 when building for Windows with MinGW-w64

[sysrsbuild-gh]

Can one of the admins verify this patch?

[Nir-Az]

Auto-generated review by rs-agentic-bot 🤖

PR Review: Don't use /W0 flag when building with MinGW-w64

Summary

This is a correct and important fix. The root cause is that WIN32 is a CMake platform variable (true whenever the target OS is Windows, regardless of compiler), while MSVC is a CMake compiler variable (true only when the Microsoft Visual C++ compiler is used). Building with MinGW-w64 on Windows sets WIN32=TRUE but MSVC=FALSE, causing the original code to incorrectly apply the MSVC-only /W0 flag instead of GCC's -w.

Findings

See inline comments below for details. No blocking bugs detected — this is a clean targeted fix.

Test Coverage

⚠️ No MinGW-w64 CI build configuration is included or updated with this PR. Without a Windows+MinGW CI lane, this fix could silently regress. Consider adding a MinGW-w64 build to the test matrix or documenting how to manually verify this path.

[Nir-Az]

✅ Correct fix — MSVC is the right guard here.

Why this matters:

• WIN32 is a platform variable — TRUE for any Windows target regardless of compiler (MSVC, MinGW-w64/GCC, Clang, ICC).
• MSVC is a compiler variable — TRUE only when the Microsoft Visual C++ compiler is in use.

With the old if (WIN32) guard, a MinGW-w64 build would attempt to pass /W0 (an MSVC-only flag) to GCC, causing a compiler error. The new if (MSVC) guard correctly restricts MSVC flags to MSVC builds, and allows the else() branch to apply GCC-compatible -w for MinGW-w64.

Note: This also implicitly fixes the same issue for regular Clang-on-Windows (non-clang-cl mode), which is also GCC-compatible and would similarly fail with /W0.

🤖 This comment was auto-generated by the rs-agentic bot

[Nir-Az]

✅ Same correct fix applied to the else branch (non-INTERFACE_LIBRARY targets).

Enhancement suggestion: Consider whether the two branches (INTERFACE_LIBRARY vs. regular targets) could be refactored to avoid duplicating this platform/compiler logic. For example:

macro(disable_third_party_warnings target)
    if (TARGET ${target})
        get_target_property(target_type ${target} TYPE)

        if (MSVC)
            set(_warn_flag /W0)
        else()
            set(_warn_flag -w)
        endif()

        if (target_type STREQUAL "INTERFACE_LIBRARY")
            target_compile_options(${target} INTERFACE ${_warn_flag})
        else()
            target_compile_options(${target} PRIVATE ${_warn_flag})
        endif()
    else()
        message(WARNING "Target '${target}' does not exist.")
    endif()
endmacro()

This reduces the duplication of the compiler-detection logic and makes future compiler additions (e.g. Intel ICX) a single-point change. This is an optional enhancement and not a blocker.

🤖 This comment was auto-generated by the rs-agentic bot

[Nir-Az]

⚠️ Missing test / CI coverage

This fix addresses MinGW-w64 builds, but the repository does not appear to have a MinGW-w64 CI pipeline lane (per the supported platforms table in .github/copilot-instructions.md, Windows is tested with MSVC only).

To ensure this fix doesn't silently regress:

• Add a MinGW-w64 build job to the CI matrix (e.g., GitHub Actions with windows-latest + mingw-w64 toolchain), or
• Document in the PR description the manual steps to verify this with a MinGW-w64 build environment.

Without CI coverage, a future refactor of disable_third_party_warnings could re-introduce the same bug undetected.

🤖 This comment was auto-generated by the rs-agentic bot

[Nir-Az]

LGTM

[Nir-Az]

@brechtsanders thanks for the PR, great enhancement, merged!