Fix/bucket iterator indirection operator (#220 rewrite) (#221)

* Add tests for member of pointer operator for bucket iterators

* Fix erroneous conversion from node* to value_type* in bucket iterators

Originally caughy by vslashg  from PR https://github.com/boostorg/unordered/pull/220

* Update change log

* Update GHA config to use containers in C++20 builds for clang-14 as it's incompatible with libstdc++-13

* fixed Python installation problem

* tried variation of former fix

* tried another variation of former fix

* tried yet another variation

* editorial

---------

Co-authored-by: Christian Mazakas <christian.mazakas@gmail.com>

.github/workflows/ci.yml

@@ -95,12 +95,12 @@ jobs:
           - { name: "clang-14 w/ sanitizers (17)", sanitize: yes,
               compiler: clang-14,  cxxstd: '17',    os: 'ubuntu-22.04', ccache_key: "san2" }
           - { name: "clang-14 w/ sanitizers (20)", sanitize: yes,
-              compiler: clang-14,  cxxstd: '20',    os: 'ubuntu-22.04', ccache_key: "san2" }
+              compiler: clang-14,  cxxstd: '20',   container: 'ubuntu:22.04',  os: 'ubuntu-latest', ccache_key: "san2" }
           - { name: "clang-14 w/ sanitizers (2b)", sanitize: yes,
-              compiler: clang-14,  cxxstd: '2b',    os: 'ubuntu-22.04', ccache_key: "san2" }
+              compiler: clang-14,  cxxstd: '2b',   container: 'ubuntu:22.04',  os: 'ubuntu-latest', ccache_key: "san2" }
 
           - { name: "cfoa tsan (clang-14)", cxxstd: '11,14,17,20,2b', os: 'ubuntu-22.04', compiler: clang-14,
-              targets: 'libs/unordered/test//cfoa_tests', thread-sanitize: yes, 
+              targets: 'libs/unordered/test//cfoa_tests', thread-sanitize: yes,
               stdlib: libc++, install: 'clang-14 libc++-14-dev libc++abi-14-dev', ccache_key: "tsan" }
 
           - { compiler: 'clang-15', cxxstd: '11,14',    os: 'ubuntu-22.04', stdlib: libc++, install: 'clang-15 libc++-15-dev libc++abi-15-dev' }
@@ -134,7 +134,7 @@ jobs:
                 apt-key adv --keyserver hkp://keyserver.ubuntu.com:80 --recv-keys E1DD270288B4E6030699E45FA1715D88E1DF1F24
                 for i in {1..${NET_RETRY_COUNT:-3}}; do sudo -E add-apt-repository -y ppa:git-core/ppa && break || sleep 10; done
                 apt-get -o Acquire::Retries=$NET_RETRY_COUNT update
-                apt-get -o Acquire::Retries=$NET_RETRY_COUNT install -y g++ python libpython-dev git
+                apt-get -o Acquire::Retries=$NET_RETRY_COUNT install -y g++ python-is-python3 git
             fi
             # For jobs not compatible with ccache, use "ccache: no" in the matrix
             if [[ "${{ matrix.ccache }}" == "no" ]]; then

doc/unordered/changes.adoc

@@ -19,6 +19,9 @@ a concurrent container from user code.
 * Added Boost.Serialization support to all containers and their (non-local) iterator types.
 * Added support for fancy pointers to open-addressing and concurrent containers.
   This enables scenarios like the use of Boost.Interprocess allocators to construct containers in shared memory.
