[ublas] Problems with range, slice, and indirect_array classes

From: Russ (c.r.coggrave_at_[hidden])
Date: 2006-04-24 12:43:19

Point 1:
========
Currently in ublas we have to write indirect_array<> to get the default template 
argument. Other classes have a typedef in fwd.hpp, such as

typedef basic_range<> range

Would it not be a good idea therefore to rename the indirect_array class 
basic_indirect_array, and introduce a new typedef in fwd.hpp as follows:

typedef basic_indirect_array<> indirect_array

Point 2:
========
Whilst studying the indirect_array class I started to add new constructors as 
follows:

indirect_array (const range &r):
indirect_array (const slice &s):

Similarly

slice (const range &r)

However, I soon realised I had a problem when r or s is a reference for all_ (i.
e. a placeholder for "all entries"). Unfortunately, when I came to look at how 
these classes handle the special condition it seems flawed and restrictive.

Consider for example,

        const slice &All( slice::all() );
        slice S(All);         // Copy constructor breaks special condition
        std::size_t size(3);
        All.preprocess(size); // Gives slice (0, 1, size)
        S.preprocess(size);   // Gives slice (0, 1, slice::size_type(-1))
 
The problem arises due to the way the preprocess() method checks for the special 
condition...

        basic_slice preprocess (size_type size) const {
            if (this != &all_)
                return *this;
            return basic_slice (0, 1, size);
        }

Here, preprocess() is assuming that the special condition only occurs if

(this == &all_)

evaluates to true. However, this test gives incorrect results when a slice 
object with the special all condition is copied.

It seems that there are 3 possible workarounds:
1. Prevent copying of range, slice, and indirect_array objects
2. Add a boolean member to these classes to indicate the special condition.
3. Perform comparison with all_ by comparing the object contents, rather than 
object address.

Taking these in order:
1. This is unnecessarily restrictive, and prevents us writing convenient code.
2. My preferred option - it adds to the size of the object slightly, but adding 
a single boolean member is never going to be a performance issue.
3. This simple workaround is not reliable. Consider the following...

        basic_slice preprocess (size_type size) const {
                 if (*this != all_)
                return *this;
            return basic_slice (0, 1, size);
        }
...
   template <class Z, class D>
    const basic_slice<Z,D> basic_slice<Z,D>::all_  (0, 1, size_type (-1));

At first sight it would seem that we can use a negative size attribute to 
represent a special case, however, our storage type for size is an unsigned 
type. Therefore, there is the risk that the user might want to use this unsigned 
value for a legitimate range.

Would anyone have any objection if I implemented solution 2.

I would also like to add a method is_all() to these classes to test for the 
special condition. This is required to properly handle objects of these classes 
for IO purposes.

Regards,

Russ