Guard against overloaded operator&

teslamotors · May 4, 2024 · 20ad5da · 20ad5da
1 parent 4d53055
commit 20ad5da
Show file tree

Hide file tree

Showing 9 changed files with 190 additions and 30 deletions.
diff --git a/include/fixed_containers/algorithm.hpp b/include/fixed_containers/algorithm.hpp
@@ -1,5 +1,7 @@
 #pragma once
 
+#include "fixed_containers/memory.hpp"
+
 #include <memory>
 
 namespace fixed_containers::algorithm
@@ -11,8 +13,8 @@ constexpr FwdIt2 uninitialized_relocate(FwdIt1 first, FwdIt1 last, FwdIt2 d_firs
 {
     while (first != last)
     {
-        std::construct_at(&*d_first, std::move(*first));
-        std::destroy_at(&*first);
+        memory::construct_at_address_of(*d_first, std::move(*first));
+        memory::destroy_at_address_of(*first);
         ++d_first;
         ++first;
     }
@@ -28,8 +30,8 @@ constexpr BidirIt2 uninitialized_relocate_backward(BidirIt1 first, BidirIt1 last
     {
         --d_last;
         --last;
-        std::construct_at(&*d_last, std::move(*last));
-        std::destroy_at(&*last);
+        memory::construct_at_address_of(*d_last, std::move(*last));
+        memory::destroy_at_address_of(*last);
     }
     return d_last;
 }

diff --git a/include/fixed_containers/fixed_deque.hpp b/include/fixed_containers/fixed_deque.hpp
@@ -318,7 +318,7 @@ class FixedDequeBase
     {
         check_not_full(loc);
         auto entry_it = advance_all_after_iterator_by_n(it, 1);
-        std::construct_at(&*entry_it, v);
+        memory::construct_at_address_of(*entry_it, v);
         return entry_it;
     }
     constexpr iterator insert(
@@ -328,7 +328,7 @@ class FixedDequeBase
     {
         check_not_full(loc);
         auto entry_it = advance_all_after_iterator_by_n(it, 1);
-        std::construct_at(&*entry_it, std::move(v));
+        memory::construct_at_address_of(*entry_it, std::move(v));
         return entry_it;
     }
     template <InputIterator InputIt>
@@ -355,7 +355,7 @@ class FixedDequeBase
     {
         check_not_full(std_transition::source_location::current());
         auto entry_it = advance_all_after_iterator_by_n(it, 1);
-        std::construct_at(&*entry_it, std::forward<Args>(args)...);
+        memory::construct_at_address_of(*entry_it, std::forward<Args>(args)...);
         return entry_it;
     }
 
@@ -597,7 +597,7 @@ class FixedDequeBase
         auto write_it = advance_all_after_iterator_by_n(it, entry_count_to_add);
         for (auto w_it = write_it; first != last; std::advance(first, 1), std::advance(w_it, 1))
         {
-            std::construct_at(&*w_it, *first);
+            memory::construct_at_address_of(*w_it, *first);
         }
         return write_it;
     }
@@ -740,7 +740,7 @@ class FixedDequeBase
     constexpr void destroy_at(std::size_t i)
         requires NotTriviallyDestructible<T>
     {
-        std::destroy_at(&unchecked_at(i));
+        memory::destroy_at_address_of(unchecked_at(i));
     }
 
     constexpr void destroy_range(iterator /*first*/, iterator /*last*/)
@@ -752,23 +752,23 @@ class FixedDequeBase
     {
         for (; first != last; ++first)
         {
-            std::destroy_at(&*first);
+            memory::destroy_at_address_of(*first);
         }
     }
 
     constexpr void place_at(const std::size_t i, const value_type& v)
     {
-        std::construct_at(&unchecked_at(i), v);
+        memory::construct_at_address_of(unchecked_at(i), v);
     }
     constexpr void place_at(const std::size_t i, value_type&& v)
     {
-        std::construct_at(&unchecked_at(i), std::move(v));
+        memory::construct_at_address_of(unchecked_at(i), std::move(v));
     }
 
     template <class... Args>
     constexpr void emplace_at(const std::size_t i, Args&&... args)
     {
-        std::construct_at(&unchecked_at(i), std::forward<Args>(args)...);
+        memory::construct_at_address_of(unchecked_at(i), std::forward<Args>(args)...);
     }
 
     // [WORKAROUND-1] - Needed by the non-trivially-copyable flavor of FixedDeque

diff --git a/include/fixed_containers/fixed_vector.hpp b/include/fixed_containers/fixed_vector.hpp
@@ -5,6 +5,7 @@
 #include "fixed_containers/consteval_compare.hpp"
 #include "fixed_containers/iterator_utils.hpp"
 #include "fixed_containers/max_size.hpp"
+#include "fixed_containers/memory.hpp"
 #include "fixed_containers/optional_storage.hpp"
 #include "fixed_containers/preconditions.hpp"
 #include "fixed_containers/random_access_iterator_transformer.hpp"
@@ -167,7 +168,7 @@ class FixedVectorBase
         {
             if (std::is_constant_evaluated())
             {
-                std::construct_at(&array());
+                memory::construct_at_address_of(array());
             }
         }
     }
@@ -297,7 +298,7 @@ class FixedVectorBase
     {
         check_not_full(loc);
         auto entry_it = advance_all_after_iterator_by_n(it, 1);
-        std::construct_at(&*entry_it, v);
+        memory::construct_at_address_of(*entry_it, v);
         return entry_it;
     }
     constexpr iterator insert(
@@ -307,7 +308,7 @@ class FixedVectorBase
     {
         check_not_full(loc);
         auto entry_it = advance_all_after_iterator_by_n(it, 1);
-        std::construct_at(&*entry_it, std::move(v));
+        memory::construct_at_address_of(*entry_it, std::move(v));
         return entry_it;
     }
     template <InputIterator InputIt>
@@ -338,7 +339,7 @@ class FixedVectorBase
     {
         check_not_full(std_transition::source_location::current());
         auto entry_it = advance_all_after_iterator_by_n(it, 1);
-        std::construct_at(&*entry_it, std::forward<Args>(args)...);
+        memory::construct_at_address_of(*entry_it, std::forward<Args>(args)...);
         return entry_it;
     }
 
@@ -505,10 +506,13 @@ class FixedVectorBase
         return unchecked_at(back_index());
     }
 
-    constexpr value_type* data() noexcept { return &optional_storage_detail::get(*array().data()); }
+    constexpr value_type* data() noexcept
+    {
+        return std::addressof(optional_storage_detail::get(*array().data()));
+    }
     constexpr const value_type* data() const noexcept
     {
-        return &optional_storage_detail::get(*array().data());
+        return std::addressof(optional_storage_detail::get(*array().data()));
     }
 
     /**
@@ -610,7 +614,7 @@ class FixedVectorBase
         auto write_it = advance_all_after_iterator_by_n(it, entry_count_to_add);
         for (auto w_it = write_it; first != last; std::advance(first, 1), std::advance(w_it, 1))
         {
-            std::construct_at(&*w_it, *first);
+            memory::construct_at_address_of(*w_it, *first);
         }
         return write_it;
     }
@@ -720,7 +724,7 @@ class FixedVectorBase
     constexpr void destroy_at(std::size_t i)
         requires NotTriviallyDestructible<T>
     {
-        std::destroy_at(&unchecked_at(i));
+        memory::destroy_at_address_of(unchecked_at(i));
     }
 
     constexpr void destroy_range(iterator /*first*/, iterator /*last*/)
