[Octave-bug-tracker] [bug #59500] sub-classes of octave_base_value aren't assignable

[anonymous]

Follow-up Comment #8, bug #59500 (project octave):


struct refcount
{
  refcount (int v) : i(v) {}
  refcount (const refcount&) = delete;
  refcount& operator = (const refcount&) = delete;
  int i;
};

struct octave_base_value
{
  octave_base_value () : count (1) {}
  octave_base_value (const octave_base_value&) : count (1) {}
  refcount count;
};

struct octave_scalar : octave_base_value
{
  double scalar;
};
struct octave_diag : octave_base_value
{
  
};


I can explain more. In the above example I provided a minimal example that
resembles the current (Octave 6.1) implementation of the classes related to
this thread.
Please see the struct 'refcount'. Its copy constructor (CC) and copy
assignment operator (CAO) are explicitly deleted.
Now see the struct 'octave_base_value'. It has a member named 'count' that is
of class 'refcount'. Because the refcount class has deleted CC and CAO the
default CC and CAO of octave_base_value also are automatically deleted. But we
can define custom CC and CAO for octave_base_value to override the default
behavior and enabling copying and assignment.
As we can see the current implementation of octave_base_value has a custom
copy constructor. Without the custom copy constructor octave_base_value and
all its subclasses would not be copyable.
we also can see that in the copy constructor the count member isn't copied
instead it gets the value 1. It allows to copy the data contents of octave
objects (of subclasses of octave_base_value) without copying the reference
count.
In other words one can use such expression:


octave_scalar a;
octave_scalar b(a);


But octave_base_value doesn't have a custom copy assignment operator.
Therefore it is currently impossible to write such expressions:


octave_base_value a;
octave_base_value b;
b = a;

octave_scalar c;
octave_scalar d;
d = c;


What I suggested is a minimal change that is required to enable copy
assignment of octave_base_value and its subclasses.


octave_base_value& operator = (const octave_base_value&) { return *this; }


While it seems that it does nothing but it actually does something. It, like
the copy constructor, doesn't copy the refcount. It overrides the default
behavior and enables assigning to octave_base_value and it subclasses.
The following example is invalid and won't compile:
  

octave_scalar a;
octave_diag b;

a = b;


Because octave_scalar and octave_diag are different types and octave_scalar
doesn't overload an assignment operator with an argument of type octave_diag.
However every class can define its own assignment operator. But I think it is
unneeded and the definition for octave_base_value is sufficient.

Some questions and answers:

Q: Is the decision for deleting CC and CAO of refcount in Octave 6.1 is a
correct decision?
A: Yes, I agree that the refcount shouldn't be copyable ans assignable.

Q: What are the outcomes of the new implementation of refcout?
A: Since the refcount is used in some classes other than octave_base_value
they now aren't copyable nor assignable. In a quick review I noticed that much
of them are private classes and nothing is required to be done. But I think
'base_property' in libinterp/corefcn/graphic.in.h needs a custom assignment
operator.

Q: Is defining the custom assignment operator of octave_base_value needed for
the interpreter to work correctly?
A: No, the interpreter works perfectly without the custom assignment
operator.

Q: What percentage of the users/developers may need/use/think about such a
feature?
A: I cannot estimate but I know at least one person! They can life without
that feature but with a little overhead!


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?59500>

_______________________________________________
  Message sent via Savannah
  https://savannah.gnu.org/