build: Use C++20 (#1332)

Signed-off-by: Julien Jerphanion <[email protected]> Co-authored-by: Klaim (Joël Lamotte) <[email protected]> Co-authored-by: Alex Owens <[email protected]>
man-group · Feb 26, 2024 · ac89e3e · ac89e3e
1 parent 4d20f00
commit ac89e3e
Show file tree

Hide file tree

Showing 17 changed files with 168 additions and 85 deletions.
diff --git 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
@@ -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<arcticdb::entity::AtomKeyImpl> {
     }
 };
 }
+
+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
@@ -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<times
 } //namespace arcticdb::entity
 
 namespace fmt {
-using namespace arcticdb::entity;
 
 template<>
-struct formatter<TimestampRange> {
+struct formatter<arcticdb::entity::TimestampRange> {
     template<typename ParseContext>
     constexpr auto parse(ParseContext &ctx) { return ctx.begin(); }
 
     template<typename FormatContext>
-    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<IndexRange> {
+struct formatter<arcticdb::entity::IndexRange> {
     template<typename ParseContext>
     constexpr auto parse(ParseContext &ctx) { return ctx.begin(); }
 
     template<typename FormatContext>
-    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
@@ -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<arcticdb::entity::RefKey> {
     }
 };
 }
+
+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
@@ -98,8 +98,8 @@ struct formatter<arcticdb::entity::DataType> {
     constexpr auto parse(ParseContext &ctx) { return ctx.begin(); }
 
     template<typename FormatContext>
-    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<arcticdb::entity::Dimension> {
     constexpr auto parse(ParseContext &ctx) { return ctx.begin(); }
 
     template<typename FormatContext>
-    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<uint32_t >(dim));
     }
 };
@@ -120,7 +120,7 @@ struct formatter<arcticdb::entity::TypeDescriptor> {
     constexpr auto parse(ParseContext &ctx) { return ctx.begin(); }
 
     template<typename FormatContext>
-    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<type={}, dim={}>", td.data_type_, td.dimension_);
     }
 };
@@ -131,11 +131,10 @@ struct formatter<arcticdb::entity::StreamId> {
     constexpr auto parse(ParseContext &ctx) { return ctx.begin(); }
 
     template<typename FormatContext>
-    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
@@ -128,7 +128,7 @@ namespace fmt {
         template<typename FormatContext>
         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<typename FormatContext>
         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<typename FormatContext>
         auto format(const arcticdb::entity::IndexDescriptor& idx, FormatContext& ctx) const {
-            return format_to(ctx.out(), "IDX<size={}, kind={}>", idx.field_count(), static_cast<char>(idx.type()));
+            return fmt::format_to(ctx.out(), "IDX<size={}, kind={}>", idx.field_count(), static_cast<char>(idx.type()));
         }
     };
 
@@ -162,9 +162,9 @@ namespace fmt {
         template<typename FormatContext>
         auto format(const arcticdb::entity::Field& fd, FormatContext& ctx) const {
             if (!fd.name().empty())
-                return format_to(ctx.out(), "FD<name={}, type={}>", fd.name(), fd.type());
+                return fmt::format_to(ctx.out(), "FD<name={}, type={}>", fd.name(), fd.type());
             else
-                return format_to(ctx.out(), "FD<type={}>", fd.type());
+                return fmt::format_to(ctx.out(), "FD<type={}>", fd.type());
         }
     };
 }
diff --git 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<arcticdb::FailureType> {
+    template<typename ParseContext>
+    constexpr auto parse(ParseContext &ctx) { return ctx.begin(); }
+
+    template<typename FormatContext>
+    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<const char*, std::string>;
@@ -173,15 +191,3 @@ class StorageFailureSimulator {
 
 } //namespace arcticdb
 
-namespace fmt {
-template<>
-struct formatter<arcticdb::FailureType> {
-    template<typename ParseContext>
-    constexpr auto parse(ParseContext &ctx) { return ctx.begin(); }
-
-    template<typename FormatContext>
-    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
@@ -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<arcticdb::storage::DefaultStringViewable> {
+    template<typename ParseContext>
+    constexpr auto parse(ParseContext &ctx) { return ctx.begin(); }
+
+    template<typename FormatContext>
+    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<arcticdb::storage::DefaultStringViewable> {
+
+    inline arcticdb::HashedValue operator()(const arcticdb::storage::DefaultStringViewable &v) const noexcept {
+        return v.hash();
+    }
+};
+
+}
+
+namespace arcticdb::storage {
+
 template<class StringViewable=DefaultStringViewable>
 class LibraryPathImpl {
     static constexpr std::uint8_t NUM_LIBRARY_PARTS = 3;
@@ -135,30 +167,18 @@ using LibraryPath = LibraryPathImpl<DefaultStringViewable>;
 
 } //namespace arcticdb::storage
 
-namespace fmt {
-
-using namespace arcticdb::storage;
-
-template<>
-struct formatter<DefaultStringViewable> {
-    template<typename ParseContext>
-    constexpr auto parse(ParseContext &ctx) { return ctx.begin(); }
 
-    template<typename FormatContext>
-    auto format(const DefaultStringViewable &dsv, FormatContext &ctx) const {
-        return fmt::format_to(ctx.out(), "{}", std::string_view{dsv});
-    }
-};
+namespace fmt {
 
 template<>
-struct formatter<LibraryPath> {
+struct formatter<arcticdb::storage::LibraryPath> {
     template<typename ParseContext>
     constexpr auto parse(ParseContext &ctx) {
         return ctx.begin();
     }
 
     template<typename FormatContext>
-    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<LibraryPath> {
 }
 
 namespace std {
-template<>
-struct hash<arcticdb::storage::DefaultStringViewable> {
-
-    inline arcticdb::HashedValue operator()(const arcticdb::storage::DefaultStringViewable &v) const noexcept {
-        return v.hash();
-    }
-};
 
 template<class StringViewable>
 struct hash<arcticdb::storage::LibraryPathImpl<StringViewable>> {
-
     inline arcticdb::HashedValue operator()(const arcticdb::storage::LibraryPathImpl<StringViewable> &v) const noexcept {
         return v.hash();
     }

diff --git a/cpp/arcticdb/storage/lmdb/lmdb_storage.hpp b/cpp/arcticdb/storage/lmdb/lmdb_storage.hpp
@@ -15,11 +15,18 @@
 #include <folly/Range.h>
 #include <arcticdb/util/composite.hpp>
 
+// 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 <lmdb++.h>
 #else
     #include <third_party/lmdbxx/lmdb++.h>
 #endif
+#pragma GCC diagnostic pop
 
 #include <filesystem>
 

diff --git 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
@@ -22,7 +22,7 @@ namespace arcticdb::util {
 template<class Msg, class ExcType=std::invalid_argument>
 [[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 {