Skip to content
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.

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

Fix builds on macOS #85

Closed

Conversation

ClausKlein
Copy link
Contributor

@ClausKlein ClausKlein commented Dec 7, 2024

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to keep the toolchain file declarative and split out appleclang into a separate toolchain.

Copy link
Contributor Author

@ClausKlein ClausKlein Dec 8, 2024

Choose a reason for hiding this comment

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

Clang-19 on Windows, Linux, and Darwin my be used with the same preset.
The only differences are in setting sanitiser flags.

Why duplicate 95 % of cmake code for each OS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, we may use an other implementation:

option(ENABLE_ASAN "Enable AddressSanitizer" YES)

if(MSVC)
  if(ENABLE_ASAN)
    string(REPLACE "/RTC1" "" CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG}")
    string(REPLACE "/RTC1" "" CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG}")
    add_compile_options(/fsanitize=address /fsanitize-address-use-after-return)
  endif()
elseif(CMAKE_C_COMPILER_ID MATCHES "GNU|Clang")

  option(ENABLE_LSAN "Enable LeakSanitizer" NO)
  option(ENABLE_TSAN "Enable ThreadSanitizer" NO)
  option(ENABLE_UBSAN "Enable UndefinedBehaviorSanitizer" YES)
  if(NOT APPLE)
    option(ENABLE_MSAN "Enable MemorySanitizer" NO)
  endif()
  if((ENABLE_ASAN AND (ENABLE_TSAN OR ENABLE_MSAN)) OR
     (ENABLE_LSAN AND (ENABLE_TSAN OR ENABLE_MSAN)) OR
     (ENABLE_TSAN AND ENABLE_MSAN))
     message(FATAL_ERROR
       "Invalid sanitizer combination:\n"
       "  ENABLE_ASAN:  ${ENABLE_ASAN}\n"
       "  ENABLE_LSAN:  ${ENABLE_LSAN}\n"
       "  ENABLE_TSAN:  ${ENABLE_TSAN}\n"
       "  ENABLE_MSAN:  ${ENABLE_MSAN}"
     )
  endif()
  add_compile_options(
         -fno-omit-frame-pointer
         $<$<BOOL:${ENABLE_ASAN}>:-fsanitize=address>
         $<$<BOOL:${ENABLE_LSAN}>:-fsanitize=leak>
         $<$<BOOL:${ENABLE_MSAN}>:-fsanitize=memory>
         $<$<BOOL:${ENABLE_TSAN}>:-fsanitize=thread>
         $<$<BOOL:${ENABLE_UBSAN}>:-fsanitize=undefined>
  )
  add_link_options(
         $<$<BOOL:${ENABLE_ASAN}>:-fsanitize=address>
         $<$<BOOL:${ENABLE_LSAN}>:-fsanitize=leak>
         $<$<BOOL:${ENABLE_MSAN}>:-fsanitize=memory>
         $<$<BOOL:${ENABLE_TSAN}>:-fsanitize=thread>
         $<$<BOOL:${ENABLE_UBSAN}>:-fsanitize=undefined>
  )
endif()

Copy link
Member

@neatudarius neatudarius Jan 20, 2025

Choose a reason for hiding this comment

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

I would also want separate files, as @steve-downey suggested

@coveralls
Copy link

Coverage Status

coverage: 91.209%. remained the same
when pulling 9d714ea on ClausKlein:feature/fix-macos-builds
into 5765b9d on bemanproject:main.

Copy link
Member

@neatudarius neatudarius left a comment

Choose a reason for hiding this comment

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

Please address feedback. Thanks for contributions!

@ClausKlein
Copy link
Contributor Author

ClausKlein commented Jan 20, 2025

No, I will not continue this endless waiting for constructive working.

@ClausKlein ClausKlein closed this Jan 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants