Skip to content

Commit

Permalink
Fix EHa flag compiler filtering
Browse files Browse the repository at this point in the history
The previous attempt to enable Structured Exception Handling in
(only) MSVC using the /EHa compiler flag caused trouble with flags
defined with Cmake Generator Expressions. These expressions are
not fully resolved, and they fail validation checks in
check_cxx_compiler_flag(). The previous attempt at applying
/EHa to only MVSC involved refactoring a direct call to
target_compile_options() with a call to the Slang helper function
add_supported_cxx_flags() where an additional MSVC filter was
introduced, but the helper also calls check_cxx_compiler_flag() to
see if flags are supported. It was okay for /EHa, but not some
other flags that ended up getting newly validated.

The above issue is fixed by re-implementing the change that added
/EHa to only MSVC. This change goes back to adding compiler flags
without the helper function with its extra validation, instead using
an additional cmake generator expression to apply /EHa only to MSVC.
  • Loading branch information
cheneym2 committed Nov 8, 2024
1 parent 64c6cb1 commit 98f8e4a
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 8 deletions.
4 changes: 0 additions & 4 deletions cmake/CompilerFlags.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ function(add_supported_cxx_flags target)
endif()

foreach(flag ${flags})
# /EHa enables SEH on Windows, it is not available in Linux.
if(NOT CMAKE_CXX_COMPILER_ID MATCHES "MSVC")
string(REGEX REPLACE "/EHa" "" flag "${flag}")
endif()
# remove the `no-` prefix from warnings because gcc doesn't treat it as an
# error on its own
string(REGEX REPLACE "\\-Wno\\-(.+)" "-W\\1" flag_to_test "${flag}")
Expand Down
5 changes: 2 additions & 3 deletions cmake/SlangTarget.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -337,10 +337,9 @@ function(slang_add_target dir type)
)
endif()
if(ARG_EXTRA_COMPILE_OPTIONS_PRIVATE)
add_supported_cxx_flags(
target_compile_options(
${target}
PRIVATE
${ARG_EXTRA_COMPILE_OPTIONS_PRIVATE}
PRIVATE ${ARG_EXTRA_COMPILE_OPTIONS_PRIVATE}
)
endif()

Expand Down
2 changes: 1 addition & 1 deletion tools/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ if(SLANG_ENABLE_TESTS)
EXTRA_COMPILE_DEFINITIONS_PRIVATE
$<$<BOOL:${SLANG_ENABLE_CUDA}>:RENDER_TEST_CUDA>
$<$<BOOL:${SLANG_ENABLE_OPTIX}>:RENDER_TEST_OPTIX>
EXTRA_COMPILE_OPTIONS_PRIVATE /EHa
EXTRA_COMPILE_OPTIONS_PRIVATE $<$<CXX_COMPILER_ID:MSVC>:/EHa>
OUTPUT_NAME render-test-tool
REQUIRED_BY slang-test
FOLDER test/tools
Expand Down

0 comments on commit 98f8e4a

Please sign in to comment.