facebook / facebook/hermes

`API/jsi/jsi/CMakeLists.txt` tests `CMAKE_CXX_COMPILER_ID MATCHES "MSVC"`, so clang-cl gets neither branch's flags

Open Beginner friendly
#2,174 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
11.3k
Forks
859
Avg merge
1h 30m
Merged PRs (30d)
3

Description

`API/jsi/jsi/CMakeLists.txt:15-24` chooses `jsi`'s compile flags off the
**compiler ID string**:

```cmake
set(jsi_compile_flags "")
if ("${CMAKE_CXX_COMPILER_ID}" MATCHES "Clang" OR
"${CMAKE_CXX_COMPILER_ID}" MATCHES "GNU")
list(APPEND jsi_compile_flags "-Wno-non-virtual-dtor")
elseif ("${CMAKE_CXX_COMPILER_ID}" MATCHES "MSVC")
# Turn on Error Handling in MSVC, otherwise objects are not destructed
# when they go out of scope due to exceptions.
list(APPEND jsi_compile_flags "/EHsc")
endif()
```

Under **clang-cl** the ID is `Clang`, so the first branch is taken and the
`/EHsc` at `:22` — whose comment says exactly why it is needed — is never
reached. That matters because clang-cl targets the MSVC ABI and defaults to
exceptions **off**, so `jsi.cpp` fails to compile:

```
jsi.cpp(113,5): error: cannot use 'throw' with exceptions disabled (x11)
FAILED: .../jsi/CMakeFiles/jsi.dir/jsi.cpp.obj
```

CMake's `MSVC` variable is **true for clang-cl** and is the test that expresses
the intent here ("this compiler is targeting the MSVC ABI"); the ID string is
not.

**This is filed separately from the `hermes_update_cxx_flags` /
`/EHsc`-strip issue (#2173) on purpose.** `jsi` is the one target in the tree that never
goes through `hermes_update_cxx_flags`, so fixing that function would not fix
this file. It is the same mistake in a second, independent place.

### Repro

```bat
git clone https://github.com/facebook/hermes.git hermes-static
cd hermes-static
git checkout 5cee10abc93667ea5538caecaf0a457c66fa5bdc

call "C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Auxiliary\Build\vcvars64.bat"
set CLANGCL=C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/Llvm/x64/bin/clang-cl.exe

cmake -S . -B build -G Ninja -DCMAKE_BUILD_TYPE=Release ^
-DCMAKE_C_COMPILER="%CLANGCL%" -DCMAKE_CXX_COMPILER="%CLANGCL%" ^
-DHERMES_ENABLE_TEST_SUITE=OFF -DHERMES_ENABLE_NAPI=OFF
```

(`HERMES_ENABLE_TEST_SUITE=OFF` and `HERMES_ENABLE_NAPI=OFF` only shorten the
build; neither is load-bearing for the failure below.) Then:

```bat
cmake --build build --target jsi
```

Expected: compiles. Actual: eleven `cannot use 'throw' with exceptions disabled`
in `API/jsi/jsi/jsi.cpp`.

To see the choice itself rather than its consequence, add
`message(STATUS "jsi flags: ${jsi_compile_flags}")` after `:23` — it prints
`-Wno-non-virtual-dtor` under clang-cl, with no `/EHsc`.

### Environment

```
Windows 11 Pro 10.0.26200, x86-64
CMake 3.31.6-msvc6 (the one Visual Studio 2022 ships), Ninja 1.13.2
Visual Studio 2022 Community

> clang-cl --version
clang version 19.1.5
Target: x86_64-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\Llvm\x64\bin
```

### Proposed fix (one line)

Change the `elseif` at `API/jsi/jsi/CMakeLists.txt:19` to test CMake's `MSVC`
variable rather than the compiler ID string — i.e. `elseif (MSVC)` — so any
MSVC-ABI compiler, clang-cl included, gets the `/EHsc` the comment above it asks
for. (If the `-Wno-non-virtual-dtor` is still wanted for clang-cl, the two are
not exclusive and both can be appended.)

### Related issues

- #1247 (2024), where the maintainers said Static Hermes would support Windows "only via Clang" — i.e. clang-cl is the intended Windows compiler, and this file's test excludes it.
- The `hermes_update_cxx_flags` / `/EHsc`-strip issue in `cmake/modules/Hermes.cmake`, #2173, is the same mistake in the other place; fixing either alone leaves the tree failing.

### Where this came from

Measured while embedding `static_h` as the second runtime of a native game host on Windows (the repository is private at the time of filing, so no links). The workaround used downstream is a directory-scope `/EHsc` in the *consuming* project, chosen over `target_compile_options(jsi PRIVATE /EHsc)` precisely because the per-target fix covered this file and left four others failing (#2173). **No file under the Hermes checkout was edited.**

Contributor guide

Open the contributing guide

Research direction

Start in API/jsi/jsi/CMakeLists.txt:15-24 and compare the compiler-ID branches with CMake's MSVC variable. Configure the project with clang-cl as described, then run cmake --build build --target jsi. Done means clang-cl receives /EHsc and the jsi target compiles without exceptions-disabled errors.

Written by the indexing model from the issue text.

Assessment

Tech stack
cmake, cpp
Domain
build-system
Issue type
Bug
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
90/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.