[ublas] shallow_array_adaptor bug and fixes

Subject: [ublas] shallow_array_adaptor bug and fixes
From: Guillermo Ruiz Troyano (ruiztroyano_at_[hidden])
Date: 2012-02-28 08:10:41

I'm been trying to use shallow_array_adaptor as storage with ublas::vector to modify external data. When you try this:

#define BOOST_UBLAS_SHALLOW_ARRAY_ADAPTOR
#include <boost/numeric/ublas/vector.hpp>

struct point {
   double x;
   double y;
   double z;
};

void test() {
   using namespace boost::numeric::ublas;

   point p = { 1, 2, 3 }
   shallow_array_adaptor<double> a(3, &p.x);            // Ok, a holds p address
   vector<double, shallow_array_adaptor<double> > v(a); // Ok, v holds p address
   
   v += v;     // Ok, now p = { 2, 4, 6 }
   v /= 2;     // Ok, now p = { 1, 2, 3 }
   v = v*2;    // <- Oh no, p = { 1, 2, 3 } !!!
}

The bug comes from swap function of shallow_array_adapter that is used by vector. When you assign v with an expression, this constructs a temporary and then swaps with it. The issue is that it swaps pointers without check if both objects own the data:

class shallow_array_adaptor<T> : … {
   ...
   // Swapping
   BOOST_UBLAS_INLINE
   void swap (shallow_array_adaptor &a) {
      if (this != &a) {
         std::swap (size_, a.size_);
         std::swap (own_, a.own_);
         std::swap (data_, a.data_); // This is definitely infidelity
      }
   }
   ...
};

When both are data owners can do that (as unbounded_array does). But if they are not then it should swap as bounded_array does. Something like this:

class shallow_array_adaptor<T> : … {
   ...
   // Swapping
   BOOST_UBLAS_INLINE
   void swap (array_adaptor& a) {
      if (own_ && a.own_) {
         if (this != &a) {
            std::swap(size_, a.size_);
            std::swap(own_, a.own_);
            std::swap(data_, a.data_);
         }
      }
      else if (&data_[0] != &a.data_[0])
         std::swap_ranges(data_, data_+size_, a.data_);
   }
   ...
};

Fixing this function I get:

void test() {
   point p = { 1, 2, 3 }
   shallow_array_adaptor<double> a(3, &p.x);            // Ok, a holds p address
   vector<double, shallow_array_adaptor<double> > v(a); // Ok, v holds p address
   v = v*2;    // <- Ok, now p = { 2, 4, 6 } and v holds p address!
}

Sometimes you need something like shallow_array_adaptor. On the other hand, array_adaptor is a redundant class that unbounded_array or bounded_array can do if they have a constructor like this (or with a pair of iterators as parameters):

unbounded_array(size_t size, T* data)
: size_(size), data_(new value_type[size]) {
   std::copy(data, data+size, data_);
}

I would like to see:
1. A fixed shallow_array_adaptor or a similar storage class that references external data.
2. unbounded_array and bounded_array constructor that you pass an iterator range for copy.  adaptor_array is to use a sledgehammer to crack a nut!

Regards,
Guillermo Ruiz Troyano