CMake: cl.exe-only `/D` and `/wd` flags injected for Clang on Windows in the bundled-protobuf path
envgap__google__sentencepiece-1246
01 / FAILURE SIGNATURE
As reported upstream
clang++: error: no such file or directory: '/DHAVE_PTHREAD'
Not a benchmark task.
- The project already builds and runs before the fix, so there is nothing to repair.
02 / ENVIRONMENT RECIPE
- Base commit
b52379a12779d94c09fd4c16dd3c2bdfe736d413- Manifest
src/CMakeLists.txt- Reproduce
Awaiting issue-specific recipe- Run under trace
Awaiting a meaningful runtime command
03 / ORIGINAL ISSUE TEXT
google/sentencepiece #1246 · read the original issue
**Describe the bug**
In `src/CMakeLists.txt`, the bundled-protobuf branch (`if (SPM_PROTOBUF_PROVIDER STREQUAL "internal")`, the default) injects compiler flags using a two-way split keyed on `WIN32`:
```cmake
if (WIN32)
add_definitions("/DHAVE_PTHREAD /wd4018 /wd4514")
else()
add_definitions("-pthread -DHAVE_PTHREAD=1 -Wno-sign-compare -Wno-deprecated-declarations")
endif()
```
The `WIN32` branch assumes the compiler is `cl.exe` and emits cl.exe-only syntax: `/D<macro>` for defines and `/wd<n>` for warning suppression. But `WIN32` is true for **any** Windows toolchain, including:
- Clang invoked via its GNU-style driver (`clang.exe`, e.g. `clang.exe --target=arm64-pc-windows-msvc`).
- MinGW GCC / Clang.
With a GNU-style driver, slash-prefixed tokens are not interpreted as flags. Clang treats them as input file paths, producing:
```
clang++: error: no such file or directory: '/DHAVE_PTHREAD'
clang++: error: no such file or directory: '/wd4018'
clang++: error: no such file or directory: '/wd4514'
```
…and the build of `sentencepiece-static`'s bundled protobuf-lite objects (`arena.cc`, etc.) fails immediately.
`clang-cl.exe` is unaffected because it is a cl.exe-compatible driver and CMake reports `MSVC=TRUE` for it.
The `else` branch has the mirror-image issue: `-pthread` is a GNU/Unix driver flag with no meaning on Windows. The GNU-style Windows-Clang driver rejects it as well, but that branch is not currently reached because `WIN32` short-circuits to the broken branch first.
**Follow-up to #1240**
This is a follow-up to my own PR #1240 / commit `b4816a6aef1e375a5e3c16049588e83508044579` ("avoid injecting -fPIC and other unix flags on windows", fixed #1239). That PR switched this gate from `if (MSVC)` to `if (WIN32)` to keep Windows configurations on the cl.exe-style branch. That worked for `cl.exe` and `clang-cl.exe` but I missed that the body of the `WIN32` branch uses MSVC-specific syntax (`/D`, `/wd`), which Windows-non-MSVC compilers (Clang via the GNU-style driver, MinGW) cannot consume. The fix below splits the branch so the cl.exe-style body is only used when the compiler is actually MSVC-compatible.
**To Reproduce**
- OS: Windows 10/11 (any Windows host with a GNU-style Clang or MinGW reproduces).
- Compiler: `clang.exe` driven by a CMake toolchain file that targets the MSVC ABI via the GNU-style driver, e.g.
```
set(CMAKE_C_COMPILER clang.exe)
set(CMAKE_CXX_COMPILER clang++.exe)
set(CMAKE_C_COMPILER_TARGET arm64-pc-windows-msvc)
set(CMAKE_CXX_COMPILER_TARGET arm64-pc-windows-msvc)
```
MinGW also reproduces.
- CMake: 3.27+, Ninja generator.
- Defaults: `SPM_PROTOBUF_PROVIDER=internal` (the default).
- Commands:
```
cmake -S . -B build ^
-G Ninja ^
-DCMAKE_TOOLCHAIN_FILE=<path/to/clang-windows.cmake> ^
-DCMAKE_BUILD_TYPE=Release
cmake --build build
```
- Observed: `clang++: error: no such file or directory: '/DHAVE_PTHREAD'` (and `/wd4018`, `/wd4514`) when compiling `sentencepiece-static.dir/.../protobuf-lite/arena.cc.obj`. `clang-cl.exe` and `cl.exe` are unaffected; setting `SPM_PROTOBUF_PROVIDER=package` also avoids the block.
**Expected behavior**
The cl.exe-only `/D` and `/wd` flags should only be applied when the compiler is actually `cl.exe` (or a cl.exe-compatible driver such as `clang-cl.exe` for which `MSVC=TRUE`). For other Windows compilers (GNU-style Clang, MinGW), GNU-style flags should be used — without `-pthread`, since that does not apply on Windows.
**Proposed fix**
Split the two-way branch into a three-way branch keyed on the compiler (`MSVC`), not the OS (`WIN32`). The new middle branch covers Windows-but-not-cl.exe — exactly the configurations that are broken today:
```cmake
if (MSVC)
add_definitions("/DHAVE_PTHREAD /wd4018 /wd4514")
elseif (WIN32)
# Windows but not cl.exe (e.g. Clang targeting *-pc-windows-msvc, MinGW).
add_definitions("-DHAVE_PTHREAD=1 -Wno-sign-compare -Wno-deprecated-declarations")
else()
add_definitions("-pthread -DHAVE_PTHREAD=1 -Wno-sign-compare -Wno-deprecated-declarations")
endif()
```
`clang-cl.exe` keeps the `MSVC` branch (CMake reports `MSVC=TRUE` for it). `cl.exe`, Linux GCC/Clang, and macOS Clang receive the same flags as before.
| Toolchain | `MSVC` | `WIN32` | Branch (before) | Branch (after) | Behavior change |
|---|:-:|:-:|---|---|---|
| Windows + cl.exe | ✓ | ✓ | `WIN32` | `MSVC` | identical flags |
| Windows + clang-cl.exe | ✓ | ✓ | `WIN32` | `MSVC` | identical flags |
| Windows + Clang (GNU-style driver) | ✗ | ✓ | `WIN32` (broken) | new `WIN32` branch | formerly broken, now compiles |
| Windows + MinGW | ✗ | ✓ | `WIN32` (broken) | new `WIN32` branch | formerly broken, now compiles |
| Linux GCC/Clang | ✗ | ✗ | `else` | `else` | identical flags |
| macOS Clang | ✗ | ✗ | `else` | `else` | identical flags |
04 / LABELS
Labels from the report text only; not yet run
No supported category has been assigned.
Label rules and the text that matched
[]