[!250] Fix VariableBlockVector's CreateIterator

Merge branch 'bugfix/fix-vbv-create-iterator' into 'master'

ref:core/dune-istl When the postfix increment operator is used, block sizes
will not be set properly, see [#56].

This implementation places the allocation into the destructor of the iterator.

It does not need additional structures.

This MR also fixes the test for interoperability with STL algorithms: As the
CreateIterator is only an output iterator, use std::fill_n() instead of
std::fill().

See [!249]. Fixes [#56].

See merge request [!250]

  [#56]: gitlab.dune-project.org/NoneNone/issues/56
  [!249]: gitlab.dune-project.org/NoneNone/merge_requests/249
  [!250]: gitlab.dune-project.org/core/dune-istl/merge_requests/250


Closes #56

dune/istl/test/vbvectortest.cc

@@ -32,7 +32,8 @@ int main()
     Calling the create iterator is not allowed, now.
     We have to un-initialize it first:
   */
-  v3.resize(20); // this makes v3 unitialized again
+  std::size_t size = 20;
+  v3.resize(size); // this makes v3 unitialized again
 
   // Set block sizes with CreateIterator:
   for (auto cIt = v3.createbegin(); cIt!=v3.createend(); ++cIt)
@@ -41,7 +42,10 @@ int main()
   v3 = 1.0;
 
   // Test whether something from <algorithm> can be used to set the block sizes
-  std::fill(v1.createbegin(), v1.createend(), 10);
+  // We can't use std::fill() here, as that requires a forward iterator, std::fill_n()
+  // is more lenient and settles for an output iterator
+  v1.resize(size);
+  std::fill_n(v1.createbegin(), size, 10);
 
   // More formally: test whether the CreateIterator is an output iterator in the stl sense
   v1.resize(5);

dune/istl/vbvector.hh

@@ -397,70 +397,72 @@ namespace Dune {
 
     //===== the creation interface
 
+    class CreateIterator;
+
+#ifndef DOXYGEN
+
+    // The window_type does not hand out a reference to its size,
+    // so in order to provide a valid iterator, we need a workaround
+    // to make assignment possible. This proxy enables just that by
+    // implicitly converting to the stored size for read access and
+    // tunneling assignment to the accessor method of the window.
+    struct SizeProxy
+    {
+
+      operator size_type() const
+      {
+        return target->getsize();
+      }
+
+      SizeProxy& operator=(size_type size)
+      {
+        target->setsize(size);
+        return *this;
+      }
+
+    private:
+
+      friend class CreateIterator;
+
+      SizeProxy(window_type& t)
+        : target(&t)
+      {}
+
+      window_type* target;
+    };
+
+#endif // DOXYGEN
+
     //! Iterator class for sequential creation of blocks
     class CreateIterator
     : public std::iterator<std::output_iterator_tag,   // iterator category
                            size_type,                  // value type
                            size_type*,                 // pointer type
-                           size_type&>                 // reference type
+                           SizeProxy>                 // reference type
     {
     public:
       //! constructor
-      CreateIterator (VariableBlockVector& _v, int _i) : v(_v)
-      {
-        i = _i;
-        k = 0;
-        n = 0;
+      CreateIterator (VariableBlockVector& _v, int _i, bool _isEnd) :
+        v(_v),
+        i(_i),
+        isEnd(_isEnd) {}
+
+      ~CreateIterator() {
+        // When the iterator gets destructed, we allocate the memory
+        // for the VariableBlockVector if
+        // 1. the current iterator was not created as enditarator
+        // 2. we're at the last block
+        // 3. the vector hasn't been initialized earlier
+        if (not isEnd && i==v.nblocks && not v.initialized)
+          v.allocate();
       }
 
       //! prefix increment
       CreateIterator& operator++()
       {
-        // we are at block i and the blocks size is known
-
-        // set the blocks size to current k
-        v.block[i].setsize(k);
-
-        // accumulate total size
-        n += k;
-
         // go to next block
         ++i;
 
-        // reset block size
-        k = 0;
-
-        // if we are past the last block, finish off
-        if (i==v.nblocks)
-        {
-          // now we can allocate the big array in the base class of v
-          v.n = n;
-          if (n>0)
-          {
-            // allocate and construct objects
-            v.p = v.allocator_.allocate(n);
-            new (v.p)B[n];
-          }
-          else
-          {
-            v.n = 0;
-            v.p = nullptr;
-          }
-
-          // and we set the window pointer
-          if (v.nblocks>0)
-          {
-            v.block[0].setptr(v.p);                       // pointer tofirst block
-            for (size_type j=1; j<v.nblocks; ++j)               // and the rest
-              v.block[j].setptr(v.block[j-1].getptr()+v.block[j-1].getsize());
-          }
-
-          // and the vector is ready
-          v.initialized = true;
-
-          //std::cout << "made vbvector with " << v.n << " components" << std::endl;
-        }
-
         return *this;
       }
 
@@ -493,20 +495,24 @@ namespace Dune {
       //! set size of current block
       void setblocksize (size_type _k)
       {
-        k = _k;
+        v.block[i].setsize(_k);
       }
 
       //! Access size of current block
-      size_type& operator*()
+#ifdef DOXYGEN
+      size_type&
+#else
+      SizeProxy
+#endif
+      operator*()
       {
-        return k;
+        return {v.block[i]};
       }
 
     private:
       VariableBlockVector& v;     // my vector
       size_type i;                      // current block to be defined
-      size_type k;                      // size of current block to be defined
-      size_type n;                      // total number of elements to be allocated
+      const bool isEnd; // flag if this object was created as the end iterator.
     };
 
     // CreateIterator wants to set all the arrays ...
@@ -518,13 +524,13 @@ namespace Dune {
 #ifdef DUNE_ISTL_WITH_CHECKING
       if (initialized) DUNE_THROW(ISTLError,"no CreateIterator in initialized state");
 #endif
-      return CreateIterator(*this,0);
+      return CreateIterator(*this,0, false);
     }
 
     //! get create iterator pointing to one after the last block
     CreateIterator createend ()
     {
-      return CreateIterator(*this,nblocks);
+      return CreateIterator(*this,nblocks, true);
     }
 
 
@@ -712,6 +718,38 @@ namespace Dune {
 
 
   private:
+
+    void allocate() {
+      if (this->initialized)
+        DUNE_THROW(ISTLError, "Attempt to re-allocate already initialized VariableBlockVector");
+
+      // calculate space needed:
+      this->n=0;
+      for(size_type i = 0; i < nblocks; i++) {
+        this->n += block[i].size();
+      }
+
+      // now we can allocate the big array in the base class of v
+      if (this->n>0)
+      {
+        // allocate and construct objects
+        this->p = allocator_.allocate(this->n);
+        new (this->p)B[this->n];
+      }
+      else
+      {
+        this->p = nullptr;
+      }
+
+      // and we set the window pointers
+      this->block[0].setptr(this->p); // pointer to first block
+      for (size_type j=1; j<nblocks; ++j) // and the rest
+        block[j].setptr(block[j-1].getptr()+block[j-1].getsize());
+
+      // and the vector is ready
+      this->initialized = true;
+    }
+
     size_type nblocks;            // number of blocks in vector
     window_type* block;     // array of blocks pointing to the array in the base class
     bool initialized;       // true if vector has been initialized