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

Add ability to build with Clang on macOS #500

Open
wants to merge 43 commits into
base: main
Choose a base branch
from

Conversation

Petua41
Copy link
Collaborator

@Petua41 Petua41 commented Dec 2, 2024

Add ability to build with Clang compiler (Homebrew) on macOS:

  • remove GCC-specific code
  • add corresponding tests to CI

@Petua41 Petua41 force-pushed the homebrew-clang-buildability branch from 564d95c to 3896521 Compare December 3, 2024 10:26
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/core/util/auto_join_thread.h Outdated Show resolved Hide resolved
src/core/util/auto_join_thread.h Outdated Show resolved Hide resolved
src/core/util/bitset_extensions.h Outdated Show resolved Hide resolved
src/core/util/bitset_extensions.h Outdated Show resolved Hide resolved
src/core/util/bitset_extensions.h Outdated Show resolved Hide resolved
src/core/util/bitset_extensions.h Outdated Show resolved Hide resolved
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

src/core/util/bitset_extensions.h Outdated Show resolved Hide resolved
@Petua41 Petua41 force-pushed the homebrew-clang-buildability branch from 3b3b750 to 30c4d53 Compare December 7, 2024 08:21
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 25 out of 243. Check the log or trigger a new build to see more.

src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 25 out of 218. Check the log or trigger a new build to see more.

src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
src/core/util/gcc_bitset.h Outdated Show resolved Hide resolved
@Petua41 Petua41 force-pushed the homebrew-clang-buildability branch from cb5b500 to 17f840a Compare December 16, 2024 15:59
Add `set -e` in build.sh to stop build process on error
Add SYSTEM flag to add_subdirectory(".../googletest") in CMakeLists.txt
to supress warnings from the inside of googletest
Replace INSTANTIATE_TEST_CASE_P, which is deprecated, with
INSTANTIATE_TEST_SUITE_P in NDVerifier tests
Mark unused fields as [[maybe_unused]], because clang produces warning
that becomes an error in Debug mode. Use MAYBE_UNUSED macro, because
g   produces warning when [[maybe_unused]] macro is present, and clang --
when it isn't.
Convert double literals to model::DoubleType and model::IntType explicitly, because
otherwise clang produces a warning, that becomes an error in debug mode
Add run_tests_clang action in core_tests.yml
Also, work around clang-18 bug
- Check that column is numeric *before* casting it to INumericType
in DataStats::GetMedianAD
- Don't process empty vector in GetPartition in gfd_validation.cpp
- Disable tests causing float-to-long cast when result is infinity
Add Clang installation guide to README.md
Use simple RAII wrapper of std::thread instead of std::jthread
when the latter isn't implemented (which is the case in Apple Clang)
Use custom implementations of std::bitset::_Find_dirst and
_Find_next when gcc intrinsics aren't availible
Use some tricks with boost::dynamic_bitset in fd_tree_bitset
to avoid GCC intrinsics _Find_first and _Find_next
Use std::chrono::high_resolution_clock instead of
std::chrono::_V2::high_resolution_clock in fastod/util/timer.h,
because symbol versioning is gcc-specific
Make GetByte function contexpr only if std::vector::operator[]
is contexpr (which isn't on old versions of GCC)
Shift bitset 1 bit left in CreateDynamicBitset (fallback variant),
as it done in main (gcc intrinsic) variant. Also, rename this function
for clarity.
Use system_clock everywhere instead of mix of system_clock and
high_resolution_clock in Cords::ExecuteInternal
Add instructions how to build boost with clang
Link clang build with libc   instead of libstc   in CI
Also build boost with clang in CI
Don't violate implicit noexcept(true) in AutoJoinThread's destructor
Version 1.13.0 of googletest produces "narrowing conversion" warning due to a bug
(issue #4206, google/googletest#4206) when building
with Clang. This bug was fixed in 1.14.0 (commit 3044657,
google/googletest@3044657)
Install all LLVM packages in install-clang action in CI,
not only LLVM and Clang
Use concepts to test GCC intrinsics precence, instead of SFINAE
Use stable_sort instead of sort to avoid relying on
unspecified behaviour (wheter sort preserves order of equal elements)
Sort pairs by value if frequencies are equal in Cords'
FrequencyHandler::InitFrequencyHandler
@Petua41 Petua41 force-pushed the homebrew-clang-buildability branch from 17f840a to 95242e0 Compare December 19, 2024 09:20
Use reverse iterators to traverse set in reverse order, instead of
messing with normal iterators
Disable alloc-dealloc-mismatch check as it's false positive
on Ubuntu (see comment in CMakeLists.txt)
Limit UB sanitizer "function" check to only our code when building with
Clang on macOS
Shift bitset 1 bit left in CreateDynamicBitset (fallback variant),
as it done in main (gcc intrinsic) variant. Also, rename this function
for clarity.
@Petua41 Petua41 force-pushed the homebrew-clang-buildability branch from 95242e0 to 37233d3 Compare December 21, 2024 15:34
Build boost insteads on installing it with brew
There's no brach "release-1.14.0" in googletest repo
Install boost into /usr/local on macOS in CI, as /usr/{lib,include,bin,etc.} are read-only
- Add fallback behaviour in hymd::utility::MdLessPairs if
  operator <=> for vectors isn't availible
- Add explicit #include <string> in exceptions.h
Add brew Clang building isntructions to README
@Petua41 Petua41 marked this pull request as ready for review December 22, 2024 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant