Hi Theo,

The way I discovered it was to try to use the iterators to fill the arrays in the "storage" testcase, IIRC the "C" array is a 2D, mixed ascending/descending array and if you create begin() and increment it, you get an out of bounds. Try this:

  GeneralArrayStorage<2> storage;
  storage.ordering() = firstRank, secondRank;
  storage.base() = 0,0;
  storage.ascendingFlag() = false,true;

  Array<int,2> C;
  C.setStorage(storage);
  C.resize(2,2);
  C(0,0)=0;
  C(0,1)=1;
  C(1,0)=2;
  C(1,1)=3;
  a2::iterator i=C.begin();
  cout << *i << '\t' << endl; ++i;
  cout << *i << '\t' << endl; ++i;
  cout << *i << '\t' << endl; ++i;
  cout << *i << '\t' << endl; ++i;
  assert(i==C.end());


On Sat, Jun 11, 2011 at 4:40 AM, Theodore Papadopoulo <Theodore.Papadopoulo@inria.fr> wrote:
This is clearly desirable for performance reasons. Randomizing the
memory accesses will in many cases have an important performance impact.

I agree that there will be a performance impact. However, I'm not sure I see the point of worrying about performance about something that works so unpredictably that it doesn't get used. Currently, I've simply stopped trying to use the Array iterators for anything since they are so unreliable. Though now that I think about it it the last time I tried might have been on an array that had been reverse()'d, so that would fall under the same fail mode.

Similarly, not defining an stl-compliant iterator post-increment operator for performance reasons seems misguided to me. I have no idea which algorithms rely on correct post-increment behavior, so this just means I can't trust that any will work. Really, if I'm using the iterators as opposed to blitz expressions, I don't think the cost of a temporary in the operator will matter.
 
Honestly, I would be cautious about such a change...

The trouble here is that the behavior between the old Vector and Array<x,1> is inconsistent. If you take the "initialize" test case and substitute Vector -> Array<x,1>, it will fail because the Vector behavior respects the stride, whereas the Array one does not but just writes blindly in ascending memory order from dataFirst(). There is thus no way to keep current behavior and also make it consistent. In my mind, the array behavior simply can't be considered correct since it will corrupt memory for non-ascending arrays and change elements not part of the array for noncontiguous arrays. 

I don't feel so strongly about the logical order, so maybe we can settle on:

* iterators should work correctly on all arrays. The traversal order can depend on the storage, but for "simple" contiguous arrays with ascending order, it will be from dataFirst() in increasing memory order.

* list initialization works in iterator order. This will preserve current behavior for "simple" arrays and work correctly in the cases that now are broken, so should not break existing code.

Thoughts?

/Patrik