Skip to content

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

Description

@altween

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

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

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:

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

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.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions