Skip to content

Commit

Permalink
Fix compiler issues again
Browse files Browse the repository at this point in the history
  • Loading branch information
lczech committed Dec 9, 2024
1 parent 2d6b7f1 commit 5736ced
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 39 deletions.
30 changes: 24 additions & 6 deletions .github/workflows/ci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ jobs:

matrix:

# Need dummy entries for the matrix to pick up the variables.
# Add basic entries for the matrix to run the latest versions.
# We later set all other combinations that we want explicitly.
os:
- ubuntu-latest
Expand All @@ -68,7 +68,23 @@ jobs:

# C++ standard
cppstd:
- auto
- auto

# -------------------------------------------------------
# exclude
# -------------------------------------------------------

exclude:

# Currently (as of 2024-12), the latest llvm used here is clang 18,
# which has several bugs concerning deprecated functions, see
# https://github.com/llvm/llvm-project/issues/76515 and
# https://gcc.gnu.org/pipermail/libstdc++/2024-March/058502.html
# which only seem to occur when the environment also has a gcc std lib
# present. That is mostly a setup issue here in the CI, so we do not
# solve this for now, and just deactivate that test case completely.
- os: ubuntu-latest
compiler: llvm

# -------------------------------------------------------
# include
Expand All @@ -87,7 +103,8 @@ jobs:
# the very same version of llvm-9, and got tons of segfaults for all kind of functions.

# We also currently exclude clang from macos, as it fails with different types of
# linker errors. As this is merely a problem in the setup, we defer solving this for now.
# linker errors. As this is merely a problem in the setup in the CI environment,
# we defer solving this for now.

include:

Expand Down Expand Up @@ -449,9 +466,10 @@ jobs:
- os: ubuntu-latest
compiler: gcc
htslib: OFF
- os: ubuntu-latest
compiler: llvm
htslib: OFF
# Deactivate due to bug in llvm 18, see above
# - os: ubuntu-latest
# compiler: llvm
# htslib: OFF
- os: macos-latest
compiler: gcc
htslib: OFF
Expand Down
37 changes: 5 additions & 32 deletions lib/genesis/utils/threading/thread_pool.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -496,9 +496,11 @@ class ThreadPool
// All this wrapping should be completely transparent to the compiler, and removed.
// The task captures the package including the promise that is needed for the future.
WrappedTask wrapped_task;
std::function<result_type()> task_function = std::bind(
std::forward<F>(f), std::forward<Args>(args)...
);
wrapped_task.function = make_wrapped_task_with_promise_(
task_promise,
make_task_function_( std::forward<F>(f), std::forward<Args>(args)... )
task_promise, std::move( task_function )
);

// We first incrementi the unfinished counter, and only decrementing it once the task has
Expand Down Expand Up @@ -531,7 +533,7 @@ class ThreadPool
// All this wrapping should be completely transparent to the compiler, and removed.
// The task captures the package including the promise that is needed for the future.
WrappedTask wrapped_task;
auto task_function = make_task_function_( std::forward<F>(f), std::forward<Args>(args)... );
auto task_function = std::bind( std::forward<F>(f), std::forward<Args>(args)... );
wrapped_task.function = [task_function, this]()
{
// Run the actual work task here. Once done, we can signal this to the unfinished list.
Expand Down Expand Up @@ -698,35 +700,6 @@ class ThreadPool
}
}

template<typename F, typename... Args>
inline auto make_task_function_( F&& f, Args&&... args )
-> std::function<typename genesis_invoke_result<F, Args...>::type ()>
{
// Unfortunately, Clang 18 when compiled under the C++20 standard somehow uses
// std::result_of within std::bind, despite that being deprecated in the standard,
// and hence leads to a warning, and as we set warnings as errors, fails to compile.
// See https://gcc.gnu.org/pipermail/libstdc++/2024-March/058502.html for details.
// This is the reason why we internally use genesis_invoke_result instead, to switch
// between the two. But doesn't work of course for the STL...
// So we need a workaround for this. Silencing via #pragma diagnostic does not seem
// to work, so instead, we completely get rid of the std::bind and use perfect captures.
// This is the whole reason for this function. Super ugly, but it is what it is.
#if GENESIS_CPP_STD >= GENESIS_CPP_STD_20

// Use a modern way to bind the args to the function.
return [f = std::forward<F>(f), ...args = std::forward<Args>(args)]() mutable
{
return f(args...);
};

#else

// Make the function via binding.
return std::bind( std::forward<F>(f), std::forward<Args>(args)... );

#endif
}

template<typename T>
inline std::function<void()> make_wrapped_task_with_promise_(
std::shared_ptr<std::promise<T>>& task_promise,
Expand Down
2 changes: 1 addition & 1 deletion test/src/utils/threading/concurrent_vector_guard.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ TEST( Threading, VectorEntries )
// them from their vec within data, until empty, and then moves to the next one.
auto thread_pool = std::make_shared<ThreadPool>( num_threads );
auto vector_guard = ConcurrentVectorGuard( num_vecs );
auto num_elem = std::atomic<size_t>{0};
std::atomic<size_t> num_elem{0};
for( size_t t = 0; t < num_threads; ++t ) {
thread_pool->enqueue_detached([&, t]()
{
Expand Down

0 comments on commit 5736ced

Please sign in to comment.