From 652c180b06655d6714799084f94704c2e242160e Mon Sep 17 00:00:00 2001
From: Felix Salfelder <felix@salfelder.org>
Date: Tue, 28 Feb 2017 21:30:49 +0000
Subject: [PATCH] bucket_sorter bug fix in "remove"

removing an element from a stack does not always work, since the
predecessor of a stack element is not (always) stored. this patch
merges the array head into next, so a top element in a stack can point
to the head of the stack it is in.

remarks:
- i am guessing the intended use (see test below), as it works like that
  for elements deeper down in a stack
- the fix abuses pointers/iterators and infers offsets from address
  differences. (yes it's a bit ugly.)
- memory needs and complexity are unaffected, size_type is probably big
  enough.

test case. B is a bucketsorter operating on a vector V.

 V[0]=0;
 V[1]=1;
 B.push(0);
 B.push(1);

// now, stacks 0 and 1 are singletons.
// try to move 0 to stack #1, should result in
// head_1->0->1->end, but calls remove first, then

 V[0]=1;
 B.update(0); // <- BOOM

// the update calls remove, it "removes" 0 from stack #V[0]=1.
// it's not there yet (!).
// instead 1 (top of bucket #1) must die.
// the result is head_1->0->end, and wrong.
---
 include/boost/pending/bucket_sorter.hpp | 29 ++++++++++++++---------------
 1 file changed, 14 insertions(+), 15 deletions(-)

diff --git a/include/boost/pending/bucket_sorter.hpp b/include/boost/pending/bucket_sorter.hpp
index 8b7926d2..bb93d600 100644
--- a/include/boost/pending/bucket_sorter.hpp
+++ b/include/boost/pending/bucket_sorter.hpp
@@ -12,6 +12,7 @@
 // Revision History:
 //   13 June 2001: Changed some names for clarity. (Jeremy Siek)
 //   01 April 2001: Modified to use new <boost/limits.hpp> header. (JMaddock)
+//   28 Feb 2017: change bucket head, fix bug in remove. (Felix Salfelder)
 //
 #ifndef BOOST_GRAPH_DETAIL_BUCKET_SORTER_HPP
 #define BOOST_GRAPH_DETAIL_BUCKET_SORTER_HPP
@@ -33,9 +34,9 @@ namespace boost {
     bucket_sorter(size_type _length, bucket_type _max_bucket, 
                   const Bucket& _bucket = Bucket(), 
                   const ValueIndexMap& _id = ValueIndexMap()) 
-      : head(_max_bucket, invalid_value()),
-        next(_length, invalid_value()), 
+      : next(_length+_max_bucket, invalid_value()),
         prev(_length, invalid_value()),
+        head(next.size()?(&next[_length]):NULL),
         id_to_value(_length),
         bucket(_bucket), id(_id) { }
     
@@ -47,11 +48,8 @@ namespace boost {
       //check if i is the end of the bucket list 
       if ( next_node != invalid_value() )
         prev[next_node] = prev_node; 
-      //check if i is the begin of the bucket list
-      if ( prev_node != invalid_value() )
-        next[prev_node] = next_node;
-      else //need update head of current bucket list
-        head[ bucket[x] ] = next_node;
+      //update predecessor
+      next[prev_node] = next_node;
     }
 
     void push(const value_type& x) {
@@ -78,13 +76,14 @@ namespace boost {
 
     class stack {
     public:
-      stack(bucket_type _bucket_id, Iter h, Iter n, Iter p, IndexValueMap v,
-            const ValueIndexMap& _id)
+      stack(bucket_type _bucket_id, typename Iter::value_type* h,
+            Iter n, Iter p, IndexValueMap v, const ValueIndexMap& _id)
       : bucket_id(_bucket_id), head(h), next(n), prev(p), value(v), id(_id) {}
 
       // Avoid using default arg for ValueIndexMap so that the default
       // constructor of the ValueIndexMap is not required if not used.
-      stack(bucket_type _bucket_id, Iter h, Iter n, Iter p, IndexValueMap v)
+      stack(bucket_type _bucket_id, typename Iter::value_type* h,
+            Iter n, Iter p, IndexValueMap v)
         : bucket_id(_bucket_id), head(h), next(n), prev(p), value(v) {}
       
       void push(const value_type& x) {
@@ -92,7 +91,7 @@ namespace boost {
         const size_type current = head[bucket_id];
         if ( current != invalid_value() )
           prev[current] = new_head;
-        prev[new_head] = invalid_value();
+        prev[new_head] = bucket_id + (head - next);
         next[new_head] = current;
         head[bucket_id] = new_head;
       }
@@ -101,7 +100,7 @@ namespace boost {
         size_type next_node = next[current];
         head[bucket_id] = next_node;
         if ( next_node != invalid_value() )
-          prev[next_node] = invalid_value();
+          prev[next_node] = bucket_id + (head - next);
       }
       value_type& top() { return value[ head[bucket_id] ]; }
       const value_type& top() const { return value[ head[bucket_id] ]; }
@@ -116,14 +115,14 @@ namespace boost {
     };
     
     stack operator[](const bucket_type& i) {
-      assert(i < head.size());
-      return stack(i, head.begin(), next.begin(), prev.begin(),
+      assert(i < next.size());
+      return stack(i, head, next.begin(), prev.begin(),
                    id_to_value.begin(), id);
     }
   protected:
-    std::vector<size_type>   head;
     std::vector<size_type>   next;
     std::vector<size_type>   prev;
+    typename std::vector<size_type>::value_type* head;
     std::vector<value_type>  id_to_value;
     Bucket bucket;
     ValueIndexMap id;