Mailing List Archive

"base class should be explicitly initialized" errors
Hi all. I'm still slogging my way through all the warnings created by
adding the -Wextra flag to compiles, and decided it was finally time to
tackle this class of warning:

In copy constructor ‘blah::blah(const blah&)’:
warning: base class ‘class QObject’ should be explicitly
initialized in the copy constructor [-Wextra]
  blah::blah(const blah &other)

This warning occurs on objects defined in these files:

libs/libmythbase/filesysteminfo.h
libs/libmythbase/mythdownloadmanager.cpp
libs/libmythbase/mythsystemlegacy.h
libs/libmythmetadata/metadatagrabber.h
libs/libmythservicecontracts/enums/recStatus.h
Everything under libs/libmythservicecontracts/datacontracts

The trivial solution would seem to be initializing the base qobject in
the copy constructor, only that results in a different bunch of
warnings like this:

In copy constructor ‘blah::blah(const blah&)’:
error: ‘QObject::QObject(const QObject&)’ is private within
this context : QObject(other)
/usr/include/qt5/QtCore/qobject.h: note: declared private here
     Q_DISABLE_COPY(QObject)

That got me researching the topic, which led me to these references..

    http://doc.qt.io/qt-5/qobject.html#Q_DISABLE_COPY
    https://www.ics.com/designpatterns/book/qobject.html
 
..which basically say you should never copy things derived from a
qobject. They represent unique entities, not things that can be run
through a xerox machine.

There are simple solutions for a couple of these files. The
MetaGrabberScript object defined in metadatagrabber.h doesn't appear to
use any of the properties of a QObject, so a one line change to make it
a standalone class produces a clean compile. The simple solution for
the MythSystemLegacy object boils down to removing the unused copy
constructor.

The solution for the FileSystemInfo object is fairly straight forward.
The copy constructor can be removed, and the rest of MythTV can be
modified to pass around pointers (or lists of pointers) to
FileSystemInfo objects. There is some ripple effect into a few other
files, as direct references to object functions have to be converted
into indirect references to the same functions.

The MythCookieJar object in mythdownloadmanager.cpp needs a little more
thought given to how to fix it. Aside from loading/saving cookies, its
purpose appears to be to copy cookie jars from one network manager to
another. This makes it hard to not want a copy constructor. Perhaps
instead of creating new cookie jars each time, a way could be found to
use the allCookies/setAllCookies functions and only update the contents
of the jars, not create new jars each time.

The last problem is everything in under libmythservicecontracts. All of
these objects defined under this directory declare and register
metatypes for both the object and a pointer to the object. This sets up
a contradiction though, as using the Q_DECLARE_METATYPE macro on an
object requires that the object have a public copy constructor and
being based on QObject requires that it doesn't. This contradiction
doesn't extend to *pointers* to objects based on QObject; in fact
pointers to classes derived from QObject are automatically registered
with the QMetaType system and you don't even need to use the
Q_DECLARE_METATYPE macro. The CopyListContents function in
datacontracthelper.h also needs to be tweaked as its current form
implicitly creates copies of objects.

If anyone has any thoughts on any of this I'd love to hear it. I have
prototype changes that compile cleanly, and I can start a
backend/frontend pair and perform basic actions like watching
recordings and videos, visiting web sites, etc.  If anyone wants to
look through my diffs I can post them somewhere (or create a pull
request, although I know several of the fixes still need refinement).

Thanks for reading this far.

David




_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org
Re: "base class should be explicitly initialized" errors [ In reply to ]
Hi

On 3 June 2017 at 02:06, David Hampton <mythtv@dhampton.net> wrote:
> Hi all. I'm still slogging my way through all the warnings created by
> adding the -Wextra flag to compiles, and decided it was finally time to
> tackle this class of warning:
>
> In copy constructor ‘blah::blah(const blah&)’:
> warning: base class ‘class QObject’ should be explicitly
> initialized in the copy constructor [-Wextra]
> blah::blah(const blah &other)

Yes, the base constructor must be called here.. I'm surprised it could
ever compile without it.

