clang-tidy

ndsev · Sep 10, 2024 · 8c21b6a · 8c21b6a
1 parent c6a6d7a
commit 8c21b6a
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 8 deletions.
diff --git a/runtime/ClangTidySuppressions.txt b/runtime/ClangTidySuppressions.txt
@@ -1,3 +1,10 @@
+# Variant is a generic container and exceptions may be thrown from the element types.
+bugprone-exception-escape:src/zserio/Variant.h:242
+bugprone-exception-escape:src/zserio/Variant.h:269
+
+# Iteration through all variant types is needed for folding expression. m_data active element is moved out but index remains so it's safe to access.
+bugprone-use-after-move:src/zserio/Variant.h:666
+
 # This is necessary for low level implementation of Span to mimic standard C++20 'std::span' abstraction.
 cppcoreguidelines-avoid-c-arrays:src/zserio/Span.h:112
 cppcoreguidelines-avoid-c-arrays:src/zserio/Span.h:123

diff --git a/runtime/src/zserio/Variant.h b/runtime/src/zserio/Variant.h
@@ -67,7 +67,14 @@ template <auto I>
 constexpr in_place_index_t<I> in_place_index{};
 
 /**
- * Type safe container for single values of any of the listed types which doesn't need RTTI.
+ * Implementation of variant type which allocates memory when selected type
+ * doesn't fit under 3*sizeof(void*)
+ *
+ * Largely compatible with std::variant with following differences:
+ *+ access only through index whose type has to be specified - requested by NDS
+ *+ holds_alternative() ommited
+ *+ variant_npos ommited
+ *+ get/get_if() exists as member functions as well
  */
 template <typename ALLOC, typename INDEX, typename... T>
 class BasicVariant : public AllocatorHolder<ALLOC>
@@ -93,6 +100,8 @@ class BasicVariant : public AllocatorHolder<ALLOC>
      * Constructor from given allocator.
      *
      * \param allocator Allocator to be used.
+     *
+     * \throw can throw any exception thrown by T[0]
      */
     explicit BasicVariant(const ALLOC& allocator) :
             AllocatorHolder<ALLOC>(allocator)
@@ -108,6 +117,8 @@ class BasicVariant : public AllocatorHolder<ALLOC>
      *
      * \param in_place_index Index of the active element.
      * \param args Arguments to be forwarded for element construction.
+     *
+     * \throw can throw any exception thrown by T[I]
      */
     template <INDEX I, typename... ARGS, std::enable_if_t<!detail::is_heap_allocated_v<I, T...>>* = nullptr,
             std::enable_if_t<!detail::is_first_alloc<ARGS...>::value>* = nullptr>
@@ -121,6 +132,8 @@ class BasicVariant : public AllocatorHolder<ALLOC>
      * \param in_place_index Index of the active element.
      * \param allocator Allocator to be used.
      * \param args Arguments to be forwarded for element construction.
+     *
+     * \throw can throw any exception thrown by T[I]
      */
     template <INDEX I, typename... ARGS, std::enable_if_t<!detail::is_heap_allocated_v<I, T...>>* = nullptr>
     BasicVariant(zserio::in_place_index_t<I>, const ALLOC& allocator, ARGS&&... args) :
@@ -134,20 +147,23 @@ class BasicVariant : public AllocatorHolder<ALLOC>
      * \param in_place_index Index of the active element.
      * \param allocator Allocator to be used.
      * \param args Arguments to be forwarded for element construction.
+     *
+     * \throw can throw any exception thrown by T[I]
      */
     template <INDEX I, typename... ARGS, std::enable_if_t<detail::is_heap_allocated_v<I, T...>>* = nullptr>
     BasicVariant(zserio::in_place_index_t<I>, const ALLOC& allocator, ARGS&&... args) :
             AllocatorHolder<ALLOC>(allocator),
-            m_data(std::in_place_index<static_cast<size_t>(I)>, nullptr)
-    {
-        emplace<I>(std::forward<ARGS>(args)...);
-    }
+            m_data(std::in_place_index<static_cast<size_t>(I)>,
+                    allocateValue<detail::type_at_t<static_cast<size_t>(I), T...>>(std::forward<ARGS>(args)...))
+    {}
 
     /**
      * Constructor with initial emplacement.
      *
      * \param in_place_index Index of the active element.
      * \param args Arguments to be forwarded for element construction.
+     *
+     * \throw can throw any exception thrown by T[I]
      */
     template <INDEX I, typename... ARGS, std::enable_if_t<detail::is_heap_allocated_v<I, T...>>* = nullptr,
             std::enable_if_t<!detail::is_first_alloc<ARGS...>::value>* = nullptr>
@@ -182,6 +198,8 @@ class BasicVariant : public AllocatorHolder<ALLOC>
      *
      * \param other Variant to copy.
      * \param allocator Allocator to be used for dynamic memory allocations.
+     *
+     * \throw can throw any exception thrown by the active element in other
      */
     BasicVariant(const BasicVariant& other, const ALLOC& allocator) :
             AllocatorHolder<ALLOC>(allocator)
@@ -195,6 +213,8 @@ class BasicVariant : public AllocatorHolder<ALLOC>
      * \param other Variant to copy.
      *
      * \return Reference to this.
+     *
+     * \throw can throw any exception thrown by the active element in other
      */
     BasicVariant& operator=(const BasicVariant& other)
     {
@@ -215,8 +235,10 @@ class BasicVariant : public AllocatorHolder<ALLOC>
      * Move constructor.
      *
      * \param other Variant to move from.
+     *
+     * \throw can throw any exception thrown by the active element in other
      */
-    BasicVariant(BasicVariant&& other) noexcept :
+    BasicVariant(BasicVariant&& other) :
             AllocatorHolder<ALLOC>(std::move(other.get_allocator_ref()))
     {
         move(std::move(other));
@@ -227,6 +249,8 @@ class BasicVariant : public AllocatorHolder<ALLOC>
      *
      * \param other Variant to move from.
      * \param allocator Allocator to be used for dynamic memory allocations.
+     *
+     * \throw can throw any exception thrown by the active element in other
      */
     BasicVariant(BasicVariant&& other, const ALLOC& allocator) :
             AllocatorHolder<ALLOC>(allocator)
@@ -258,6 +282,7 @@ class BasicVariant : public AllocatorHolder<ALLOC>
 
     /**
      * Reports if variant is in nonstandard empty state.
+     * Note: which operations lead to an empty state is not specified by ISO C++
      */
     bool valueless_by_exception() const noexcept
     {
@@ -649,13 +674,14 @@ class BasicVariant : public AllocatorHolder<ALLOC>
         }
         if constexpr (detail::is_heap_allocated_v<I, T...>)
         {
-            auto& ptr = std::get<I>(other.m_data);
+            auto& ptr = *std::get_if<I>(&other.m_data);
             m_data.template emplace<I>(ptr);
             ptr = nullptr;
         }
         else
         {
-            m_data.template emplace<I>(std::move(std::get<I>(other.m_data)));
+            auto& val = *std::get_if<I>(&other.m_data);
+            m_data.template emplace<I>(std::move(val));
         }
     }