[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: overloaded functions vs. internals
From: |
Jaroslav Hajek |
Subject: |
Re: overloaded functions vs. internals |
Date: |
Wed, 7 Oct 2009 22:09:40 +0200 |
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