+* Fixed bug in member of pointer operator for local iterators of closed-addressing
+  containers ({github-pr-url}/221[PR#221^], credit goes to GitHub user vslashg for finding
+  and fixing this issue).
 * Starting with this release, `boost::unordered_[multi]set` and `boost::unordered_[multi]map`
   only work with C++11 onwards.
 
@@ -39,7 +42,7 @@ when the returned proxy is not used.
   `boost::unordered_node_map` and `boost::unordered_node_set`.
 * Extended heterogeneous lookup to more member functions as specified in
   https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2363r5.html[P2363].
-* Replaced the previous post-mixing process for open-addressing containers with 
+* Replaced the previous post-mixing process for open-addressing containers with
   a new algorithm based on extended multiplication by a constant.
 * Fixed bug in internal emplace() impl where stack-local types were not properly
   constructed using the Allocator of the container which breaks uses-allocator
@@ -56,20 +59,20 @@ when the returned proxy is not used.
 
 * Refactor internal implementation to be dramatically faster
 * Allow `final` Hasher and KeyEqual objects
-* Update documentation, adding benchmark graphs and notes on the new internal 
+* Update documentation, adding benchmark graphs and notes on the new internal
   data structures
 
 == Release 1.79.0
 
 * Improved {cpp}20 support:
-  ** All containers have been updated to support 
+  ** All containers have been updated to support
     heterogeneous `count`, `equal_range` and `find`.
   ** All containers now implement the member function `contains`.
   ** `erase_if` has been implemented for all containers.
 * Improved {cpp}23 support:
   ** All containers have been updated to support
     heterogeneous `erase` and `extract`.
-* Changed behavior of `reserve` to eagerly 
+* Changed behavior of `reserve` to eagerly
   allocate ({github-pr-url}/59[PR#59^]).
 * Various warning fixes in the test suite.
 * Update code to internally use `boost::allocator_traits`.

include/boost/unordered/detail/fca.hpp

@@ -318,7 +318,10 @@ namespace boost {
 
         reference operator*() const noexcept { return dereference(); }
 
-        pointer operator->() const noexcept { return boost::to_address(p); }
+        pointer operator->() const noexcept
+        {
+          return std::addressof(dereference());
+        }
 
         grouped_local_bucket_iterator& operator++() noexcept
         {
@@ -386,7 +389,10 @@ namespace boost {
 
         reference operator*() const noexcept { return dereference(); }
 
-        pointer operator->() const noexcept { return boost::to_address(p); }
+        pointer operator->() const noexcept
+        {
+          return std::addressof(dereference());
+        }
 
         const_grouped_local_bucket_iterator& operator++() noexcept
         {

test/helpers/test.hpp

@@ -7,6 +7,7 @@
 #define BOOST_UNORDERED_TEST_TEST_HEADER
 
 #include <boost/core/lightweight_test.hpp>
+#include <boost/core/lightweight_test_trait.hpp>
 #include <boost/preprocessor/cat.hpp>
 #include <boost/preprocessor/stringize.hpp>
 

test/unordered/bucket_tests.cpp

@@ -15,6 +15,7 @@
 #include "../objects/test.hpp"
 #include "../helpers/random_values.hpp"
 #include "../helpers/helpers.hpp"
+#include "../helpers/metafunctions.hpp"
 
 #if BOOST_WORKAROUND(BOOST_MSVC, < 1400)
 #pragma warning(disable : 4267) // conversion from 'size_t' to 'unsigned int',
@@ -31,6 +32,7 @@ namespace bucket_tests {
 
     typedef typename X::size_type size_type;
     typedef typename X::const_local_iterator const_local_iterator;
+    typedef typename X::value_type value_type;
     test::random_values<X> v(1000, generator);
 
     X x(v.begin(), v.end());
@@ -58,15 +60,42 @@ namespace bucket_tests {
     }
 
     for (size_type i = 0; i < x.bucket_count(); ++i) {
-      BOOST_TEST(x.bucket_size(i) ==
+      {
-                 static_cast<size_type>(std::distance(x.begin(i), x.end(i))));
+        auto begin = x.begin(i);
-      BOOST_TEST(x.bucket_size(i) ==
+        auto end = x.end(i);
-                 static_cast<size_type>(std::distance(x.cbegin(i), x.cend(i))));
+
-      X const& x_ref = x;
+        BOOST_TEST(x.bucket_size(i) ==
-      BOOST_TEST(x.bucket_size(i) == static_cast<size_type>(std::distance(
+                   static_cast<size_type>(std::distance(begin, end)));
-                                       x_ref.begin(i), x_ref.end(i))));
+
-      BOOST_TEST(x.bucket_size(i) == static_cast<size_type>(std::distance(
+        for (auto pos = begin; pos != end; ++pos) {
-                                       x_ref.cbegin(i), x_ref.cend(i))));
+          using pointer_type = typename std::conditional<test::is_set<X>::value,
+            value_type const*, value_type*>::type;
+
+          pointer_type p = pos.operator->();
+          BOOST_TEST_EQ(p, std::addressof(*pos));
+        }
+
+        auto cbegin = x.cbegin(i);
+        auto cend = x.cend(i);
+        BOOST_TEST(x.bucket_size(i) ==
+                   static_cast<size_type>(std::distance(cbegin, cend)));
+
+        for (auto pos = cbegin; pos != cend; ++pos) {
+          value_type const* p = pos.operator->();
+          BOOST_TEST_EQ(p, std::addressof(*pos));
+        }
+      }
+
+      {
+        X const& x_ref = x;
+        BOOST_TEST_TRAIT_SAME(
+          decltype(x_ref.begin()), decltype(x_ref.cbegin()));
+
+        BOOST_TEST(x.bucket_size(i) == static_cast<size_type>(std::distance(
+                                         x_ref.begin(i), x_ref.end(i))));
+        BOOST_TEST(x.bucket_size(i) == static_cast<size_type>(std::distance(
+                                         x_ref.cbegin(i), x_ref.cend(i))));
+      }
     }
   }