Re: overloaded functions vs. internals

[Jaroslav Hajek]

On Wed, Oct 7, 2009 at 8:39 PM, John W. Eaton <address@hidden> wrote:
> Given the following class constructor and method:
>
> address@hidden/foo.m:
>  function x = foo ()
>    x = class (struct (), 'foo');
>  end
>
> address@hidden/size.m:
>  function s = size (x)
>    s = [13, 42];
>  end
>
> Octave will do the following:
>
>  octave> x = foo ();
>  octave> size (x)
>  ans =
>
>     13   42
>
>  octave> whos x
>  Variables in the current scope:
>
>    Attr Name        Size                     Bytes  Class
>    ==== ====        ====                     =====  =====
>         x           1x1                          0  foo
>
> The problem is that internally, whos is computing size using the
> octave_value::dims function, and that does not call size, even though
> the octave_value::size function has been overloaded to call a
> user-defined size method if it is available.
>
> Fixing whos to call size instead of using dims, and then construct the
> dimension string from that fixes the problem for whos.  As a quick fix
> for the person who reported this problem to me, I checked in the
> following change:
>
>  http://hg.savannah.gnu.org/hgweb/octave/rev/bb413c0d0d6d
>
> However, the problem remains for any other internal function that
> calls dims on a class object that has an overloaded size function.
>
> I started working on a better fix, but ran into problems with const.
> For example, I would like to replace the current octave_class::dims
> function with
>
>  dim_vector
>  octave_class::dims (void) const
>  {
>    // Be consistent with size if it is overloaded with a user-defined
>    // method.
>
>    Matrix sz = size ();
>
>    dim_vector dv (sz.numel ());
>
>    for (int i = 0; i < dv.length (); i++)
>      dv(i) = sz(i);
>
>    return dv;
>  }
>
> but this won't work because size is not const (and can't be; see
> below).  Dropping the const qualifier from the dims function is not
> desirable, because dims is used in many other const functions, so it
> would cause a cascade of const removal.  I don't think we want that.
>
> So why isn't the octave_value::size method constant?  Because the
> octave_class definition of size is
>
>  Matrix
>  octave_class::size (void)
>  {
>    Matrix retval (1, 2, 1.0);
>    octave_value meth = symbol_table::find_method ("size", class_name ());
>
>    if (meth.is_defined ())
>      {
>        count++;
>        octave_value_list args (1, octave_value (this));
>
>        octave_value_list lv = feval (meth.function_value (), args, 1);
>        if (lv.length () == 1 && lv(0).is_matrix_type () && lv(0).dims 
> ().is_vector ())
>          retval = lv(0).matrix_value ();
>        else
>          error ("@%s/size: invalid return value");
>      }
>
>    return retval;
>  }
>
> Note the lines
>
>        count++;
>        octave_value_list args (1, octave_value (this));
>
> This sets up the argument list for the call to the user-defined
> method.  We have to increment the reference count of the octave_class
> object when we put it inside a new octave_value object so that it can
> go in the argument list.  Since count is a member of
> octave_base_value, and octave_class is derived from that,
> octave_class::size can't be const.
>
> My next thought was to make count mutable, but that is not sufficient
> because if octave_value::size is const, then "this" is const inside
> octave_value::size, and although we have an
>
>  octave_value::octave_value (octave_base_value *)
>
> constructor, there is no
>
>  octave_value::octave_value (const octave_base_value *)
>
> constructor because octave_value::rep is not const and copying the
> const argument to the non-const rep won't work.  And anyway, rep can't
> be const if we want to call any non-const member functions through the
> rep pointer.
>
> So, I'm a bit stuck here.  Any thoughts about how to get out of this
> mess?
>
> Ideally, I'd like to find a solution for the const problem so that we
> can call overloaded methods without having to give up const.  Perhaps
> there is something simple that I'm missing, but I just don't see a
> solution at the moment.
>

I mostly thought along the same lines when I was improving
octave_class - that's why I decided to create a separate non-const
size() method (previously, octave_value::size existed but only called
dims() and thus wasn't much useful), that will behave correctly even
with classes. It returns a Matrix because the original
octave_value::size did so and also because a user-defined size()
method may actually return something more complicated than a canonical
dim_vector.

Btw., for the same reason there is numel (void) const and numel (const
octave_value_list&), the latter calling overloaded numel() for classes
but the former not.

This seemed to me as the least painful approach, because I found out
the same difficulties as you did.
Currently, IMHO the only const-clean approach to do what you would
like to is to construct
octave_value (this->clone ()) and pass that to the method. That
involves some overhead of duplicating the object's internals - map and
parent_list. map is reference counted, so copying it is actually quite
cheap, parent_list is worse, but probably easily acceptable.
A dirty alternative, of course, is going via const_cast.

I didn't do it myself when thinking of it because I wasn't so sure it
is a good idea to let dims() call overloaded size() method. I think an
infinite recursion might easily occur, given how often dims() is
called. There should be some mechanism to get the internal dims, and
to avoid the recursion. map_value ().dims () will work, but is not
very nice.

Generally, I think one usually wants to handle classes as a separate
case anyway, so it doesn't hurt much to have separate methods. For
instance, currently dims() never generates an error; that would also
change. But I don't really object to such changes, just pointing out
possible problems.
The same solution and similar arguments apply to the other const
methods you refer to below.

regards

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