Skip to content

Experiments with more compiler flags #3998

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jhale wants to merge 19 commits into
base: main
Choose a base branch
from
+56 −15
Open

Experiments with more compiler flags #3998

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
f2af3d1
Fix.
jhale Nov 20, 2025
8453093
Try enabling C++ stdlib debugging.
jhale Nov 20, 2025
c572797
Does this trigger a runtime assertion anywhere?
jhale Nov 20, 2025
d1fd08d
Try replicating developer flags exactly.
jhale Nov 20, 2025
8657316
Fix
jhale Nov 20, 2025
35fd620
Remove some legacy cxx standard stuff.
jhale Nov 20, 2025
7b12705
Try with just assertions/hardening.
jhale Nov 20, 2025
8423335
Missing Developer flag.
jhale Nov 20, 2025
c1f4871
Fix.
jhale Nov 20, 2025
fc185c7
Actually apply fix.
jhale Nov 20, 2025
113adc2
Fix up syntax.
jhale Nov 20, 2025
ba27d79
Interesting to see performance hit on address sanitising.
jhale Nov 20, 2025
8317669
Merge remote-tracking branch 'origin/main' into jhale/address-sanitize
jhale Nov 20, 2025
9c9f174
Add to tests.
jhale Nov 20, 2025
8d13f9d
Fix.
jhale Nov 20, 2025
23fb87d
Simplify.
jhale Nov 20, 2025
331673e
Fix.
jhale Nov 20, 2025
644efbd
Fix.
jhale Nov 20, 2025
46f971a
Typo
jhale Nov 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion cpp/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,13 +227,29 @@ endif()
# Check for some compiler flags
include(CheckCXXCompilerFlag)

set(DOLFINX_CXX_DEVELOPER_FLAGS "")
set(DOLFINX_CXX_DEVELOPER_DEFINITIONS "")
set(DOLFINX_LINK_DEVELOPER_FLAGS "")

# Add some strict compiler checks
check_cxx_compiler_flag("-Wall -Werror -Wextra -pedantic" HAVE_PEDANTIC)

if(HAVE_PEDANTIC)
list(APPEND DOLFINX_CXX_DEVELOPER_FLAGS -Wall;-Werror;-Wextra;-pedantic)
endif()

# Nicer stack traces
check_cxx_compiler_flag("-fno-omit-frame-pointer" HAVE_NO_OMIT_FRAME_POINTER)
if(HAVE_NO_OMIT_FRAME_POINTER)
list(APPEND DOLFINX_CXX_DEVELOPER_FLAGS -fno-omit-frame-pointer)
endif()

# Add fast memory error detector
check_cxx_compiler_flag("-fsanitize=address" HAVE_ADDRESS_SANITIZER)
if(HAVE_ADDRESS_SANITIZER)
list(APPEND DOLFINX_CXX_DEVELOPER_FLAGS -fsanitize=address)
list(APPEND DOLFINX_LINK_DEVELOPER_FLAGS -fsanitize=address)
endif()

# Debug flags
check_cxx_compiler_flag(-g HAVE_DEBUG)
if(HAVE_DEBUG)
Expand Down
6 changes: 6 additions & 0 deletions cpp/dolfinx/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@ target_compile_definitions(
dolfinx PRIVATE $<$<AND:$<CONFIG:Developer>,$<COMPILE_LANGUAGE:CXX>>:${DOLFINX_CXX_DEVELOPER_DEFINITIONS}>
)

# Set 'Developer' link options
target_link_options(
dolfinx PRIVATE $<$<CONFIG:Developer>:${DOLFINX_LINK_DEVELOPER_FLAGS}>
)


# Add version to definitions (public)
target_compile_definitions(dolfinx PUBLIC DOLFINX_VERSION="${DOLFINX_VERSION}")

Expand Down
47 changes: 33 additions & 14 deletions cpp/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ if (PROJECT_IS_TOP_LEVEL)
include(CTest) # enables testing
find_package(DOLFINX REQUIRED)

set(DOLFINX_CXX_DEVELOPER_FLAGS "")
set(DOLFINX_CXX_DEVELOPER_DEFINITIONS "")
set(DOLFINX_LINK_DEVELOPER_FLAGS "")

# NOTE: The following must precisely replicate the code in ../CMakeLists.txt.
# Check for some compiler flags
include(CheckCXXCompilerFlag)
Expand All @@ -19,6 +23,19 @@ if (PROJECT_IS_TOP_LEVEL)
list(APPEND DOLFINX_CXX_DEVELOPER_FLAGS -Wall;-Werror;-Wextra;-pedantic)
endif()

# Nicer stack traces
check_cxx_compiler_flag("-fno-omit-frame-pointer" HAVE_NO_OMIT_FRAME_POINTER)
if(HAVE_NO_OMIT_FRAME_POINTER)
list(APPEND DOLFINX_CXX_DEVELOPER_FLAGS -fno-omit-frame-pointer)
endif()

# Add fast memory error detector
check_cxx_compiler_flag("-fsanitize=address" HAVE_ADDRESS_SANITIZER)
if(HAVE_ADDRESS_SANITIZER)
list(APPEND DOLFINX_CXX_DEVELOPER_FLAGS -fsanitize=address)
list(APPEND DOLFINX_LINK_DEVELOPER_FLAGS -fsanitize=address)
endif()

Comment on lines +26 to +38
Copy link
Contributor

@schnellerhase schnellerhase Dec 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should think about a nicer way to maintain these centrally and not twice in cpp and cpp/test.

# Debug flags
check_cxx_compiler_flag(-g HAVE_DEBUG)
if(HAVE_DEBUG)
Expand Down Expand Up @@ -121,21 +138,23 @@ if (NOT PROJECT_IS_TOP_LEVEL)
)
endif()

if (DEFINED DOLFINX_CXX_DEVELOPER_FLAGS)
target_compile_options(
unittests
PRIVATE
$<$<AND:$<CONFIG:Developer>,$<COMPILE_LANGUAGE:CXX>>:${DOLFINX_CXX_DEVELOPER_FLAGS}>
)
endif()
target_compile_options(
unittests
PRIVATE
$<$<AND:$<CONFIG:Developer>,$<COMPILE_LANGUAGE:CXX>>:${DOLFINX_CXX_DEVELOPER_FLAGS}>
)

if (DEFINED DOLFINX_CXX_DEVELOPER_DEFINITIONS)
target_compile_definitions(
unittests
PRIVATE
$<$<AND:$<CONFIG:Developer>,$<COMPILE_LANGUAGE:CXX>>:${DOLFINX_CXX_DEVELOPER_DEFINITIONS}>
)
endif()
target_compile_definitions(
unittests
PRIVATE
$<$<AND:$<CONFIG:Developer>,$<COMPILE_LANGUAGE:CXX>>:${DOLFINX_CXX_DEVELOPER_DEFINITIONS}>
)

target_link_options(
unittests
PRIVATE
$<$<CONFIG:Developer>:${DOLFINX_LINK_DEVELOPER_FLAGS}>
)

if(ENABLE_CLANG_TIDY)
find_program(CLANG_TIDY NAMES clang-tidy REQUIRED)
Expand Down
Loading