← All tasks
cppgoogle/sentencepiece #1246Not a task: already works

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 |
Continue on GitHub ↗

04 / LABELS

Labels from the report text only; not yet run

No supported category has been assigned.

Label rules and the text that matched
[]