@@ -732,23 +736,23 @@ class FixedVectorBase
     {
         for (; first != last; ++first)
         {
-            std::destroy_at(&*first);
+            memory::destroy_at_address_of(*first);
         }
     }
 
     constexpr void place_at(const std::size_t i, const value_type& v)
     {
-        std::construct_at(&unchecked_at(i), v);
+        memory::construct_at_address_of(unchecked_at(i), v);
     }
     constexpr void place_at(const std::size_t i, value_type&& v)
     {
-        std::construct_at(&unchecked_at(i), std::move(v));
+        memory::construct_at_address_of(unchecked_at(i), std::move(v));
     }
 
     template <class... Args>
     constexpr void emplace_at(const std::size_t i, Args&&... args)
     {
-        std::construct_at(&unchecked_at(i), std::forward<Args>(args)...);
+        memory::construct_at_address_of(unchecked_at(i), std::forward<Args>(args)...);
     }
 
     // [WORKAROUND-1] - Needed by the non-trivially-copyable flavor of FixedVector

diff --git a/include/fixed_containers/memory.hpp b/include/fixed_containers/memory.hpp
@@ -0,0 +1,23 @@
+#pragma once
+
+#include <memory>
+
+namespace fixed_containers::memory
+{
+// Similar to https://en.cppreference.com/w/cpp/memory/construct_at
+// but uses references and correctly handles types that overload operator&
+template <typename T, typename ... Args>
+constexpr T& construct_at_address_of(T& p, Args&&... args)
+{
+    return *std::construct_at(std::addressof(p), std::forward<Args>(args)...);
+}
+
+// Similar to https://en.cppreference.com/w/cpp/memory/destroy_at
+// but uses references and correctly handles types that overload operator&
+template <typename T>
+constexpr void destroy_at_address_of(T& p)
+{
+    std::destroy_at(std::addressof(p));
+}
+
+}  // namespace fixed_containers::memory
diff --git a/include/fixed_containers/random_access_iterator.hpp b/include/fixed_containers/random_access_iterator.hpp
@@ -6,6 +6,7 @@
 #include <cstddef>
 #include <cstdint>
 #include <iterator>
