From ac89e3ef72844f33b973844fc209d6b2a65878fa Mon Sep 17 00:00:00 2001 From: Julien Jerphanion Date: Mon, 26 Feb 2024 20:33:20 +0100 Subject: [PATCH] build: Use C++20 (#1332) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Julien Jerphanion Co-authored-by: Klaim (Joël Lamotte) <142265+Klaim@users.noreply.github.com> Co-authored-by: Alex Owens <73388657+alexowens90@users.noreply.github.com> --- cpp/CMakeLists.txt | 15 +++-- cpp/arcticdb/entity/atom_key.hpp | 12 +++- cpp/arcticdb/entity/index_range.hpp | 40 +++++++------ cpp/arcticdb/entity/ref_key.hpp | 12 +++- cpp/arcticdb/entity/types-inl.hpp | 11 ++-- cpp/arcticdb/entity/types_proto.hpp | 10 ++-- cpp/arcticdb/storage/failure_simulation.hpp | 30 ++++++---- cpp/arcticdb/storage/library_path.hpp | 58 +++++++++++-------- cpp/arcticdb/storage/lmdb/lmdb_storage.hpp | 7 +++ .../storage/memory/memory_storage.cpp | 2 +- cpp/arcticdb/util/pb_util.hpp | 2 +- cpp/arcticdb/util/preconditions.hpp | 4 +- cpp/arcticdb/util/test/generators.hpp | 4 +- cpp/arcticdb/util/test/test_storage_lock.cpp | 16 +++-- .../folly/dont-inherit-cpp-version.patch | 16 +++++ ...re-uninitialized-local-variable-used.patch | 12 ++++ .../vcpkg_overlays/folly/portfile.cmake | 2 + 17 files changed, 168 insertions(+), 85 deletions(-) create mode 100644 cpp/third_party/vcpkg_overlays/folly/dont-inherit-cpp-version.patch create mode 100644 cpp/third_party/vcpkg_overlays/folly/ignore-uninitialized-local-variable-used.patch diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index 32904a9f88..be04efd6f4 100755 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -1,7 +1,15 @@ cmake_minimum_required(VERSION 3.21) # TARGET_RUNTIME_DLLS -set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/CMake" ${CMAKE_MODULE_PATH}) +# Make the `project` command handle the version of the project. cmake_policy(SET CMP0048 NEW) + +set(CMAKE_CXX_STANDARD 20) +set(CMAKE_CXX_STANDARD_REQUIRED ON) +# We do not need any compilers' extensions, so we disable them. +set(CMAKE_CXX_EXTENSIONS OFF) +set(CMAKE_POSITION_INDEPENDENT_CODE ON) + +set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/CMake" ${CMAKE_MODULE_PATH}) project(arcticdb VERSION 0.0.1) enable_testing() @@ -86,11 +94,6 @@ if(NOT CMAKE_BUILD_TYPE) set(CMAKE_BUILD_TYPE Release) endif() -set(CMAKE_CXX_STANDARD 17) -set(CMAKE_CXX_STANDARD_REQUIRED ON) -# We do not need any compilers' extensions, so we disable them. -set(CMAKE_CXX_EXTENSIONS OFF) -set(CMAKE_POSITION_INDEPENDENT_CODE ON) set(FIND_LIBRARY_USE_LIB64_PATHS ON) include(PythonUtils) # Must be called before Pybind (third_party) to override its finding mechanism diff --git a/cpp/arcticdb/entity/atom_key.hpp b/cpp/arcticdb/entity/atom_key.hpp index ae6ef5c1e0..6bdc5c1402 100644 --- a/cpp/arcticdb/entity/atom_key.hpp +++ b/cpp/arcticdb/entity/atom_key.hpp @@ -109,9 +109,7 @@ class AtomKeyImpl { return *hash_; } - void set_string() const { - str_ = fmt::format("{}", *this); - } + void set_string() const; std::string_view view() const { if(str_.empty()) set_string(); return {str_}; } @@ -259,3 +257,11 @@ struct hash { } }; } + +namespace arcticdb::entity +{ + // This needs to be defined AFTER the formatter for AtomKeyImpl + inline void AtomKeyImpl::set_string() const { + str_ = fmt::format("{}", *this); + } +} diff --git a/cpp/arcticdb/entity/index_range.hpp b/cpp/arcticdb/entity/index_range.hpp index e453dde0f4..3b7accd3e8 100644 --- a/cpp/arcticdb/entity/index_range.hpp +++ b/cpp/arcticdb/entity/index_range.hpp @@ -71,18 +71,7 @@ struct IndexRange { // Indices of non-matching types will always be excluded, might want to assert though // as this should never happen - bool accept(const IndexValue &index) { - if (!specified_) - return true; - - if (index >= start_ && index <= end_) { - ARCTICDB_DEBUG(log::inmem(), "Returning index {} which is in range {}", index, *this); - return true; - } - - ARCTICDB_DEBUG(log::inmem(), "Filtered index {} as it was not in range {}", index, *this); - return false; - } + bool accept(const IndexValue &index); // N.B. Convenience function, variant construction will be too expensive for tight loops friend bool intersects(const IndexRange &left, const IndexRange& right) { @@ -155,28 +144,45 @@ inline IndexRange universal_range(){ return IndexRange{std::numeric_limits -struct formatter { +struct formatter { template constexpr auto parse(ParseContext &ctx) { return ctx.begin(); } template - auto format(const TimestampRange &r, FormatContext &ctx) const { + auto format(const arcticdb::entity::TimestampRange &r, FormatContext &ctx) const { return fmt::format_to(ctx.out(), "{}-{}", r.first, r.second); } }; template<> -struct formatter { +struct formatter { template constexpr auto parse(ParseContext &ctx) { return ctx.begin(); } template - auto format(const IndexRange &r, FormatContext &ctx) const { + auto format(const arcticdb::entity::IndexRange& r, FormatContext& ctx) const { return fmt::format_to(ctx.out(), "{}-{}", r.start_, r.end_); } }; } //namespace fmt + +namespace arcticdb::entity { + +// Note: this needs to be defined after formatters. +inline bool IndexRange::accept(const IndexValue &index) { + if (!specified_) + return true; + + if (index >= start_ && index <= end_) { + ARCTICDB_DEBUG(log::inmem(), "Returning index {} which is in range {}", index, *this); + return true; + } + + ARCTICDB_DEBUG(log::inmem(), "Filtered index {} as it was not in range {}", index, *this); + return false; +} + +} // namespace arcticdb::entity diff --git a/cpp/arcticdb/entity/ref_key.hpp b/cpp/arcticdb/entity/ref_key.hpp index dccfa7735a..59645e1b9b 100644 --- a/cpp/arcticdb/entity/ref_key.hpp +++ b/cpp/arcticdb/entity/ref_key.hpp @@ -57,9 +57,7 @@ namespace arcticdb::entity { std::string_view view() const { if(str_.empty()) set_string(); return std::string_view{str_}; } - void set_string() const { - str_ = fmt::format("{}", *this); - } + void set_string() const; private: StreamId id_; @@ -95,3 +93,11 @@ struct hash { } }; } + +namespace arcticdb::entity +{ + // Note: this needs to be defined after formatters. + inline void RefKey::set_string() const { + str_ = fmt::format("{}", *this); + } +} diff --git a/cpp/arcticdb/entity/types-inl.hpp b/cpp/arcticdb/entity/types-inl.hpp index de4727a7b3..3e7b1e87da 100644 --- a/cpp/arcticdb/entity/types-inl.hpp +++ b/cpp/arcticdb/entity/types-inl.hpp @@ -98,8 +98,8 @@ struct formatter { constexpr auto parse(ParseContext &ctx) { return ctx.begin(); } template - auto format(arcticdb::entity::DataType dt, FormatContext &ctx) const { - return fmt::format_to(ctx.out(), datatype_to_str(dt)); + constexpr auto format(const arcticdb::entity::DataType dt, FormatContext &ctx) const { + return fmt::format_to(ctx.out(), "{}", datatype_to_str(dt)); } }; @@ -109,7 +109,7 @@ struct formatter { constexpr auto parse(ParseContext &ctx) { return ctx.begin(); } template - auto format(arcticdb::entity::Dimension dim, FormatContext &ctx) const { + constexpr auto format(const arcticdb::entity::Dimension dim, FormatContext &ctx) const { return fmt::format_to(ctx.out(), "{}", static_cast(dim)); } }; @@ -120,7 +120,7 @@ struct formatter { constexpr auto parse(ParseContext &ctx) { return ctx.begin(); } template - auto format(const arcticdb::entity::TypeDescriptor &td, FormatContext &ctx) const { + constexpr auto format(const arcticdb::entity::TypeDescriptor &td, FormatContext &ctx) const { return fmt::format_to(ctx.out(), "TD", td.data_type_, td.dimension_); } }; @@ -131,11 +131,10 @@ struct formatter { constexpr auto parse(ParseContext &ctx) { return ctx.begin(); } template - auto format(const arcticdb::entity::StreamId &tsid, FormatContext &ctx) const { + constexpr auto format(const arcticdb::entity::StreamId &tsid, FormatContext &ctx) const { return std::visit([&ctx](auto &&val) { return fmt::format_to(ctx.out(), "{}", val); }, tsid); } }; - } diff --git a/cpp/arcticdb/entity/types_proto.hpp b/cpp/arcticdb/entity/types_proto.hpp index fe993be9a4..ed54518239 100644 --- a/cpp/arcticdb/entity/types_proto.hpp +++ b/cpp/arcticdb/entity/types_proto.hpp @@ -128,7 +128,7 @@ namespace fmt { template auto format(const arcticdb::proto::descriptors::TypeDescriptor& type_desc, FormatContext& ctx) const { auto td = arcticdb::entity::type_desc_from_proto(type_desc); - return format_to(ctx.out(), "{}", td); + return fmt::format_to(ctx.out(), "{}", td); } }; @@ -139,7 +139,7 @@ namespace fmt { template auto format(const arcticdb::proto::descriptors::StreamDescriptor_FieldDescriptor& field_desc, FormatContext& ctx) const { - return format_to(ctx.out(), "{}: {}", field_desc.name(), field_desc.type_desc()); + return fmt::format_to(ctx.out(), "{}: {}", field_desc.name(), field_desc.type_desc()); } }; @@ -150,7 +150,7 @@ namespace fmt { template auto format(const arcticdb::entity::IndexDescriptor& idx, FormatContext& ctx) const { - return format_to(ctx.out(), "IDX", idx.field_count(), static_cast(idx.type())); + return fmt::format_to(ctx.out(), "IDX", idx.field_count(), static_cast(idx.type())); } }; @@ -162,9 +162,9 @@ namespace fmt { template auto format(const arcticdb::entity::Field& fd, FormatContext& ctx) const { if (!fd.name().empty()) - return format_to(ctx.out(), "FD", fd.name(), fd.type()); + return fmt::format_to(ctx.out(), "FD", fd.name(), fd.type()); else - return format_to(ctx.out(), "FD", fd.type()); + return fmt::format_to(ctx.out(), "FD", fd.type()); } }; } diff --git a/cpp/arcticdb/storage/failure_simulation.hpp b/cpp/arcticdb/storage/failure_simulation.hpp index 9172f7baeb..012ae7d88a 100644 --- a/cpp/arcticdb/storage/failure_simulation.hpp +++ b/cpp/arcticdb/storage/failure_simulation.hpp @@ -34,6 +34,24 @@ static const char* failure_names[] = { "DELETE", }; +} + +// Formatters are defined here since they are used in implementations bellow. +namespace fmt { +template<> +struct formatter { + template + constexpr auto parse(ParseContext &ctx) { return ctx.begin(); } + + template + auto format(const arcticdb::FailureType failure_type, FormatContext &ctx) const { + return fmt::format_to(ctx.out(), fmt::runtime(arcticdb::failure_names[int(failure_type)])); + } +}; +} + +namespace arcticdb { + /** Function holder with a description. */ struct FailureAction { using Description = std::variant; @@ -173,15 +191,3 @@ class StorageFailureSimulator { } //namespace arcticdb -namespace fmt { -template<> -struct formatter { - template - constexpr auto parse(ParseContext &ctx) { return ctx.begin(); } - - template - auto format(arcticdb::FailureType failure_type, FormatContext &ctx) const { - return fmt::format_to(ctx.out(), arcticdb::failure_names[int(failure_type)]); - } -}; -} diff --git a/cpp/arcticdb/storage/library_path.hpp b/cpp/arcticdb/storage/library_path.hpp index 2058f94e3d..73a880d2e0 100644 --- a/cpp/arcticdb/storage/library_path.hpp +++ b/cpp/arcticdb/storage/library_path.hpp @@ -55,6 +55,38 @@ inline bool operator==(const DefaultStringViewable &l, const DefaultStringViewab || (l.hash() == r.hash() && std::string_view{l} == std::string_view{r}); } +} + +// Formatters are defined here since they are used in implementations bellow. +namespace fmt { + +template<> +struct formatter { + template + constexpr auto parse(ParseContext &ctx) { return ctx.begin(); } + + template + auto format(const arcticdb::storage::DefaultStringViewable &dsv, FormatContext &ctx) const { + return fmt::format_to(ctx.out(), "{}", std::string_view{dsv}); + } +}; + +} + +namespace std { + +template<> +struct hash { + + inline arcticdb::HashedValue operator()(const arcticdb::storage::DefaultStringViewable &v) const noexcept { + return v.hash(); + } +}; + +} + +namespace arcticdb::storage { + template class LibraryPathImpl { static constexpr std::uint8_t NUM_LIBRARY_PARTS = 3; @@ -135,30 +167,18 @@ using LibraryPath = LibraryPathImpl; } //namespace arcticdb::storage -namespace fmt { - -using namespace arcticdb::storage; - -template<> -struct formatter { - template - constexpr auto parse(ParseContext &ctx) { return ctx.begin(); } - template - auto format(const DefaultStringViewable &dsv, FormatContext &ctx) const { - return fmt::format_to(ctx.out(), "{}", std::string_view{dsv}); - } -}; +namespace fmt { template<> -struct formatter { +struct formatter { template constexpr auto parse(ParseContext &ctx) { return ctx.begin(); } template - auto format(const LibraryPath &lib, FormatContext &ctx) const { + auto format(const arcticdb::storage::LibraryPath &lib, FormatContext &ctx) const { auto out = ctx.out(); fmt::format_to(out, "{}", lib.to_delim_path()); @@ -169,17 +189,9 @@ struct formatter { } namespace std { -template<> -struct hash { - - inline arcticdb::HashedValue operator()(const arcticdb::storage::DefaultStringViewable &v) const noexcept { - return v.hash(); - } -}; template struct hash> { - inline arcticdb::HashedValue operator()(const arcticdb::storage::LibraryPathImpl &v) const noexcept { return v.hash(); } diff --git a/cpp/arcticdb/storage/lmdb/lmdb_storage.hpp b/cpp/arcticdb/storage/lmdb/lmdb_storage.hpp index 02313f903a..845cb15287 100644 --- a/cpp/arcticdb/storage/lmdb/lmdb_storage.hpp +++ b/cpp/arcticdb/storage/lmdb/lmdb_storage.hpp @@ -15,11 +15,18 @@ #include #include +// LMDB++ is using `std::is_pod` in `lmdb++.h`, which is deprecated as of C++20. +// See: https://github.com/drycpp/lmdbxx/blob/0b43ca87d8cfabba392dfe884eb1edb83874de02/lmdb%2B%2B.h#L1068 +// See: https://en.cppreference.com/w/cpp/types/is_pod +// This suppresses the warning. +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wdeprecated-declarations" #ifdef ARCTICDB_USING_CONDA #include #else #include #endif +#pragma GCC diagnostic pop #include diff --git a/cpp/arcticdb/storage/memory/memory_storage.cpp b/cpp/arcticdb/storage/memory/memory_storage.cpp index b80fed3ba9..bf11171930 100644 --- a/cpp/arcticdb/storage/memory/memory_storage.cpp +++ b/cpp/arcticdb/storage/memory/memory_storage.cpp @@ -110,7 +110,7 @@ namespace arcticdb::storage::memory { auto it = key_vec.find(k); if(it != key_vec.end()) { - ARCTICDB_DEBUG(log::storage(), "Read key {}: {}, with {} bytes of data", variant_key_type(k), variant_key_view(k)); + ARCTICDB_DEBUG(log::storage(), "Removed key {}: {}", variant_key_type(k), variant_key_view(k)); key_vec.erase(it); } else if (!opts.ignores_missing_key_) { throw KeyNotFoundException(std::move(k)); diff --git a/cpp/arcticdb/util/pb_util.hpp b/cpp/arcticdb/util/pb_util.hpp index 120743ea1c..307e0d00cb 100644 --- a/cpp/arcticdb/util/pb_util.hpp +++ b/cpp/arcticdb/util/pb_util.hpp @@ -22,7 +22,7 @@ namespace arcticdb::util { template [[noreturn]] void raise_error_msg(const char *pattern, const Msg &msg) { // google::protobuf::TextFormat::PrintToString(msg, &s); - throw ExcType(fmt::format(pattern, msg.DebugString())); + throw ExcType(fmt::format(fmt::runtime(pattern), msg.DebugString())); } namespace { diff --git a/cpp/arcticdb/util/preconditions.hpp b/cpp/arcticdb/util/preconditions.hpp index f15d52ad83..ad3d28996b 100644 --- a/cpp/arcticdb/util/preconditions.hpp +++ b/cpp/arcticdb/util/preconditions.hpp @@ -24,7 +24,7 @@ struct Raise { template [[noreturn]] void operator()(fmt::format_string format, Args&&...args) const { std::string combo_format = fmt::format(FMT_COMPILE("{} {}"), error_code_data.name_, format); - std::string msg = fmt::format(combo_format, std::forward(args)...); + std::string msg = fmt::format(fmt::runtime(combo_format), std::forward(args)...); if constexpr(error_category == ErrorCategory::INTERNAL) log::root().error(msg); throw_error(msg); @@ -33,7 +33,7 @@ struct Raise { template [[noreturn]] void operator()(FormatString format, Args&&...args) const { std::string combo_format = fmt::format(FMT_COMPILE("{} {}"), error_code_data.name_, format); - std::string msg = fmt::format(combo_format, std::forward(args)...); + std::string msg = fmt::format(fmt::runtime(combo_format), std::forward(args)...); if constexpr(error_category == ErrorCategory::INTERNAL) log::root().error(msg); throw_error(msg); diff --git a/cpp/arcticdb/util/test/generators.hpp b/cpp/arcticdb/util/test/generators.hpp index 223af624fe..d313d548aa 100644 --- a/cpp/arcticdb/util/test/generators.hpp +++ b/cpp/arcticdb/util/test/generators.hpp @@ -54,7 +54,7 @@ struct SinkWrapperImpl { SchemaPolicy{ index_.create_stream_descriptor(std::move(stream_id), fields), index_ }, - [=]( + [this]( SegmentInMemory &&mem ) { sink_->segments_.push_back(std::move(mem)); @@ -383,7 +383,7 @@ struct SegmentSinkWrapperImpl { SchemaPolicy{ index.create_stream_descriptor(std::move(stream_id), fields_from_range(fields)), index }, - [=](SegmentInMemory&& mem) { + [this](SegmentInMemory&& mem) { sink_->segments_.push_back(std::move(mem)); }, typename AggregatorType::SegmentingPolicyType{} diff --git a/cpp/arcticdb/util/test/test_storage_lock.cpp b/cpp/arcticdb/util/test/test_storage_lock.cpp index baef9ba281..bd0d9b95d6 100644 --- a/cpp/arcticdb/util/test/test_storage_lock.cpp +++ b/cpp/arcticdb/util/test/test_storage_lock.cpp @@ -83,7 +83,9 @@ struct OptimisticLockTask { data_->contended_ = true; } else { - ++data_->vol_; + // As of C++20, '++' expression of 'volatile'-qualified type is deprecated. + const uint64_t vol_ = data_->vol_ + 1; + data_->vol_ = vol_; ++data_->atomic_; lock.unlock(data_->store_); } @@ -129,7 +131,9 @@ struct PessimisticLockTask { else lock.lock(data_->store_); - ++data_->vol_; + // As of C++20, '++' expression of 'volatile'-qualified type is deprecated. + const uint64_t vol_ = data_->vol_ + 1; + data_->vol_ = vol_; ++data_->atomic_; lock.unlock(data_->store_); } @@ -156,7 +160,9 @@ struct ForceReleaseLockTask { try { lock.lock_timeout(data_->store_, timeout_ms_); - ++data_->vol_; + // As of C++20, '++' expression of 'volatile'-qualified type is deprecated. + const uint64_t vol_ = data_->vol_ + 1; + data_->vol_ = vol_; ++data_->atomic_; // Dont unlock } @@ -195,7 +201,9 @@ struct OptimisticForceReleaseLockTask { data_->timedout_ = true; data_->contended_ = true; } else { - ++data_->vol_; + // As of C++20, '++' expression of 'volatile'-qualified type is deprecated. + const uint64_t vol_ = data_->vol_ + 1; + data_->vol_ = vol_; ++data_->atomic_; // Dont unlock } diff --git a/cpp/third_party/vcpkg_overlays/folly/dont-inherit-cpp-version.patch b/cpp/third_party/vcpkg_overlays/folly/dont-inherit-cpp-version.patch new file mode 100644 index 0000000000..11a8fa3414 --- /dev/null +++ b/cpp/third_party/vcpkg_overlays/folly/dont-inherit-cpp-version.patch @@ -0,0 +1,16 @@ +diff --git a/CMake/FollyCompilerMSVC.cmake b/CMake/FollyCompilerMSVC.cmake +index ad03039aa..42fd6fc22 100644 +--- a/CMake/FollyCompilerMSVC.cmake ++++ b/CMake/FollyCompilerMSVC.cmake +@@ -118,10 +118,10 @@ function(apply_folly_compile_options_to_target THETARGET) + /Zc:threadSafeInit # Enable thread-safe function-local statics initialization. + /Zc:throwingNew # Assume operator new throws on failure. + ++ PRIVATE + /permissive- # Be mean, don't allow bad non-standard stuff (C++/CLI, __declspec, etc. are all left intact). + /std:${MSVC_LANGUAGE_VERSION} # Build in the requested version of C++ + +- PRIVATE + /bigobj # Support objects with > 65k sections. Needed due to templates. + /favor:${MSVC_FAVORED_ARCHITECTURE} # Architecture to prefer when generating code. + /Zc:inline # Have the compiler eliminate unreferenced COMDAT functions and data before emitting the object file. diff --git a/cpp/third_party/vcpkg_overlays/folly/ignore-uninitialized-local-variable-used.patch b/cpp/third_party/vcpkg_overlays/folly/ignore-uninitialized-local-variable-used.patch new file mode 100644 index 0000000000..dec03bd6ab --- /dev/null +++ b/cpp/third_party/vcpkg_overlays/folly/ignore-uninitialized-local-variable-used.patch @@ -0,0 +1,12 @@ +diff --git i/CMake/FollyCompilerMSVC.cmake w/CMake/FollyCompilerMSVC.cmake +index ad03039aa..349fa2bbf 100644 +--- i/CMake/FollyCompilerMSVC.cmake ++++ w/CMake/FollyCompilerMSVC.cmake +@@ -213,6 +213,7 @@ function(apply_folly_compile_options_to_target THETARGET) + /wd4100 # Unreferenced formal parameter. + /wd4459 # Declaration of parameter hides global declaration. + /wd4505 # Unreferenced local function has been removed. ++ /wd4700 # Uninitialized local variable used. + /wd4701 # Potentially uninitialized local variable used. + /wd4702 # Unreachable code. + diff --git a/cpp/third_party/vcpkg_overlays/folly/portfile.cmake b/cpp/third_party/vcpkg_overlays/folly/portfile.cmake index 345eabd3f0..ad7c9222eb 100644 --- a/cpp/third_party/vcpkg_overlays/folly/portfile.cmake +++ b/cpp/third_party/vcpkg_overlays/folly/portfile.cmake @@ -18,6 +18,8 @@ vcpkg_from_github( boost-1.70.patch fix-windows-minmax.patch fix-deps.patch + dont-inherit-cpp-version.patch + ignore-uninitialized-local-variable-used.patch ) file(REMOVE "${SOURCE_PATH}/CMake/FindFmt.cmake")