Re: [major] struct array indexing in tip

[John W. Eaton]

[moved from the bug list.  --jwe]

On 21-Jan-2009, Jaroslav Hajek wrote:

| Okay, I've seen a couple of bugs myself, so here's another shot:
| http://hg.savannah.gnu.org/hgweb/octave/rev/906f976d35a8
| 
| This time, I didn't try to allow further indexing on cs-lists, like
| a(1:2).x(1), because Matlab doesn't allow it either, and it
| complicates things a little.
| 
| octave:1> [a(1:2).x(2)] = deal(3,4)
| error: a cs-list cannot be further indexed
| error: evaluating argument list element number 1

OK.

| I've revamped the tree_index_expression::lvalue code to avoid useless
| evaluation of the trailing indexing expr just for getting dimensions,
| so things should be somewhat faster now. Also, re-evaluation of an
| already evaluated portion of the index chain is avoided in both lvalue
| and rvalue, so that the following code calls testfun only once:
| 
| function y = testfun (x)
|   disp ('called');
|   for i = 1:5
|     y{i} = (1:i) * x;
|   endfor
| endfunction
| 
| testfun (5) {end} (end)
| 
| instead of 3 times (prior to this patch).

OK, I think this was the intent all along, but I'm not surprised that
it was not doing the right thing...

| Also, I've disallowed automatic resizing in the normal subsref
| function, so that the following code will now produce a proper error:
| 
| a(1).x.x = 1
| a(2).x

OK.

| > Maybe someone (other than Jaroslav) would like to generate some tests
| > so these problems don't resurface later?
| >
| 
| That would be great. I *may* do it eventually myself, but no promise :)

We need the tests, so I'm hoping someone else will have the time to do
it.  Anyone?

| I'd still like to make more changes to the indexing code:
| 
| First, an indexed assignment to an out-of-bounds element that does not
| occur last in the indexing chain
| does unnecessarily resize the array three times via the indexing with
| resize_ok. I don't think that Array<T>::index (..., resize_ok) is
| actually used by any instance than Cell (i.e. Array<octave_value>),
| and probably the only correct usage is with scalar-resulting indices,
| so I'd like to move the methods away from Array<T> to Cell and
| optimize the all-scalars case to avoid the unnecessary resizes.

OK.

| Second, given that octave_value_lists are often converted to Cells and
| vice versa, I'd like to consider rebasing octave_value_list as a Cell
| subclass. That would allow virtually all argument-passing code benefit
| from the shallow copy mechanism of Array<T> (now even for contiguous
| subranges), and thus reduce the amount of copying octave_values
| around. That would allow things like func (args{:}) to work without
| copying.

I think this is OK too, but should octave_value_list be derived from
Cell, or contain a Cell?  Does it need to inherit all the operations
of a Cell?  If not, I think maybe it would be better if it simply
contained a Cell instead of a std::vector<octave_value> object.  But
I haven't given it a lot of thought so maybe I'm missing something.

jwe