>
> This warning occurs on objects defined in these files:
>
> libs/libmythbase/filesysteminfo.h
> libs/libmythbase/mythdownloadmanager.cpp
> libs/libmythbase/mythsystemlegacy.h
> libs/libmythmetadata/metadatagrabber.h
> libs/libmythservicecontracts/enums/recStatus.h
> Everything under libs/libmythservicecontracts/datacontracts
>
> The trivial solution would seem to be initializing the base qobject in
> the copy constructor, only that results in a different bunch of
> warnings like this:
>
> In copy constructor ‘blah::blah(const blah&)’:
> error: ‘QObject::QObject(const QObject&)’ is private within
> this context : QObject(other)
> /usr/include/qt5/QtCore/qobject.h: note: declared private here
> Q_DISABLE_COPY(QObject)
>
> That got me researching the topic, which led me to these references..
>
> http://doc.qt.io/qt-5/qobject.html#Q_DISABLE_COPY
> https://www.ics.com/designpatterns/book/qobject.html
>
> ..which basically say you should never copy things derived from a
> qobject. They represent unique entities, not things that can be run
> through a xerox machine.

well you can, but not a plain QObject. But things like QString and so
on, handle Copy on Write
_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org
Re: "base class should be explicitly initialized" errors [ In reply to ]
On Tue, 2017-06-06 at 10:45 +0200, Jean-Yves Avenard wrote:
> Hi
>
> On 3 June 2017 at 02:06, David Hampton <mythtv@dhampton.net> wrote:
> > Hi all. I'm still slogging my way through all the warnings created
> > by
> > adding the -Wextra flag to compiles, and decided it was finally
> > time to
> > tackle this class of warning:
> >
> >   In copy constructor ‘blah::blah(const blah&)’:
> >   warning: base class ‘class QObject’ should be explicitly
> >   initialized in the copy constructor [-Wextra]
> >   blah::blah(const blah &other)
>
> Yes, the base constructor must be called here.. I'm surprised it
> could
> ever compile without it.
>
> >
> > This warning occurs on objects defined in these files:
> >
> >   libs/libmythbase/filesysteminfo.h
> >   libs/libmythbase/mythdownloadmanager.cpp
> >   libs/libmythbase/mythsystemlegacy.h
> >   libs/libmythmetadata/metadatagrabber.h
> >   libs/libmythservicecontracts/enums/recStatus.h
> >   Everything under libs/libmythservicecontracts/datacontracts
> >
> > The trivial solution would seem to be initializing the base qobject
> > in
> > the copy constructor, only that results in a different bunch of
> > warnings like this:
> >
> >   In copy constructor ‘blah::blah(const blah&)’:
> >   error: ‘QObject::QObject(const QObject&)’ is private within
> >   this context : QObject(other)
> >   /usr/include/qt5/QtCore/qobject.h: note: declared private here
> >      Q_DISABLE_COPY(QObject)
> >
> > That got me researching the topic, which led me to these
> > references..
> >
> >     http://doc.qt.io/qt-5/qobject.html#Q_DISABLE_COPY
> >     https://www.ics.com/designpatterns/book/qobject.html
> >
> > ..which basically say you should never copy things derived from a
> > qobject. They represent unique entities, not things that can be run
> > through a xerox machine.
>
> well you can, but not a plain QObject. But things like QString and so
> on, handle Copy on Write

QString isn't derived from QObject.

The problem as I understand it is this: QObject supports parent/child
object relationships using a single pointer for the parent and a list
for the children. If you were to copy a QObject, you would get a new
object that points to a parent that doesn't know about it, but much
worse you would have two objects that contain the same list of
children. If either of these objects is destroyed, thereby destroying
all its children, the other object ends up holding a list of dangling
pointers.

This holds true for any object derived from a QObject. In order to
create a true copy of that object, you would need to copy the QObject
it is derived from. You can't do that without running into the same
problem as copying a plain QObject. On the other hand, if your "copy"
function doesn't copy the entire object including the base QObject, but
instead creates a new base QObject with no parent and no children while
copying everything else, is it truly a copy function? (I know, that
sounds like a semantics question.)

I don't have any right answers here. I'm just trying to figure out the
implications of the Q_DISABLE_COPY macro, and what's the right solution
for MythTV. I'll implement whatever the group decides is the right
solution.

David

_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org
Re: "base class should be explicitly initialized" errors [ In reply to ]
On 6 June 2017 5:45:14 pm David Hampton <mythtv@dhampton.net> wrote:

