Respond to MR comments

Move the new is_pod method to simdb::utils::is_pod, so that it won't be confused with std::is_pod in the event of `using namespace std'. Also move the struct definition to a new file, `simdb/utils/CompatUtils.hpp', so that it is centralized in one place.
sparcians · Jan 17, 2024 · 4626e0d · 4626e0d
1 parent 8291100
commit 4626e0d
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 25 deletions.
diff --git a/sparta/simdb/include/simdb/schema/ColumnMetaStructs.hpp b/sparta/simdb/include/simdb/schema/ColumnMetaStructs.hpp
@@ -11,6 +11,7 @@
 #include "simdb/schema/ColumnTypedefs.hpp"
 #include "simdb/schema/DatabaseTypedefs.hpp"
 #include "simdb/Errors.hpp"
+#include "simdb/utils/CompatUtils.hpp"
 
 #include <string>
 #include <type_traits>
@@ -23,20 +24,14 @@ namespace simdb {
 template <typename ColumnT, typename Enable = void>
 struct column_info;
 
-template<typename T>
-struct is_pod {
-  static constexpr bool value = std::is_trivial<T>::value
-                             && std::is_standard_layout<T>::value;
-};
-
 //! int8_t
 template <>
 struct column_info<int8_t> {
     static ColumnDataType data_type() {
         return ColumnDataType::int8_t;
     }
     using value_type = int8_t;
-    static constexpr bool is_fixed_size = is_pod<int8_t>::value;
+    static constexpr bool is_fixed_size = utils::is_pod<int8_t>::value;
 };
 
 //! uint8_t
@@ -46,7 +41,7 @@ struct column_info<uint8_t> {
         return ColumnDataType::uint8_t;
     }
     using value_type = uint8_t;
-    static constexpr bool is_fixed_size = is_pod<uint8_t>::value;
+    static constexpr bool is_fixed_size = utils::is_pod<uint8_t>::value;
 };
 
 //! int16_t
@@ -56,7 +51,7 @@ struct column_info<int16_t> {
         return ColumnDataType::int16_t;
     }
     using value_type = int16_t;
-    static constexpr bool is_fixed_size = is_pod<int16_t>::value;
+    static constexpr bool is_fixed_size = utils::is_pod<int16_t>::value;
 };
 
 //! uint16_t
@@ -66,7 +61,7 @@ struct column_info<uint16_t> {
         return ColumnDataType::uint16_t;
     }
     using value_type = uint16_t;
-    static constexpr bool is_fixed_size = is_pod<uint16_t>::value;
+    static constexpr bool is_fixed_size = utils::is_pod<uint16_t>::value;
 };
 
 //! int32_t
@@ -76,7 +71,7 @@ struct column_info<int32_t> {
         return ColumnDataType::int32_t;
     }
     using value_type = int32_t;
-    static constexpr bool is_fixed_size = is_pod<int32_t>::value;
+    static constexpr bool is_fixed_size = utils::is_pod<int32_t>::value;
 };
 
 //! uint32_t
@@ -86,7 +81,7 @@ struct column_info<uint32_t> {
         return ColumnDataType::uint32_t;
     }
     using value_type = uint32_t;
-    static constexpr bool is_fixed_size = is_pod<uint32_t>::value;
+    static constexpr bool is_fixed_size = utils::is_pod<uint32_t>::value;
 };
 
 //! int64_t
@@ -96,7 +91,7 @@ struct column_info<int64_t> {
         return ColumnDataType::int64_t;
     }
     using value_type = int64_t;
-    static constexpr bool is_fixed_size = is_pod<int64_t>::value;
+    static constexpr bool is_fixed_size = utils::is_pod<int64_t>::value;
 };
 
 //! uint64_t
@@ -106,7 +101,7 @@ struct column_info<uint64_t> {
         return ColumnDataType::uint64_t;
     }
     using value_type = uint64_t;
-    static constexpr bool is_fixed_size = is_pod<uint64_t>::value;
+    static constexpr bool is_fixed_size = utils::is_pod<uint64_t>::value;
 };
 
 //! float
@@ -116,7 +111,7 @@ struct column_info<float> {
         return ColumnDataType::float_t;
     }
     using value_type = float;
-    static constexpr bool is_fixed_size = is_pod<float>::value;
+    static constexpr bool is_fixed_size = utils::is_pod<float>::value;
 };
 
 //! double
@@ -126,7 +121,7 @@ struct column_info<double> {
         return ColumnDataType::double_t;
     }
     using value_type = double;
-    static constexpr bool is_fixed_size = is_pod<double>::value;
+    static constexpr bool is_fixed_size = utils::is_pod<double>::value;
 };
 
 //! string
@@ -139,7 +134,7 @@ struct column_info<ColumnT, typename std::enable_if<
         return ColumnDataType::string_t;
     }
     using value_type = ColumnT;
-    static constexpr bool is_fixed_size = is_pod<std::string>::value;
+    static constexpr bool is_fixed_size = utils::is_pod<std::string>::value;
 };
 
 //! char
@@ -149,7 +144,7 @@ struct column_info<char> {
         return ColumnDataType::char_t;
     }
     using value_type = char;
-    static constexpr bool is_fixed_size = is_pod<char>::value;
+    static constexpr bool is_fixed_size = utils::is_pod<char>::value;
 };
 
 //! Vectors of raw bytes are stored as blobs (void* / opaque)
@@ -161,7 +156,7 @@ struct column_info<ColumnT, typename std::enable_if<
         return ColumnDataType::blob_t;
     }
     using value_type = typename is_container<ColumnT>::value_type;
-    static constexpr bool is_fixed_size = is_pod<ColumnT>::value;
+    static constexpr bool is_fixed_size = utils::is_pod<ColumnT>::value;
 };
 
 //! Blob descriptor
@@ -173,11 +168,11 @@ struct column_info<ColumnT, typename std::enable_if<
         return ColumnDataType::blob_t;
     }
     using value_type = Blob;
-    static constexpr bool is_fixed_size = is_pod<ColumnT>::value;
+    static constexpr bool is_fixed_size = utils::is_pod<ColumnT>::value;
 };
 
 //! See if the given column data type has a fixed number
-//! of bytes, as determined by is_pod<T>
+//! of bytes, as determined by utils::is_pod<T>
 inline bool getColumnIsFixedSize(const ColumnDataType dtype)
 {
     using dt = ColumnDataType;

diff --git a/sparta/simdb/include/simdb/utils/CompatUtils.hpp b/sparta/simdb/include/simdb/utils/CompatUtils.hpp
@@ -0,0 +1,19 @@
+// <CompatUtils> -*- C++ -*-
+
+#pragma once
+
+#include <type_traits>
+
+namespace simdb {
+namespace utils {
+
+//! \brief Replacement for std::is_pod, which was deprecated in C++20
+
+template<typename T>
+struct is_pod {
+    static constexpr bool value = std::is_trivial<T>::value
+                               && std::is_standard_layout<T>::value;
+};
+
+} // namespace utils
+} // namespace simdb
diff --git a/sparta/simdb/src/ObjectRef.cpp b/sparta/simdb/src/ObjectRef.cpp
@@ -4,6 +4,7 @@
 #include "simdb/utils/ObjectQuery.hpp"
 #include "simdb/TableRef.hpp"
 #include "simdb/DbConnProxy.hpp"
+#include "simdb/utils/CompatUtils.hpp"
 
 //SQLite-specific headers
 #include "simdb/impl/sqlite/TransactionUtils.hpp"
@@ -96,8 +97,7 @@ PropertyDataT LOCAL_getScalarProperty(const std::string & table_name,
 //! without using ObjectQuery.
 template <typename PropertyDataT>
 typename std::enable_if<
-    std::is_trivial<PropertyDataT>::value &&
-    std::is_standard_layout<PropertyDataT>::value,
+    utils::is_pod<PropertyDataT>::value,
 PropertyDataT>::type
 LOCAL_getScalarPropertyNoObjectQuery(
     const std::string & table_name,

diff --git a/sparta/sparta/pairs/SpartaKeyPairs.hpp b/sparta/sparta/pairs/SpartaKeyPairs.hpp
@@ -30,6 +30,8 @@
 #include "sparta/utils/MetaTypeList.hpp"
 #include "sparta/utils/MetaStructs.hpp"
 #include "sparta/utils/DetectMemberUtils.hpp"
+#include "simdb/utils/CompatUtils.hpp"
+
 
 namespace sparta {
 
@@ -1691,8 +1693,7 @@ namespace sparta {
         template<typename T>
         MetaStruct::enable_if_t<
             std::is_integral<MetaStruct::decay_t<T>>::value &&
-            std::is_trivial<MetaStruct::decay_t<T>>::value &&
-            std::is_standard_layout<MetaStruct::decay_t<T>>::value &&
+            simdb::utils::is_pod<MetaStruct::decay_t<T>>::value &&
             !MetaStruct::is_bool<MetaStruct::decay_t<T>>::value, void>
 
         updateValueInCache_(