+#include <memory>
 
 namespace fixed_containers
 {
@@ -14,9 +15,15 @@ concept RandomAccessEntryProvider = requires(P p, P other, std::size_t unsigned_
     p.advance(unsigned_integer);
     p.recede(unsigned_integer);
     p.get();
-    { p == other } -> std::same_as<bool>;
-    { p <=> other };
-    { p - other } -> std::same_as<std::ptrdiff_t>;
+    {
+        p == other
+    } -> std::same_as<bool>;
+    {
+        p <=> other
+    };
+    {
+        p - other
+    } -> std::same_as<std::ptrdiff_t>;
 };
 
 template <RandomAccessEntryProvider ConstEntryProvider,
@@ -97,7 +104,7 @@ class RandomAccessIterator
     {
         if constexpr (SAFE_LIFETIME)
         {
-            return &reference_provider_.get();
+            return std::addressof(reference_provider_.get());
         }
         else
         {

diff --git a/include/fixed_containers/random_access_iterator_transformer.hpp b/include/fixed_containers/random_access_iterator_transformer.hpp
@@ -4,6 +4,7 @@
 
 #include <compare>
 #include <iterator>
+#include <memory>
 #include <type_traits>
 #include <utility>
 
@@ -87,7 +88,10 @@ class RandomAccessIteratorTransformer
 
     constexpr reference operator*() const noexcept { return unary_function_(*iterator_); }
 
-    constexpr pointer operator->() const noexcept { return &unary_function_(*iterator_); }
+    constexpr pointer operator->() const noexcept
+    {
+        return std::addressof(unary_function_(*iterator_));
+    }
 
     constexpr reference operator[](difference_type off) const
     {

diff --git a/test/fixed_deque_test.cpp b/test/fixed_deque_test.cpp
@@ -2050,6 +2050,61 @@ TEST(FixedDeque, NonTriviallyCopyableMoveAssignment)
     EXPECT_TRUE(std::ranges::equal(v2, std::array<MockNonTrivialInt, 2>{1, 2}));
 }
 
+TEST(FixedDeque, OverloadedAddressOfOperator)
+{
+    {
+        FixedDeque<MockFailingAddressOfOperator, 15> v{};
+        v.push_back({});
+        v.assign({});
+        v.insert(v.begin(), {});
+        v.emplace(v.begin());
+        v.emplace_back();
+        v.erase(v.begin());
+        ASSERT_FALSE(v.empty());
+        v.clear();
+        ASSERT_TRUE(v.empty());
+    }
+
+    {
+        constexpr FixedDeque<MockFailingAddressOfOperator, 15> v{5};
+        static_assert(!v.empty());
+    }
+
+    {
+        FixedDeque<MockFailingAddressOfOperator, 15> v{5};
+        ASSERT_FALSE(v.empty());
+        auto it = v.begin();
+        auto it_ref = *it;
+        it_ref.do_nothing();
+        it->do_nothing();
+        (void)it++;
+        (void)it--;
+        ++it;
+        --it;
+        auto it_ref2 = *it;
+        it_ref2.do_nothing();
+        it->do_nothing();
+        it[0].do_nothing();
+    }
+
+    {
+        constexpr FixedDeque<MockFailingAddressOfOperator, 15> v{5};
+        static_assert(!v.empty());
+        auto it = v.cbegin();
+        auto it_ref = *it;
+        it_ref.do_nothing();
+        it->do_nothing();
+        (void)it++;
+        (void)it--;
+        ++it;
+        --it;
+        auto it_ref2 = *it;
+        it_ref2.do_nothing();
+        it->do_nothing();
+        it[0].do_nothing();
+    }
+}
+
 TEST(FixedDeque, ClassTemplateArgumentDeduction)
 {
     // Compile-only test