Re: fix null assignment

[Jaroslav Hajek]

On Fri, Sep 19, 2008 at 6:50 PM, Jaroslav Hajek <address@hidden> wrote:
> On Fri, Sep 19, 2008 at 5:50 PM, John W. Eaton <address@hidden> wrote:
>> On 19-Sep-2008, Jaroslav Hajek wrote:
>>
>> | It seems that there is a need to propagate the information (whether
>> | this is a deletion statement) fairly deeply - thinking of things like
>> | a(3).x.y(1:2,:) = []
>> | that should work... this stuff is resolved by subsasgn, and creating
>> | methods like subsnulasgn would probably mean a lot of duplication. It
>> | seems better to make a special kind of object for the "genuine" empty
>> | matrix that was not touched by expressions. I'm just not sure where to
>> | put it.
>>
>> I think there are a few things to consider.  First, for an expression
>> like "X(I) = []", if X is a class object, the subsasgn method for the
>> class is still called.  So in that case, if we have the parser
>> recognize "= []" as a special case and convert it to a call to a
>> function that deletes elements, then that function will need to
>> dispatch to the subsasgn method for the given class.
>>
>> Another possibility is to tag matrices that are created with [], then
>> use that information in the assign2 and assignN functions to decide
>> when to call maybe_delete_elements.  So instead of
>>
>>  if (rhs_nr == 0 && rhs_nc == 0)
>>    {
>>      lhs.maybe_delete_elements (idx_i, idx_j);
>>    }
>>
>> in the assign2 function in Array.cc, we would have something like
>>
>>  if (rhs.is_null_array ())
>>    {
>>      lhs.maybe_delete_elements (idx_i, idx_j);
>>    }
>>
>> But is this the right place for the check?  Maybe not, since it only
>> happens when the RHS is [] (i.e., double) and the LHS can be anything
>> (even a struct array).  So it should probably be handled a little
>> higher up, and the detection and call to maybe_delete_elements should be
>> removed from the assign2 and assignN functions.  In that case, I think
>> the proper places for this are the various octave_value subsasgn
>> methods.  Then the special tag for the [] value could go in the
>> octave_value object itself.  Perhaps there could even be a special
>> octave_value type for this object, similar to the octave_magic_colon
>> type.  In any case, the special nature of this value/type should not
>> be propagated by assignment or copying since
>>
>>  x(idx) = []
>>
>> deletes, but
>>
>>  y = []; x(idx) = y
>>
>> does not.
>>
>> Unless someone else is interested, I'll take a look at making these
>> changes.
>>
>
> I was about to look at this; however, if you have a better idea, I'd
> say you go ahead.
>
> My aim was to avoid checking for some special value in the
> octave_value assignment operator & copy constructor (as you have said,
> it should not propagate through by copying). octave_values are copied
> around a lot and checking stuff there just doesn't seem nice to me.
> My idea was different:
> Add a new octave_value::assign_op (say, octave_value::op_nul_asn_eq)
> and simply register proper handlers for it. Then, subclass
> tree_null_matrix from tree_constant, and have the
> tree_simple_assignment with etype = op_asn_eq mutate into
> op_nul_asn_eq when it detects that the rhs is a tree_null_matrix
> instance. The calls to maybe_delete_elements will be removed from
> assignX and called directly.
>
> What do you think about this?

OK, I see this is not that easy. Currently, the various operators in
octave_value::assign_op have no handlers actually registered; instead,
they are always decomposed in octave_value::assign and they cannot be
propagated down through subsagn & numeric_assign. This is even marked
as a FIXME in ov.cc; perhaps it is time to do it now?

I'd like to know your opinion on this before I start to do anything
with this matter.

-- 
RNDr. Jaroslav Hajek
computing expert
Aeronautical Research and Test Institute (VZLU)
Prague, Czech Republic
url: www.highegg.matfyz.cz