> On Tue, 2017-06-06 at 10:45 +0200, Jean-Yves Avenard wrote:
>> Hi
>>
>> On 3 June 2017 at 02:06, David Hampton <mythtv@dhampton.net> wrote:
>> > Hi all. I'm still slogging my way through all the warnings created
>> > by
>> > adding the -Wextra flag to compiles, and decided it was finally
>> > time to
>> > tackle this class of warning:
>> >
>> >   In copy constructor ‘blah::blah(const blah&)’:
>> >   warning: base class ‘class QObject’ should be explicitly
>> >   initialized in the copy constructor [-Wextra]
>> >   blah::blah(const blah &other)
>>
>> Yes, the base constructor must be called here.. I'm surprised it
>> could
>> ever compile without it.
>>
>> >
>> > This warning occurs on objects defined in these files:
>> >
>> >   libs/libmythbase/filesysteminfo.h
>> >   libs/libmythbase/mythdownloadmanager.cpp
>> >   libs/libmythbase/mythsystemlegacy.h
>> >   libs/libmythmetadata/metadatagrabber.h
>> >   libs/libmythservicecontracts/enums/recStatus.h
>> >   Everything under libs/libmythservicecontracts/datacontracts
>> >
>> > The trivial solution would seem to be initializing the base qobject
>> > in
>> > the copy constructor, only that results in a different bunch of
>> > warnings like this:
>> >
>> >   In copy constructor ‘blah::blah(const blah&)’:
>> >   error: ‘QObject::QObject(const QObject&)’ is private within
>> >   this context : QObject(other)
>> >   /usr/include/qt5/QtCore/qobject.h: note: declared private here
>> >      Q_DISABLE_COPY(QObject)
>> >
>> > That got me researching the topic, which led me to these
>> > references..
>> >
>> >     http://doc.qt.io/qt-5/qobject.html#Q_DISABLE_COPY
>> >     https://www.ics.com/designpatterns/book/qobject.html
>> >
>> > ..which basically say you should never copy things derived from a
>> > qobject. They represent unique entities, not things that can be run
>> > through a xerox machine.
>>
>> well you can, but not a plain QObject. But things like QString and so
>> on, handle Copy on Write
>
> QString isn't derived from QObject.
>
> The problem as I understand it is this: QObject supports parent/child
> object relationships using a single pointer for the parent and a list
> for the children. If you were to copy a QObject, you would get a new
> object that points to a parent that doesn't know about it, but much
> worse you would have two objects that contain the same list of
> children. If either of these objects is destroyed, thereby destroying
> all its children, the other object ends up holding a list of dangling
> pointers.
>
> This holds true for any object derived from a QObject. In order to
> create a true copy of that object, you would need to copy the QObject
> it is derived from. You can't do that without running into the same
> problem as copying a plain QObject. On the other hand, if your "copy"
> function doesn't copy the entire object including the base QObject, but
> instead creates a new base QObject with no parent and no children while
> copying everything else, is it truly a copy function? (I know, that
> sounds like a semantics question.)
>
> I don't have any right answers here. I'm just trying to figure out the
> implications of the Q_DISABLE_COPY macro, and what's the right solution
> for MythTV. I'll implement whatever the group decides is the right
> solution.
>
> David
>
> _______________________________________________
> mythtv-dev mailing list
> mythtv-dev@mythtv.org
> http://lists.mythtv.org/mailman/listinfo/mythtv-dev
> http://wiki.mythtv.org/Mailing_List_etiquette
> MythTV Forums: https://forum.mythtv.org

David,

Many people have looked at this conundrum and most have decided that if you
have found that you have to copy or clone a class instance that is derived
from QObject then someone has made a bad design decision to use QObject as
the base class. I found that in a lot of cases you could just remove the
underlying QObject related stuff and nine times out ten the code would
compile quite happily as none of the QObject functionality
was actually being used. It is usually only required if one actually needs
to use QT signals and slots.

Roger


