[SOLVED-ISH] Advice wanted on designing a c++ class managed by python: Constraint (baseclass)

Here's the place for discussion related to coding in FreeCAD, C++ or Python. Design, interfaces and structures.
User avatar
DeepSOIC
Posts: 7292
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

[SOLVED-ISH] Advice wanted on designing a c++ class managed by python: Constraint (baseclass)

Postby DeepSOIC » Tue Nov 12, 2019 1:45 pm

Hi!
I'm having trouble inventing a good memory management strategy for Constraint objects.
My goals:
* python-powered lifetime management
(i.e., no storage on stack)
* universal copy() method - I don't want to repeat it for every constraint
* constructable from Base::Type

So the base class:

Code: Select all

class Constraint: public BaseClass
{
protected://data
    ... //some members
    PyObject* _twin; // <- points to ConstraintPy object, which is responsible for deleting Constraint instance
public:
    virtual HConstraint copy() const;
    ... getPyObject();
}
The tricky part is copy(), particularly the desire to not have to implement it for every single subclass, as every one will have ParameterRef data members that I will have to list manually in the copy().
The savior seems to be the default assignment operator, which I could use from copy() to do all the copying, and then manually reset _twin to the new twin.

I have tested:

Code: Select all

// Example program
#include <iostream>

class A
{
public:
    int mmA = 0;
};

class B: public A
{
public:
    int mmB = 0;
    B(int mma, int mmb){
        mmA = mma;
        mmB = mmb;
    }
};

int main()
{
    A* inst1 = new B(1,1);
    A* inst2 = new B(2,2);
    
    *inst1 = *inst2;
    
    std::cout << "inst1 " << inst1->mmA <<" " << static_cast<B*>(inst2)->mmB; 
    
    delete inst1;
    delete inst2;
    
}
And to my surprise, assignment just worked, and copied both members of A and B.

Now the next problem, I'd like to protect that assignment. So that no one uses it to make a copy of _twin pointer, which then points to the wrong object. Which I failed to:

Code: Select all

// Example program
#include <iostream>

class A
{
public:
    int mmA = 0;
protected:
    virtual A& operator=(A& other) = default; // <-protected assignment 
};

class B: public A
{
public:
    int mmB = 0;
    B(int mma, int mmb){
        mmA = mma;
        mmB = mmb;
    }
};

int main()
{
    A* inst1 = new B(1,1);
    A* inst2 = new B(2,2);
    
    *static_cast<B*>(inst1) = *static_cast<B*>(inst2); // <- unfortunately, still works =(
    
    delete inst1;
    delete inst2;
    
}
Any advice?

For example, if I override assignment with actual code, is there any way to use the default assignment somehow?
Last edited by DeepSOIC on Tue Nov 12, 2019 4:10 pm, edited 3 times in total.
User avatar
DeepSOIC
Posts: 7292
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: Advice wanted on designing a c++ class managed by python: Constraint (baseclass)

Postby DeepSOIC » Tue Nov 12, 2019 2:12 pm

DeepSOIC wrote:
Tue Nov 12, 2019 1:45 pm
For example, if I override assignment with actual code, is there any way to use the default assignment somehow?
This:

Code: Select all

BaseClass::operator = (other);
seems to do the trick :roll:
Worth a try.
User avatar
DeepSOIC
Posts: 7292
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: Advice wanted on designing a c++ class managed by python: Constraint (baseclass)

Postby DeepSOIC » Tue Nov 12, 2019 2:27 pm

Therein lies another PITA:
\src\Base\BaseClass.h:127: warning: definition of implicit copy assignment operator for 'BaseClass' is deprecated because it has a user-declared destructor
:evil:
So how is it possible to make it work? virtual destructor of base class is fundamentally mandatory, and using = default instead of the do-nothing implementation doesn't eliminate the warning. Maybe it will work when it's finally obsolete, but IDK...
wmayer
Site Admin
Posts: 15277
Joined: Thu Feb 19, 2009 10:32 am

Re: Advice wanted on designing a c++ class managed by python: Constraint (baseclass)

Postby wmayer » Tue Nov 12, 2019 2:28 pm

The tricky part is copy(), particularly the desire to not have to implement it for every single subclass, as every one will have ParameterRef data members that I will have to list manually in the copy().
The savior seems to be the default assignment operator, which I could use from copy() to do all the copying, and then manually reset _twin to the new twin.
It's much easier. Just implement the assignment operator of the base class to do nothing -- unless you have to copy other data members except of the PyObject pointer. If you don't implement the assignment operator in a sub-class the compiler will generate it for you and automatically call the assignment operator of the base class.

Code: Select all

#include <iostream>

class BaseClass
{
public:
    int i = 0;
    BaseClass& operator = (const BaseClass&) {
        std::cout << "BaseClass\n";
        return *this;
    }
};

class SubClass : public BaseClass
{
public:
    int j = 0;
};

