Cleanup of subsasgn

[Max Brister]

Looking into bugs 36003 and 34778 caused me to look into the
implementation of subsasgn. There is a lot of code duplication, and
several comments indicating that this duplication exists. Worse, it
looks like the implementation of the originality duplicate sections
has started to diverge as bugs were fixed in one section, but not
another.

I have created a patch that gets rid of some, but not all of the code
duplication between octave_struct::subsasgn,
octave_scalar_struct::subsasgn, octave_class::subsasgn, and
octave_cell::subsasgn. I also tried to keep the error messages the
same. I did not get rid of the duplication between the worse
offenders, octave_struct::subsasgn and octave_class::subsasgn. It
looks to me like these classes should really inherit from a common
base class (maybe octave_struct_base?), as almost all of their methods
have a large portion of code in common. I am willing to come up with a
patch for this, if there is interest.

While looking at octave_class::subsasgn_common I found several issues:
obj.foo = 5;
obj(1).foo.bar = 5;

Is currently legal in Octave. I think this is incorrect, as

obj.foo = 5;
obj.foo.bar = 5;

is not legal. Furthermore, structs do not allow for this behaviour.

Objects in a class array should only be removed where assigned a null
value. For example,
a = [];
obj(1) = a;
should not remove obj(1).

For some 2x2 class instance, obj
[obj(:,2).foo] = {1,2}{:};
Would actually end up making obj a 1x2 array, instead of assigning 1,2
to obj(1,2),obj(2,2) respectively.

Missing check to see if index was empty in
octave_class::subsasgn_common. This check is already done elsewhere by
the interpreter, but it is possible that some mex code could call the
function directly.

These issues are fixed by the patch. For me 'make check' has the same
number of failures for 14542:bf219932bf3e with and without the patch
applied. I also expect that an analysis of the rest of the duplicate
code in octave_class and octave_struct will uncover more bugs.
Unfortunately, I have not fixed the bugs I was originally looking at.
I figure those fixed belong in a separate patch. Additionally, it
might be a good idea to introduce a patch that adds some aggressive
tests for subsasgn if they do not exist already.

Max Brister

subsasgn.patch
Description: Binary data