Move static asserts in separate classes to enable default move constr…

…uctors Summary: # Context This is a follow-up of D39355689. For short, having explicit destructors disables the generation of move constructors, which means we might be making copies when calling `std::move(x)` on those instances. # Solution Again, see D39355689. We can move the static asserts in a destructor of an empty class, with the main class inheriting from it. Reviewed By: agampe, arnaudvenet Differential Revision: D51585268 fbshipit-source-id: d0d1f54c8086a49811f0a164fdcb20b88132b9d2
facebook · Nov 28, 2023 · 0979bdc · 0979bdc
1 parent 8e291da
commit 0979bdc
Show file tree

Hide file tree

Showing 7 changed files with 194 additions and 78 deletions.
diff --git a/include/sparta/AbstractEnvironment.h b/include/sparta/AbstractEnvironment.h
@@ -21,6 +21,9 @@ namespace sparta {
 
 namespace environment_impl {
 
+template <typename Map>
+class AbstractEnvironmentStaticAssert;
+
 template <typename Map>
 class MapValue;
 
@@ -60,24 +63,13 @@ class value_is_bottom {};
 template <typename Map>
 class AbstractEnvironment final
     : public AbstractDomainScaffolding<environment_impl::MapValue<Map>,
-                                       AbstractEnvironment<Map>> {
+                                       AbstractEnvironment<Map>>,
+      private environment_impl::AbstractEnvironmentStaticAssert<Map> {
  public:
   using Variable = typename Map::key_type;
   using Domain = typename Map::mapped_type;
   using MapType = Map;
 
-  ~AbstractEnvironment() {
-    static_assert(std::is_base_of<AbstractMap<Map>, Map>::value,
-                  "Map doesn't inherit from AbstractMap");
-
-    using ValueInterface = typename Map::value_interface;
-    static_assert(std::is_base_of<AbstractMapValue<ValueInterface>,
-                                  ValueInterface>::value,
-                  "ValueInterface doesn't inherit from AbstractMapValue");
-    static_assert(ValueInterface::default_value_kind == AbstractValueKind::Top,
-                  "ValueInterface::default_value_kind is not Top");
-  }
-
   /*
    * The default constructor produces the Top value.
    */
@@ -239,6 +231,22 @@ class AbstractEnvironment final
 
 namespace environment_impl {
 
+template <typename Map>
+class AbstractEnvironmentStaticAssert {
+ protected:
+  ~AbstractEnvironmentStaticAssert() {
+    static_assert(std::is_base_of<AbstractMap<Map>, Map>::value,
+                  "Map doesn't inherit from AbstractMap");
+
+    using ValueInterface = typename Map::value_interface;
+    static_assert(std::is_base_of<AbstractMapValue<ValueInterface>,
+                                  ValueInterface>::value,
+                  "ValueInterface doesn't inherit from AbstractMapValue");
+    static_assert(ValueInterface::default_value_kind == AbstractValueKind::Top,
+                  "ValueInterface::default_value_kind is not Top");
+  }
+};
+
 /*
  * The definition of an element of an abstract environment, i.e., a map from a
  * (possibly infinite) set of variables to an abstract domain implemented as a

diff --git a/include/sparta/AbstractPartition.h b/include/sparta/AbstractPartition.h
@@ -18,6 +18,10 @@
 #include <sparta/AbstractMapValue.h>
 
 namespace sparta {
+namespace ap_impl {
+template <typename Map>
+class AbstractPartitionStaticAssert;
+}
 
 /*
  * An abstract partition based on a given abstract map.
@@ -41,25 +45,14 @@ namespace sparta {
  * This makes for a much simpler implementation.
  */
 template <typename Map>
-class AbstractPartition final : public AbstractDomain<AbstractPartition<Map>> {
+class AbstractPartition final
+    : public AbstractDomain<AbstractPartition<Map>>,
+      private ap_impl::AbstractPartitionStaticAssert<Map> {
  public:
   using Label = typename Map::key_type;
   using Domain = typename Map::mapped_type;
   using MapType = Map;
 
-  ~AbstractPartition() {
-    static_assert(std::is_base_of<AbstractMap<Map>, Map>::value,
-                  "Map doesn't inherit from AbstractMap");
-
-    using ValueInterface = typename Map::value_interface;
-    static_assert(std::is_base_of<AbstractMapValue<ValueInterface>,
-                                  ValueInterface>::value,
-                  "ValueInterface doesn't inherit from AbstractMapValue");
-    static_assert(ValueInterface::default_value_kind ==
-                      AbstractValueKind::Bottom,
-                  "ValueInterface::default_value_kind is not Bottom");
-  }
-
   /*
    * The default constructor produces the Bottom value.
    */
@@ -308,6 +301,25 @@ struct BottomValueInterface final
       AbstractValueKind::Bottom;
 };
 
+namespace ap_impl {
+template <typename Map>
+class AbstractPartitionStaticAssert {
+ protected:
+  ~AbstractPartitionStaticAssert() {
+    static_assert(std::is_base_of<AbstractMap<Map>, Map>::value,
+                  "Map doesn't inherit from AbstractMap");
+
+    using ValueInterface = typename Map::value_interface;
+    static_assert(std::is_base_of<AbstractMapValue<ValueInterface>,
+                                  ValueInterface>::value,
+                  "ValueInterface doesn't inherit from AbstractMapValue");
+    static_assert(ValueInterface::default_value_kind ==
+                      AbstractValueKind::Bottom,
+                  "ValueInterface::default_value_kind is not Bottom");
+  }
+};
+} // namespace ap_impl
+
 } // namespace sparta
 
 template <typename Map>

diff --git a/include/sparta/FlatMap.h b/include/sparta/FlatMap.h
@@ -19,6 +19,15 @@
 #include <sparta/PatriciaTreeCore.h>
 
 namespace sparta {
+namespace fm_impl {
+template <typename Key,
+          typename Value,
+          typename ValueInterface,
+          typename KeyCompare,
+          typename KeyEqual,
+          typename AllocatorOrContainer>
+class FlatMapStaticAssert;
+}
 
 /*
  * Represents a map implemented with a sorted vector.
@@ -34,12 +43,19 @@ template <typename Key,
           typename KeyEqual = std::equal_to<Key>,
           typename AllocatorOrContainer =
               boost::container::new_allocator<std::pair<Key, Value>>>
-class FlatMap final : public AbstractMap<FlatMap<Key,
-                                                 Value,
-                                                 ValueInterface,
-                                                 KeyCompare,
-                                                 KeyEqual,
-                                                 AllocatorOrContainer>> {
+class FlatMap final
+    : public AbstractMap<FlatMap<Key,
+                                 Value,
+                                 ValueInterface,
+                                 KeyCompare,
+                                 KeyEqual,
+                                 AllocatorOrContainer>>,
+      private fm_impl::FlatMapStaticAssert<Key,
+                                           Value,
+                                           ValueInterface,
+                                           KeyCompare,
+                                           KeyEqual,
+                                           AllocatorOrContainer> {
  private:
   using BoostFlatMap =
       boost::container::flat_map<Key, Value, KeyCompare, AllocatorOrContainer>;
@@ -59,15 +75,6 @@ class FlatMap final : public AbstractMap<FlatMap<Key,
   constexpr static AbstractMapMutability mutability =
       AbstractMapMutability::Mutable;
 
-  ~FlatMap() {
-    static_assert(std::is_same_v<Value, mapped_type>,
-                  "Value must be equal to ValueInterface::type");
-    static_assert(std::is_base_of<AbstractMapValue<ValueInterface>,
-                                  ValueInterface>::value,
-                  "ValueInterface doesn't inherit from AbstractMapValue");
-    ValueInterface::check_interface();
-  }
-
  private:
   struct ComparePairWithKey {
     bool operator()(const value_type& pair, const Key& key) const {
@@ -389,4 +396,25 @@ class FlatMap final : public AbstractMap<FlatMap<Key,
   BoostFlatMap m_map;
 };
 
+namespace fm_impl {
+template <typename Key,
+          typename Value,
+          typename ValueInterface,
+          typename KeyCompare,
+          typename KeyEqual,
+          typename AllocatorOrContainer>
+class FlatMapStaticAssert {
+ protected:
+  ~FlatMapStaticAssert() {
+
+    static_assert(std::is_same_v<Value, typename ValueInterface::type>,
+                  "Value must be equal to ValueInterface::type");
+    static_assert(std::is_base_of<AbstractMapValue<ValueInterface>,
+                                  ValueInterface>::value,
+                  "ValueInterface doesn't inherit from AbstractMapValue");
+    ValueInterface::check_interface();
+  }
+};
+} // namespace fm_impl
+
 } // namespace sparta
diff --git a/include/sparta/HashMap.h b/include/sparta/HashMap.h
@@ -19,6 +19,14 @@
 #include <sparta/PatriciaTreeCore.h>
 
 namespace sparta {
+namespace hm_impl {
+template <typename Key,
+          typename Value,
+          typename ValueInterface,
+          typename KeyHash,
+          typename KeyEqual>
+class HashMapStaticAssert;
+}
 
 /*
  * A hash map.
@@ -34,7 +42,9 @@ template <typename Key,
           typename KeyEqual = std::equal_to<Key>>
 class HashMap final
     : public AbstractMap<
-          HashMap<Key, Value, ValueInterface, KeyHash, KeyEqual>> {
+          HashMap<Key, Value, ValueInterface, KeyHash, KeyEqual>>,
+      private hm_impl::
+          HashMapStaticAssert<Key, Value, ValueInterface, KeyHash, KeyEqual> {
  public:
   using StdUnorderedMap = std::unordered_map<Key, Value, KeyHash, KeyEqual>;
 
@@ -53,15 +63,6 @@ class HashMap final
   constexpr static AbstractMapMutability mutability =
       AbstractMapMutability::Mutable;
 
-  ~HashMap() {
-    static_assert(std::is_same_v<Value, mapped_type>,
-                  "Value must be equal to ValueInterface::type");
-    static_assert(std::is_base_of<AbstractMapValue<ValueInterface>,
-                                  ValueInterface>::value,
-                  "ValueInterface doesn't inherit from AbstractMapValue");
-    ValueInterface::check_interface();
-  }
-
   explicit HashMap() = default;
 
   explicit HashMap(std::initializer_list<std::pair<Key, Value>> l) {
@@ -333,4 +334,23 @@ class HashMap final
   StdUnorderedMap m_map;
 };
 
+namespace hm_impl {
+template <typename Key,
+          typename Value,
+          typename ValueInterface,
+          typename KeyHash,
+          typename KeyEqual>
+class HashMapStaticAssert {
+ protected:
+  ~HashMapStaticAssert() {
+    static_assert(std::is_same_v<Value, typename ValueInterface::type>,
+                  "Value must be equal to ValueInterface::type");
+    static_assert(std::is_base_of<AbstractMapValue<ValueInterface>,
+                                  ValueInterface>::value,
+                  "ValueInterface doesn't inherit from AbstractMapValue");
+    ValueInterface::check_interface();
+  };
+};
+} // namespace hm_impl
+
 } // namespace sparta
diff --git a/include/sparta/PatriciaTreeHashMap.h b/include/sparta/PatriciaTreeHashMap.h
@@ -23,6 +23,15 @@
 #include <sparta/PatriciaTreeMap.h>
 
 namespace sparta {
+namespace pthm_impl {
+template <typename Key,
+          typename Value,
+          typename ValueInterface,
+          typename KeyHash,
+          typename KeyCompare,
+          typename KeyEqual>
+class PatriciaTreeHashMapStaticAssert;
+}
 
 /*
  * This structure implements a generalized hash map on top of patricia trees.
@@ -45,7 +54,13 @@ class PatriciaTreeHashMap final
                                              ValueInterface,
                                              KeyHash,
                                              KeyCompare,
-                                             KeyEqual>> {
+                                             KeyEqual>>,
+      private pthm_impl::PatriciaTreeHashMapStaticAssert<Key,
+                                                         Value,
+                                                         ValueInterface,
+                                                         KeyHash,
+                                                         KeyCompare,
+                                                         KeyEqual> {
  private:
   using SmallVector = boost::container::small_vector<std::pair<Key, Value>, 1>;
   using FlatMapT =
@@ -100,15 +115,6 @@ class PatriciaTreeHashMap final
   constexpr static AbstractMapMutability mutability =
       AbstractMapMutability::Mutable;
 
-  ~PatriciaTreeHashMap() {
-    static_assert(std::is_same_v<Value, mapped_type>,
-                  "Value must be equal to ValueInterface::type");
-    static_assert(std::is_base_of<AbstractMapValue<ValueInterface>,
-                                  ValueInterface>::value,
-                  "ValueInterface doesn't inherit from AbstractMapValue");
-    ValueInterface::check_interface();
-  }
-
   bool empty() const { return m_tree.empty(); }
 
   size_t size() const {
@@ -274,4 +280,24 @@ class PatriciaTreeHashMap final
   PatriciaTreeT m_tree;
 };
 
+namespace pthm_impl {
+template <typename Key,
+          typename Value,
+          typename ValueInterface,
+          typename KeyHash,
+          typename KeyCompare,
+          typename KeyEqual>
+class PatriciaTreeHashMapStaticAssert {
+ protected:
+  ~PatriciaTreeHashMapStaticAssert() {
+    static_assert(std::is_same_v<Value, typename ValueInterface::type>,
+                  "Value must be equal to ValueInterface::type");
+    static_assert(std::is_base_of<AbstractMapValue<ValueInterface>,
+                                  ValueInterface>::value,
+                  "ValueInterface doesn't inherit from AbstractMapValue");
+    ValueInterface::check_interface();
+  }
+};
+} // namespace pthm_impl
+
 } // namespace sparta