From 0979bdca186669a739a4bf262376260ffb89cc74 Mon Sep 17 00:00:00 2001 From: Maxime Arthaud Date: Tue, 28 Nov 2023 06:53:32 -0800 Subject: [PATCH] Move static asserts in separate classes to enable default move constructors 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 --- include/sparta/AbstractEnvironment.h | 34 +++++++++------- include/sparta/AbstractPartition.h | 40 ++++++++++++------- include/sparta/FlatMap.h | 58 +++++++++++++++++++++------- include/sparta/HashMap.h | 40 ++++++++++++++----- include/sparta/PatriciaTreeHashMap.h | 46 +++++++++++++++++----- include/sparta/PatriciaTreeMap.h | 35 ++++++++++++----- include/sparta/SetAbstractDomain.h | 19 ++++++--- 7 files changed, 194 insertions(+), 78 deletions(-) diff --git a/include/sparta/AbstractEnvironment.h b/include/sparta/AbstractEnvironment.h index 029c142..c0a7ab3 100644 --- a/include/sparta/AbstractEnvironment.h +++ b/include/sparta/AbstractEnvironment.h @@ -21,6 +21,9 @@ namespace sparta { namespace environment_impl { +template +class AbstractEnvironmentStaticAssert; + template class MapValue; @@ -60,24 +63,13 @@ class value_is_bottom {}; template class AbstractEnvironment final : public AbstractDomainScaffolding, - AbstractEnvironment> { + AbstractEnvironment>, + private environment_impl::AbstractEnvironmentStaticAssert { public: using Variable = typename Map::key_type; using Domain = typename Map::mapped_type; using MapType = Map; - ~AbstractEnvironment() { - static_assert(std::is_base_of, Map>::value, - "Map doesn't inherit from AbstractMap"); - - using ValueInterface = typename Map::value_interface; - static_assert(std::is_base_of, - 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 +class AbstractEnvironmentStaticAssert { + protected: + ~AbstractEnvironmentStaticAssert() { + static_assert(std::is_base_of, Map>::value, + "Map doesn't inherit from AbstractMap"); + + using ValueInterface = typename Map::value_interface; + static_assert(std::is_base_of, + 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 index afe20b5..d418842 100644 --- a/include/sparta/AbstractPartition.h +++ b/include/sparta/AbstractPartition.h @@ -18,6 +18,10 @@ #include namespace sparta { +namespace ap_impl { +template +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 -class AbstractPartition final : public AbstractDomain> { +class AbstractPartition final + : public AbstractDomain>, + private ap_impl::AbstractPartitionStaticAssert { public: using Label = typename Map::key_type; using Domain = typename Map::mapped_type; using MapType = Map; - ~AbstractPartition() { - static_assert(std::is_base_of, Map>::value, - "Map doesn't inherit from AbstractMap"); - - using ValueInterface = typename Map::value_interface; - static_assert(std::is_base_of, - 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 +class AbstractPartitionStaticAssert { + protected: + ~AbstractPartitionStaticAssert() { + static_assert(std::is_base_of, Map>::value, + "Map doesn't inherit from AbstractMap"); + + using ValueInterface = typename Map::value_interface; + static_assert(std::is_base_of, + 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 diff --git a/include/sparta/FlatMap.h b/include/sparta/FlatMap.h index 2bec2dc..0e25e6d 100644 --- a/include/sparta/FlatMap.h +++ b/include/sparta/FlatMap.h @@ -19,6 +19,15 @@ #include namespace sparta { +namespace fm_impl { +template +class FlatMapStaticAssert; +} /* * Represents a map implemented with a sorted vector. @@ -34,12 +43,19 @@ template , typename AllocatorOrContainer = boost::container::new_allocator>> -class FlatMap final : public AbstractMap> { +class FlatMap final + : public AbstractMap>, + private fm_impl::FlatMapStaticAssert { private: using BoostFlatMap = boost::container::flat_map; @@ -59,15 +75,6 @@ class FlatMap final : public AbstractMap, - "Value must be equal to ValueInterface::type"); - static_assert(std::is_base_of, - 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 +class FlatMapStaticAssert { + protected: + ~FlatMapStaticAssert() { + + static_assert(std::is_same_v, + "Value must be equal to ValueInterface::type"); + static_assert(std::is_base_of, + 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 index 00d2a47..8b8b511 100644 --- a/include/sparta/HashMap.h +++ b/include/sparta/HashMap.h @@ -19,6 +19,14 @@ #include namespace sparta { +namespace hm_impl { +template +class HashMapStaticAssert; +} /* * A hash map. @@ -34,7 +42,9 @@ template > class HashMap final : public AbstractMap< - HashMap> { + HashMap>, + private hm_impl:: + HashMapStaticAssert { public: using StdUnorderedMap = std::unordered_map; @@ -53,15 +63,6 @@ class HashMap final constexpr static AbstractMapMutability mutability = AbstractMapMutability::Mutable; - ~HashMap() { - static_assert(std::is_same_v, - "Value must be equal to ValueInterface::type"); - static_assert(std::is_base_of, - ValueInterface>::value, - "ValueInterface doesn't inherit from AbstractMapValue"); - ValueInterface::check_interface(); - } - explicit HashMap() = default; explicit HashMap(std::initializer_list> l) { @@ -333,4 +334,23 @@ class HashMap final StdUnorderedMap m_map; }; +namespace hm_impl { +template +class HashMapStaticAssert { + protected: + ~HashMapStaticAssert() { + static_assert(std::is_same_v, + "Value must be equal to ValueInterface::type"); + static_assert(std::is_base_of, + 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 index 322a150..e96e167 100644 --- a/include/sparta/PatriciaTreeHashMap.h +++ b/include/sparta/PatriciaTreeHashMap.h @@ -23,6 +23,15 @@ #include namespace sparta { +namespace pthm_impl { +template +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 { private: using SmallVector = boost::container::small_vector, 1>; using FlatMapT = @@ -100,15 +115,6 @@ class PatriciaTreeHashMap final constexpr static AbstractMapMutability mutability = AbstractMapMutability::Mutable; - ~PatriciaTreeHashMap() { - static_assert(std::is_same_v, - "Value must be equal to ValueInterface::type"); - static_assert(std::is_base_of, - 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 +class PatriciaTreeHashMapStaticAssert { + protected: + ~PatriciaTreeHashMapStaticAssert() { + static_assert(std::is_same_v, + "Value must be equal to ValueInterface::type"); + static_assert(std::is_base_of, + ValueInterface>::value, + "ValueInterface doesn't inherit from AbstractMapValue"); + ValueInterface::check_interface(); + } +}; +} // namespace pthm_impl + } // namespace sparta diff --git a/include/sparta/PatriciaTreeMap.h b/include/sparta/PatriciaTreeMap.h index bbfa93f..dcea62f 100644 --- a/include/sparta/PatriciaTreeMap.h +++ b/include/sparta/PatriciaTreeMap.h @@ -23,6 +23,10 @@ #include namespace sparta { +namespace ptm_impl { +template +class PatriciaTreeMapStaticAssert; +} /* * This structure implements a map of integer/pointer keys and AbstractDomain @@ -74,7 +78,9 @@ template > class PatriciaTreeMap final - : public AbstractMap> { + : public AbstractMap>, + private ptm_impl:: + PatriciaTreeMapStaticAssert { using Core = pt_core::PatriciaTreeCore; using Codec = typename Core::Codec; @@ -95,15 +101,6 @@ class PatriciaTreeMap final constexpr static AbstractMapMutability mutability = AbstractMapMutability::Immutable; - ~PatriciaTreeMap() { - static_assert(std::is_same_v, - "Value must be equal to ValueInterface::type"); - static_assert(std::is_base_of, - ValueInterface>::value, - "ValueInterface doesn't inherit from AbstractMapValue"); - ValueInterface::check_interface(); - } - bool empty() const { return m_core.empty(); } size_t size() const { return m_core.size(); } @@ -258,4 +255,22 @@ class PatriciaTreeMap final Core m_core; }; +namespace ptm_impl { +template +class PatriciaTreeMapStaticAssert { + protected: + ~PatriciaTreeMapStaticAssert() { + static_assert( + std::is_same_v< + Value, + typename pt_core::PatriciaTreeCore::ValueType>, + "Value must be equal to ValueInterface::type"); + static_assert(std::is_base_of, + ValueInterface>::value, + "ValueInterface doesn't inherit from AbstractMapValue"); + ValueInterface::check_interface(); + } +}; +} // namespace ptm_impl + } // namespace sparta diff --git a/include/sparta/SetAbstractDomain.h b/include/sparta/SetAbstractDomain.h index 03ba71c..3b5c1c7 100644 --- a/include/sparta/SetAbstractDomain.h +++ b/include/sparta/SetAbstractDomain.h @@ -17,6 +17,8 @@ namespace sparta { namespace set_impl { +template +class SetAbstractDomainStaticAssert; template class SetValue; @@ -30,16 +32,12 @@ class SetAbstractDomain final : public PowersetAbstractDomain, const Set&, - SetAbstractDomain> { + SetAbstractDomain>, + private set_impl::SetAbstractDomainStaticAssert { public: using Value = set_impl::SetValue; using Element = typename Set::value_type; - ~SetAbstractDomain() { - static_assert(std::is_base_of, Set>::value, - "Set doesn't inherit from AbstractSet"); - } - SetAbstractDomain() : PowersetAbstractDomain +class SetAbstractDomainStaticAssert { + protected: + ~SetAbstractDomainStaticAssert() { + static_assert(std::is_base_of, Set>::value, + "Set doesn't inherit from AbstractSet"); + } +}; + template class SetValue final : public PowersetImplementation