int main()
{
    SubClass s1,s2;
    s2.i = 1; s2.j = 4;
    s1.i = 2; s1.j = 5;

    std::cout << "i=" << s1.i << " j=" << s1.j << "\n";
    std::cout << "i=" << s2.i << " j=" << s2.j << "\n";

    s2 = s1;
    
    std::cout << "i=" << s1.i << " j=" << s1.j << "\n";
    std::cout << "i=" << s2.i << " j=" << s2.j << "\n";
}
User avatar
DeepSOIC
Posts: 7292
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: Advice wanted on designing a c++ class managed by python: Constraint (baseclass)

Postby DeepSOIC » Tue Nov 12, 2019 2:29 pm

wmayer wrote:
Tue Nov 12, 2019 2:28 pm
unless you have to copy other data members except of the PyObject pointer.
but that's exactly what I WANT to do. - copy everything **except** PyObject pointer

EDIT: oh, wait... your suggestions seems worthy!
User avatar
DeepSOIC
Posts: 7292
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: Advice wanted on designing a c++ class managed by python: Constraint (baseclass)

Postby DeepSOIC » Tue Nov 12, 2019 2:38 pm

Interesting, I very seriously didn't expect it to work. But it does!

Code: Select all

// Example program
#include <iostream>

class BaseClass
{
    int basemember = 0;
};

class A : public BaseClass
{
public:
    int mmA = 0;
public:
    A& operator=(A& other) {
        mmA = 0;
    }
};

class B: public A
{
public:
    int mmB = 0;
    B(int mma, int mmb){
        mmA = mma;
        mmB = mmb;
    }
};

int main()
{
    A* inst1 = new B(1,1);
    A* inst2 = new B(2,2);
    
    *static_cast<A*>(inst1) = *static_cast<A*>(inst2);
    
    std::cout << "inst1 " << inst1->mmA <<" " << static_cast<B*>(inst2)->mmB <<"\n"; 
    
    delete inst1;
    delete inst2;
    
}
result: mmA reset to zero, mmB copied. I never expected C++ to be that smart :shock: , I thought it will just call the operator= of A and be done with it, I didn't expect it to copy members of B.
:mrgreen: THANKS!!
wmayer
Site Admin
Posts: 15277
Joined: Thu Feb 19, 2009 10:32 am

Re: Advice wanted on designing a c++ class managed by python: Constraint (baseclass)

Postby wmayer » Tue Nov 12, 2019 2:41 pm

Therein lies another PITA:
\src\Base\BaseClass.h:127: warning: definition of implicit copy assignment operator for 'BaseClass' is deprecated because it has a user-declared destructor
I guess it's the code analyzer of QtCreator that complains about it, right? A few months ago I tried to change a few things to make the code analyzer happy with things like the export macro and a few other issues. However, the analyzer has big problems to understand forward declarations and thus will flood you with false-positive warnings. So, I disabled it again.

Btw, here the analyzer is right and to fix this specific warning you have to add these two lines:

Code: Select all

  BaseClass(const BaseClass&) = default;
  BaseClass& operator=(const BaseClass&) = default;
User avatar
DeepSOIC
Posts: 7292
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: Advice wanted on designing a c++ class managed by python: Constraint (baseclass)

Postby DeepSOIC » Tue Nov 12, 2019 2:49 pm

wmayer wrote:
Tue Nov 12, 2019 2:41 pm
Btw, here the analyzer is right and to fix this specific warning you have to add these two lines:

Code: Select all

  BaseClass(const BaseClass&) = default;
  BaseClass& operator=(const BaseClass&) = default;
:D
wmayer
Site Admin
Posts: 15277
Joined: Thu Feb 19, 2009 10:32 am

Re: Advice wanted on designing a c++ class managed by python: Constraint (baseclass)

Postby wmayer » Tue Nov 12, 2019 2:56 pm

DeepSOIC wrote:
Tue Nov 12, 2019 2:29 pm
wmayer wrote:
Tue Nov 12, 2019 2:28 pm
unless you have to copy other data members except of the PyObject pointer.
but that's exactly what I WANT to do. - copy everything **except** PyObject pointer

EDIT: oh, wait... your suggestions seems worthy!
Well, maybe it was explained a bit too circumstantially. In the assignment operator you should skip the data members you don't want to copy which in this case will be an empty implementation that only returns *this. However, if you change the class declaration on day by adding new members you also may have to adjust the assignment operator.

This is the trick I used years ago for the class Handled. It has a pointer of QAtomicInt that must not be copied either because every instance of Handled must have its own QAtomicInt instance:

Code: Select all

const Handled& Handled::operator = (const Handled&)
{
    // we must not assign _lRefCount
    return *this;
}
User avatar
DeepSOIC
Posts: 7292
Joined: Fri Aug 29, 2014 12:45 am
Location: Saint-Petersburg, Russia

Re: [SOLVED] Advice wanted on designing a c++ class managed by python: Constraint (baseclass)

Postby DeepSOIC » Tue Nov 12, 2019 3:06 pm

wmayer wrote:
Tue Nov 12, 2019 2:56 pm
However, if you change the class declaration on day by adding new members you also may have to adjust the assignment operator.
That's totally not a problem. The problem I wanted to deal with is the need of explicit listing in every subclass. And that is solved.