From a45bcdb949dd1f3f1450eb9c744fdf93a9dcda98 Mon Sep 17 00:00:00 2001 From: Lucas Czech Date: Tue, 17 Sep 2024 19:42:03 +0200 Subject: [PATCH] Fix more compatibility issues with C++17 and later --- .../utils/containers/function_cache.hpp | 12 +++-- .../containers/interval_tree/interval.hpp | 14 +++++- .../utils/containers/transform_iterator.hpp | 13 +++--- lib/genesis/utils/core/std.hpp | 45 +++++++++++++++++++ lib/genesis/utils/io/parser.cpp | 22 ++++----- .../utils/threading/thread_functions.hpp | 4 +- lib/genesis/utils/threading/thread_pool.hpp | 7 ++- tools/cmake/DetectCppVersion.cmake | 13 ++++-- 8 files changed, 99 insertions(+), 31 deletions(-) diff --git a/lib/genesis/utils/containers/function_cache.hpp b/lib/genesis/utils/containers/function_cache.hpp index ec0ba8d9..7a63049e 100644 --- a/lib/genesis/utils/containers/function_cache.hpp +++ b/lib/genesis/utils/containers/function_cache.hpp @@ -3,7 +3,7 @@ /* Genesis - A toolkit for working with phylogenetic data. - Copyright (C) 2014-2021 Lucas Czech + Copyright (C) 2014-2024 Lucas Czech This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -259,7 +259,11 @@ class FunctionCache R const& operator()( A const&... arguments ) { // Build the key. Can be done without mutex locking. - std::tuple const key( arguments... ); + // Update for C++23: We need to build the tuple in a different way, + // due to the more stringent rules regarding parameter pack expansion. + // std::tuple const key( arguments... ); + // auto const key = std::make_tuple( arguments... ); + auto const key = std::forward_as_tuple( arguments... ); // Get the shard that this key belongs to. assert( shard_index_( key ) < shards_.size() ); @@ -293,8 +297,8 @@ class FunctionCache bool contains( A const&... arguments ) const { // Simply follow the steps of the above operator(), but only check the presence of the key, - // without computing the function. - std::tuple const key( arguments... ); + // without computing the function. Same update for the key creating here for C++23. + auto const key = std::forward_as_tuple( arguments... ); auto& shard = *shards_[ shard_index_( key ) ]; std::lock_guard lock( shard.guard_ ); auto search = shard.cache_.find( key ); diff --git a/lib/genesis/utils/containers/interval_tree/interval.hpp b/lib/genesis/utils/containers/interval_tree/interval.hpp index 723089e3..02f056bf 100644 --- a/lib/genesis/utils/containers/interval_tree/interval.hpp +++ b/lib/genesis/utils/containers/interval_tree/interval.hpp @@ -3,7 +3,7 @@ /* Genesis - A toolkit for working with phylogenetic data. - Copyright (C) 2014-2022 Lucas Czech + Copyright (C) 2014-2024 Lucas Czech This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -253,6 +253,13 @@ struct IntervalClosed // Interval // ================================================================================================= +// We have issues with GCC 7 due to stupid signed integer overflow warnings. +// They do not make sense, and are indeed an overly cautious compiler, so we just deactive them. +#ifdef __GNUC__ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wstrict-overflow" +#endif + /** * @brief Type to store an interval (range) between two numbers, as used in the IntervalTree. * @@ -524,6 +531,11 @@ struct Interval data_type data_; }; +// Reset the GCC compiler warning levels. +#ifdef __GNUC__ +#pragma GCC diagnostic pop +#endif + } // namespace utils } // namespace genesis diff --git a/lib/genesis/utils/containers/transform_iterator.hpp b/lib/genesis/utils/containers/transform_iterator.hpp index a677b878..a08705de 100644 --- a/lib/genesis/utils/containers/transform_iterator.hpp +++ b/lib/genesis/utils/containers/transform_iterator.hpp @@ -3,7 +3,7 @@ /* Genesis - A toolkit for working with phylogenetic data. - Copyright (C) 2014-2020 Lucas Czech + Copyright (C) 2014-2024 Lucas Czech This program is free software: you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -19,9 +19,9 @@ along with this program. If not, see . Contact: - Lucas Czech - Exelixis Lab, Heidelberg Institute for Theoretical Studies - Schloss-Wolfsbrunnenweg 35, D-69118 Heidelberg, Germany + Lucas Czech + Department of Plant Biology, Carnegie Institution For Science + 260 Panama Street, Stanford, CA 94305, USA */ /** @@ -32,6 +32,7 @@ */ #include "genesis/utils/containers/range.hpp" +#include "genesis/utils/core/std.hpp" #include #include @@ -110,8 +111,8 @@ class TransformIterator // ------------------------------------------------------------------------- // Get the type that the TransformFunctor returns - using result_type = typename std::result_of< - TransformFunctor( typename std::iterator_traits::reference ) + using result_type = typename genesis_invoke_result< + TransformFunctor, typename std::iterator_traits::reference >::type; // We want to cache the transformed values if the transform functor returns a valy by copy diff --git a/lib/genesis/utils/core/std.hpp b/lib/genesis/utils/core/std.hpp index 9322536b..27faade2 100644 --- a/lib/genesis/utils/core/std.hpp +++ b/lib/genesis/utils/core/std.hpp @@ -35,6 +35,8 @@ #include #include #include +#include +#include namespace genesis { namespace utils { @@ -107,6 +109,49 @@ class ArrowOperatorProxy { T t; }; +// ================================================================================================= +// Helpers for portability between C++ 11 and later +// ================================================================================================= + +/** + * @brief Helper to abstract from `std::invoke_result` and `std::result_of` + * for different C++ standards. + * + * This simply switches between the two, depending on the C++ standard being compiled with. + * This is necessary as in later versions, `std::result_of` is deprecated and removed. + * Usage similar to `std::invoke_result`, such as + * + * typename genesis_invoke_result::type + * + * to obtain the resulting type of a call of function `F` with arugments `Args`. + */ +template +struct genesis_invoke_result +{ + #if ((defined(_MSVC_LANG) && _MSVC_LANG >= 201703L) || __cplusplus >= 201703L) + // C++17 or later + using type = typename std::invoke_result::type; + #else + // C++11 and C++14. Tricky to get right. + + // First attempt, simply forwarding. + // using type = typename std::result_of::type; + + // Using decltype to get the correct return type for callable objects, including lambdas + // using type = decltype( std::declval()( std::declval()... )); + + // Use decltype to deduce the result type, ensuring F is callable with Args... + // using type = typename std::enable_if< + // std::is_function::type>::value || + // std::is_function::value, + // decltype(std::declval()(std::declval()...)) + // >::type; + + // Refined version with decay for lambdas. + using type = typename std::result_of::type(Args...)>::type; + #endif +}; + // ================================================================================================= // Hash Helpers // ================================================================================================= diff --git a/lib/genesis/utils/io/parser.cpp b/lib/genesis/utils/io/parser.cpp index c40b5859..ea6cfcd5 100644 --- a/lib/genesis/utils/io/parser.cpp +++ b/lib/genesis/utils/io/parser.cpp @@ -40,15 +40,17 @@ #include // For C++17, we have a little speedup in the integer parsing part. -// Not avaiable on GCC < 8 though, so we need an additional check here... +// Not avaiable on GCC < 8 though, so we need an additional check here. +// Also not working with Clang 6-7, see https://bugzilla.redhat.com/show_bug.cgi?id=1657544 +// Basicaly, the std::from_chars function seems cursed in early implementations of C++17... #if ((defined(_MSVC_LANG) && _MSVC_LANG >= 201703L) || __cplusplus >= 201703L) #if defined( __has_include ) && __has_include() - #define GENESIS_COMPILER_HAS_CHARCONV +#if !defined(__clang__) || (__clang_major__ > 7) + // Only if all the above conditions are met do we activate our implementation. + #define GENESIS_COMPILER_HAS_STD_FROM_CHARS + #include #endif #endif - -#ifdef GENESIS_COMPILER_HAS_CHARCONV - #include #endif namespace genesis { @@ -329,7 +331,7 @@ size_t parse_unsigned_integer_from_chars_( utils::InputStream& source ) // ------------------------------------------------------------------------- // Only use C++17 code if we are compiled with that version and charconv support. -#ifdef GENESIS_COMPILER_HAS_CHARCONV +#ifdef GENESIS_COMPILER_HAS_STD_FROM_CHARS /** * @brief Another speedup technique using std::from_chars(), @@ -351,10 +353,8 @@ size_t parse_unsigned_integer_std_from_chars_( utils::InputStream& source ) using namespace utils; T x = 0; - // Fastest method accoing to + // Fastest method is from_chars(), so let's us it. See: // https://www.fluentcpp.com/2018/07/27/how-to-efficiently-convert-a-string-to-an-int-in-c/ - // is from_chars(), so let's us it! - auto const conv = std::from_chars( &buffer[ data_pos ], &buffer[ data_end ], x ); // How many chars did we consume? @@ -394,7 +394,7 @@ size_t parse_unsigned_integer_std_from_chars_( utils::InputStream& source ) return x; } -#endif // GENESIS_COMPILER_HAS_CHARCONV +#endif // GENESIS_COMPILER_HAS_STD_FROM_CHARS // ------------------------------------------------------------------------- // parse_unsigned_integer_naive_ @@ -463,7 +463,7 @@ size_t parse_unsigned_integer_size_t( utils::InputStream& source ) // If we have GCC or Clang, use our own handcrafted fast-as-hell implementation. return parse_unsigned_integer_gcc_intrinsic_( source ); - // #elif GENESIS_COMPILER_HAS_CHARCONV + // #elif GENESIS_COMPILER_HAS_STD_FROM_CHARS // // // Otherwise, if this is C++17, at least use its own fast version, // // that can use some compiler intrinsics itself. diff --git a/lib/genesis/utils/threading/thread_functions.hpp b/lib/genesis/utils/threading/thread_functions.hpp index 728f93a4..02d64029 100644 --- a/lib/genesis/utils/threading/thread_functions.hpp +++ b/lib/genesis/utils/threading/thread_functions.hpp @@ -62,6 +62,7 @@ */ #include "genesis/utils/core/options.hpp" +#include "genesis/utils/core/std.hpp" #include "genesis/utils/threading/multi_future.hpp" #include "genesis/utils/threading/thread_pool.hpp" @@ -138,7 +139,8 @@ namespace utils { template< typename F, typename T1, typename T2, typename T = typename std::common_type::type, - typename R = typename std::result_of::type(T, T)>::type + typename R = typename genesis_invoke_result::type + // typename R = typename std::result_of::type(T, T)>::type > MultiFuture parallel_block( T1 begin, T2 end, F&& body, diff --git a/lib/genesis/utils/threading/thread_pool.hpp b/lib/genesis/utils/threading/thread_pool.hpp index 7218d7e9..9afd0694 100644 --- a/lib/genesis/utils/threading/thread_pool.hpp +++ b/lib/genesis/utils/threading/thread_pool.hpp @@ -31,6 +31,7 @@ * @ingroup utils */ +#include "genesis/utils/core/std.hpp" #include "genesis/utils/threading/blocking_concurrent_queue.hpp" #include @@ -452,11 +453,9 @@ class ThreadPool */ template auto enqueue_and_retrieve( F&& f, Args&&... args ) - -> ProactiveFuture::type> + -> ProactiveFuture::type> { - // Updated for C++17 and later: - // using result_type = typename std::result_of::type; - using result_type = typename std::invoke_result::type; + using result_type = typename genesis_invoke_result::type; // Make sure that we do not enqueue more tasks than the max size. run_tasks_until_below_max_queue_size_(); diff --git a/tools/cmake/DetectCppVersion.cmake b/tools/cmake/DetectCppVersion.cmake index 1b6affe4..974ecce3 100644 --- a/tools/cmake/DetectCppVersion.cmake +++ b/tools/cmake/DetectCppVersion.cmake @@ -42,12 +42,17 @@ set(CPP_VERSIONS_AND_FLAGS # Function to detect the highest available C++ standard function(detect_highest_cpp_version HIGHEST_SUPPORTED_CXX_VERSION) # Create a minimal C++ source code snippet for testing. - # We also include the chrono header here as a super ugly ad-hoc patch for a clang bug, - # which occurs with that header and clang 10 to 14, see - # https://stackoverflow.com/a/63985023/4184258 + # We also include some headers here as a super ugly ad-hoc patch for some bugs. + # The chrono header is broken in clang 10 to 14, see https://stackoverflow.com/a/63985023/4184258 + # The regex header has issues in clang 6 to 8. + # In both cases, compiling with an earlier version of C++ as a fallback seems to work, + # so when including them here, compilation fails for the later versions, which is what we want. file( WRITE "${CMAKE_BINARY_DIR}/test_cpp_version.cpp" - "#include \n#include \nint main() { std::cout << \"C++ version check\" << std::endl; return 0; }\n" + "#include \n \ + #include \n \ + #include \n \ + int main() { std::cout << \"C++ version check\" << std::endl; return 0; }\n" ) # Iterate over the list of C++ versions and their corresponding and check if it is supported