_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org
Re: "base class should be explicitly initialized" errors [ In reply to ]
On Tue, 2017-06-06 at 19:21 +0100, Roger James wrote:
> On 6 June 2017 5:45:14 pm David Hampton <mythtv@dhampton.net> wrote:
>
> > On Tue, 2017-06-06 at 10:45 +0200, Jean-Yves Avenard wrote:
> > > Hi
> > >
> > > On 3 June 2017 at 02:06, David Hampton <mythtv@dhampton.net>
> > > wrote:
> > > > Hi all. I'm still slogging my way through all the warnings
> > > > created
> > > > by
> > > > adding the -Wextra flag to compiles, and decided it was finally
> > > > time to
> > > > tackle this class of warning:
> > > >
> > > >   In copy constructor ‘blah::blah(const blah&)’:
> > > >   warning: base class ‘class QObject’ should be explicitly
> > > >   initialized in the copy constructor [-Wextra]
> > > >   blah::blah(const blah &other)
> > >
> > > Yes, the base constructor must be called here.. I'm surprised it
> > > could
> > > ever compile without it.
> > >
> > > >
> > > > This warning occurs on objects defined in these files:
> > > >
> > > >   libs/libmythbase/filesysteminfo.h
> > > >   libs/libmythbase/mythdownloadmanager.cpp
> > > >   libs/libmythbase/mythsystemlegacy.h
> > > >   libs/libmythmetadata/metadatagrabber.h
> > > >   libs/libmythservicecontracts/enums/recStatus.h
> > > >   Everything under libs/libmythservicecontracts/datacontracts
> > > >
> > > > The trivial solution would seem to be initializing the base
> > > > qobject
> > > > in
> > > > the copy constructor, only that results in a different bunch of
> > > > warnings like this:
> > > >
> > > >   In copy constructor ‘blah::blah(const blah&)’:
> > > >   error: ‘QObject::QObject(const QObject&)’ is private within
> > > >   this context : QObject(other)
> > > >   /usr/include/qt5/QtCore/qobject.h: note: declared private
> > > > here
> > > >      Q_DISABLE_COPY(QObject)
> > > >
> > > > That got me researching the topic, which led me to these
> > > > references..
> > > >
> > > >     http://doc.qt.io/qt-5/qobject.html#Q_DISABLE_COPY
> > > >     https://www.ics.com/designpatterns/book/qobject.html
> > > >
> > > > ..which basically say you should never copy things derived from
> > > > a
> > > > qobject. They represent unique entities, not things that can be
> > > > run
> > > > through a xerox machine.
> > >
> > > well you can, but not a plain QObject. But things like QString
> > > and so
> > > on, handle Copy on Write
> >
> > QString isn't derived from QObject.
> >
> > The problem as I understand it is this: QObject supports
> > parent/child
> > object relationships using a single pointer for the parent and a
> > list
> > for the children.  If you were to copy a QObject, you would get a
> > new
> > object that points to a parent that doesn't know about it, but much
> > worse you would have two objects that contain the same list of
> > children. If either of these objects is destroyed, thereby
> > destroying
> > all its children, the other object ends up holding a list of
> > dangling
> > pointers.
> >
> > This holds true for any object derived from a QObject. In order to
> > create a true copy of that object, you would need to copy the
> > QObject
> > it is derived from. You can't do that without running into the same
> > problem as copying a plain QObject. On the other hand, if your
> > "copy"
> > function doesn't copy the entire object including the base QObject,
> > but
> > instead creates a new base QObject with no parent and no children
> > while
> > copying everything else, is it truly a copy function? (I know, that
> > sounds like a semantics question.)
> >
> > I don't have any right answers here. I'm just trying to figure out
> > the
> > implications of the Q_DISABLE_COPY macro, and what's the right
> > solution
> > for MythTV. I'll implement whatever the group decides is the right
> > solution.
> >
> > David
>
> David,
>
> Many people have looked at this conundrum and most have decided that
> if you 
> have found that you have to copy or clone a class instance that is
> derived 
> from QObject then someone has made a bad design decision to use
> QObject as 
> the base class. I found that in a lot of cases you could just remove
> the 
> underlying QObject related stuff and nine times out ten the code
> would 
> compile quite happily as none of the QObject functionality
> was actually being used. It is usually only required if one actually
> needs 
> to use QT signals and slots.
>
> Roger

Roger,

Unfortunately the MythSystemLegacy object uses signals, and all the
stuff in libmythservicecontracts uses the Q_PROPERTY macro which is
dependent on using QObjects. 

I'm not sure how I missed that FileSystemInfo just didn't need to be a
QObject. Thank you for making me double check everything.

David
_______________________________________________
mythtv-dev mailing list
mythtv-dev@mythtv.org
http://lists.mythtv.org/mailman/listinfo/mythtv-dev
http://wiki.mythtv.org/Mailing_List_etiquette
MythTV Forums: https://forum.mythtv.org